[lworld] RFR: 8359370: [lworld] allow instance fields of identity classes to be readable in the prologue phase [v13]

[Maurizio Cimadamore]

On Thu, 28 Aug 2025 20:33:39 GMT, Vicente Romero <vromero at openjdk.org> wrote:

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1261:
>> 
>>> 1259:                     for (JCTree stat : tree.body.stats) {
>>> 1260:                         prologueCode.add(stat);
>>> 1261:                         /* gather all the stats in the body until a `super` or `this` constructor invocation is found,
>> 
>> I understand that you  wanted to simplify the visitor -- but doing a linear pass on the constructor and creating a new list of statements is also kind of expensive -- maybe when we're done with this change we can see if there's a way to set a flag on the visitor to shortcircuit the analysis after the super call is found.
>
> I think I like the trade-off, we could try to infer if a constructor invocation corresponds to the class we are interested in. Like for example analyzing the symbol associated to a `super` or `this` invocation. But for erroneous invocations the symbol could be null. So what to do when we find a null symbol? We would have no clues I think.

let's wait until we address the other comments first. What I had in mind (but can be addressed in a separate PR) was maybe have a general visitor for prologue -- e.g. an helper visitor class that only visits things inside the prologue. Then you can extend that helper visitor here, to do what you need to do.

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1363:
>> 
>>> 1361:                 try {
>>> 1362:                     analyzingSelect = true;
>>> 1363:                     super.visitSelect(tree);
>> 
>> Can't we cut recursion here (instead of using `analyzingSelect` ? That's also what the new `TreeInfo.symbolsFor` does. In general it seems like these two visitors are trying to do similar things but are not 100% aligned?
>
> if one has a complex select like for example: `new SuperInitFails(){}.x` it is still necessary to look inside and see if there are some forbidden accesses

Ok, the issue might then be that, while looking inside `SuperInitFails` you find a plain early access to a field (e.g. an ident) and we end up skipping it because `analyzingSelect` is set. At the very least we should unset inside classes (maybe lambdas too).

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1420:
>> 
>>> 1418:             Symbol sym = TreeInfo.symbolFor(tree);
>>> 1419:             if (sym != null) {
>>> 1420:                 if (!sym.isStatic() && !isMethodArgument(tree)) {
>> 
>> if you have a `sym`, in order to understand if something is a method parameter (not argument?) don't you need to check if `sym.owner == MTH` ?
>
> this is for cases when we have an argument that is for example of the same type as the current class so like:
> 
> class Test {
>     String s;
>     
>     Test(Test t) {
>         // the owner of s is Test not MTH so we need to check what is the qualifier for s which at the end is the argument
>         // `t` so we ignore it
>         String s1 = t.s;
>         super();
>     }
> }

yes, so isn't just checking owner.kind != TYP enough? (e.g. "not a field")

>> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1423:
>> 
>>> 1421:                     if (sym.name == names._this || sym.name == names._super) {
>>> 1422:                         // are we seeing something like `this` or `CurrentClass.this` or `SuperClass.super::foo`?
>>> 1423:                         if (TreeInfo.isExplicitThisReference(
>> 
>> Do we always report an error when seeing `Foo.this` ? What if we're not inside the prologue of `Foo` ?
>
> all the code we analyze in this visitor is in the prologue, this is why we pre-select what code we will see

I understand -- but in the prologue of Foo we can have a Bar.this where Bar is some enclosing class?


class Foo {
        class Bar {
            Bar() { Object o = Foo.this; super();}
        }
}

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

PR Review Comment: https://git.openjdk.org/valhalla/pull/1523#discussion_r2309775683
PR Review Comment: https://git.openjdk.org/valhalla/pull/1523#discussion_r2309762993
PR Review Comment: https://git.openjdk.org/valhalla/pull/1523#discussion_r2309770322
PR Review Comment: https://git.openjdk.org/valhalla/pull/1523#discussion_r2309768254