On Thu, 28 May 2026 15:39:05 GMT, Maurizio Cimadamore <[email protected]> wrote:
>> This PR moves the early access checks back to `Resolve` from `Attr`. >> >> This simplifies the code, as we no longer need a separate visitor to check >> for early references. >> >> To do this, the code uses a simple record called `EarlyConstructorContext` >> which is used to keep track "facts" about where we are in attribution with >> respect to early initialization. This means tracking: >> * whether we need to issue warnings or errors >> * whether we are inside a lambda, or a nested class >> * who is the owner of the early construction context >> >> An instance of this class is stored in `AttrContext`, generalizing the >> current `ctorPrologue` field. `Attr` updates this field -- so, for instance, >> inside `visitLambda` a "nested" early context is derived and stored in >> `AttrContext`. >> >> The most intricate bit has to do with how to handle stuff like `Foo.this.x`. >> There are many complected issues here: >> * when we see `Foo.this` we don't know if `x` is a method or a field (so we >> might report issues too early) >> * when, later, we see `x` in Resolve, we don't know if the qualifier was >> `Foo.this` (so we might miss some issues). >> >> To overcome these problems, some coupling between `Attr` and `Resolve` is >> unavoidable. In this PR, `Attr::visitSelect` checks whether the qualifier >> happens to be `this/super` or `Foo.this/super`. If so, it _unsets_ the early >> construction context. Effectively this means that `Resolve` will NOT report >> an error. >> >> Then, in such cases, `Attr` will also pass the `selected` part to >> `selectSym` which will pass it to `Resolve::findIdentInType`. This means >> that, at this point, we can finally check `x`, but knowing that the >> qualifier was `Foo.this`. >> >> Another improvement is that there's no longer duplication between `Resolve` >> and `Attr::checkAssignable`. This duplication is present in mainline, but >> the current lworld repo (thanks to the work @vicente-romero-oracle did) >> removed that. I wanted to maintain that cleanup. To do that, now Attr passes >> the expected kind to some methods in `Resolve`. This kind is then used to >> tell whether we're inside an assignment or not. >> >> This strategy doesn't require looking inside `env.tree`, so it naturally >> side-steps issues such as https://bugs.openjdk.org/browse/JDK-8385431. >> >> I've also simplified the test harness to remove `super` from some of the >> tests (this harness is used to check the warning code path). It is now a >> simple annotation processor, so it can focus on just rewriting the tree and >> nothing else. >> ... > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Fix TestNullableInlineTypes test src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 4016: > 4014: > 4015: /** Implements early access checks for unqualified field references > (6.5.6.1) */ > 4016: private boolean isSimpleEarlyFieldRefAllowed(DiagnosticPosition pos, There's more comments than code here -- but what I found when working with this code (and the one it replaces in master) is that it's very easy to forget as to _why_ a check was there, so I thought it would be better to capture. If we think it's too much I can compress ------------- PR Review Comment: https://git.openjdk.org/valhalla/pull/2481#discussion_r3319602002
