On Thu, 28 Aug 2025 12:55:05 GMT, Vicente Romero <[email protected]> wrote:

>> Before this fix only strict fields were readable in the prologue phase. The 
>> proposed fix should allow any instance fields of identity classes to be 
>> readable in the prologue phase. This implies changes in flow analysis as 
>> before we were only tracking final and strict fields. There is also some 
>> "cooperation" needed in the code to detect cases when reading a field is not 
>> allowed in the prologue phase. For example some methods in Resolve don't 
>> have all the needed information at the moment they are dealing with some 
>> ASTs and part of the processing needs to be done in Attr
>> 
>> TIA
>> 
>> This PR is a remake of https://github.com/openjdk/valhalla/pull/1490
>
> Vicente Romero has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   minor diff

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1309:

> 1307:                 Symbol sym = symAndTree.symbol();
> 1308:                 JCTree tree = TreeInfo.skipParens(symAndTree.tree());
> 1309:                 if (sym.kind == VAR &&

Why doesn't this call analyze symbol?

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` ?

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` ?

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java line 1564:

> 1562:                         if (v.owner.kind == TYP && !v.isStatic() && 
> v.isStrict()) {
> 1563:                             // strict field initializers are inlined in 
> constructor's prologues
> 1564:                             CtorPrologueVisitor ctorPrologueVisitor = 
> new CtorPrologueVisitor(initEnv);

Nice reuse

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

PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1523#discussion_r2307950873
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1523#discussion_r2307957523
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1523#discussion_r2307959699
PR Review Comment: 
https://git.openjdk.org/valhalla/pull/1523#discussion_r2307961723

Reply via email to