[code-reflection] RFR: Cleanup and consolidate `ReflectMethods` [v2]

[Maurizio Cimadamore]

On Fri, 27 Jun 2025 12:28:11 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> This PR applies some minor cleanups to `ReflectMethods` and its companion classes.
>> 
>> * A symbol was moved from `CodeReflectionSymbols` to `Symtab` as it was a method in java.base (MH::lookup)
>> * `CodeModelToAST` was merged with `ReflectMethods` (and renamed to `CodeModelTranslator`)
>> * To facilitate the above, all the conversion from model types to javac types (and back) is centralized in `ReflectMethods`
>> * `ReflectMethod.BodyScanner` is now a regular scanner -- the filtering logic is not needed since all the various relevant trees are supported
>> * removed static methods to create javac diagnostics -- we now use the standard methods generated during the build (but this needed to add a qualified export of such generated classes to `jdk.incubator.code`)
>> 
>> There are several `@@@` comments in the code -- for now I left them in place.
>> 
>> I believe there are three main areas for subsequent cleanup and/or consolidation:
>> 
>> ##### Quotable vs. Quoted
>> 
>> The experiments with lambda to `Quoted` conversions were a first stab on how we might want to support code models in lambdas. This requires ad-hoc type system support (in `Attr`), and `ReflectMethods` does not support method references with this strategy. We should consider whether to keep this, given that we seem to have given `Quotable` a more prominent role in the API (and the language support for `Quotable` is much more in sync with what we do for e.g. serialziable lambdas).
>> 
>> ##### OpBuilder vs. Text
>> 
>> `ReflectMethods` still supports two code generation strategies. This doesn't add a lot of additional code, but I wonder if we really need this.
>> 
>> ##### Inner class support
>> 
>> The support for member inner classes and local/anonymous inner classes is a bit spotty. The generated model will emit correct code when e.g. creating an inner class -- that is, the model will correctly pass the enclosing this and captured values (where needed).
>> 
>> But, when translating e.g. a method in an inner class, references to enclosing this, or captured values will not be treated specially -- e.g. `ReflectMethods` will assume such elements to be "in scope". In other words, I have very little confidence that, e.g. we can take a code model corresponding to a method inside an inner class and e.g. generate bytecode for it.
>> 
>> I wonder if it would be better, for the time being, to just skip reflectable methods/constructors/lambdas defined inside a non-static inner class (so as to provide a cleaner boundary ...
>
> Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Cleanup `BodyScanner::loadVar`

src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/ReflectMethods.java line 168:

> 166:     }
> 167: 
> 168:     // Cannot compute within constructor due to circular dependencies on bootstrap compilation

CodeModelToAST was doing this translation in a more robust way, so I kept that code and ditched this one.

src/jdk.incubator.code/share/classes/jdk/incubator/code/internal/ReflectMethods.java line 1065:

> 1063:         private Value loadVar(Symbol sym) {
> 1064:             Value varOp = varOpValue(sym);
> 1065:             Assert.check(varOp.type() instanceof VarType);

The support for captured values in quotable/quoted lambdas has changes few times. Now we pre-generate captured block parameters before we start scanning the body, so there's no need to do anything special here.

-------------

PR Review Comment: https://git.openjdk.org/babylon/pull/468#discussion_r2171934916
PR Review Comment: https://git.openjdk.org/babylon/pull/468#discussion_r2171942271