RFR: 8349754: Invalid "early reference" error when class extends an outer class [v2]

[Archie Cobbs]

On Mon, 17 Feb 2025 17:14:10 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Hmm, re-reading that I think maybe I misinterpreted your statement... For clarity, let's just enumerate the possibilities.
>> 
>> Suppose we are inside a class `C` and there is some instance field `x` accessible to us.
>> 
>> Then any such `x` must be declared in one (_or both_) of these two "hierarchies":
>> * **Inner hierarchy**  - `x` is declared in `C` or (non-privately) in a superclass of `C`
>> * **Outer hierarchy**  - `x` is declared in a some outer class `O` containing `C`
>> 
>> In the case `x` is declared in both hierarchies, it may _also_ be the case that `O` is a superclass of `C`, but for our purposes that's "just a coincidence" - it doesn't affect how many `x` field's are accessible to us (i.e., two).
>> 
>> Now what can the various expressions mean depending on which of the two hierarchies `x` is declared in?
>> 
>> | Inner? | Outer? | `C` extends `O` | `x`  | `C.this.x` | `O.this.x` | Comments |
>> | :---:  | :---: | :---:  | :---: | :---: | :---: | :--- |
>> | Yes | No | N/A | Inner | Inner | _N/A_ | Simple inner case |
>> | No | Yes | No | Outer | _N/A_ | Outer | Simple outer case |
>> | No | Yes | Yes | Outer | _N/A_ | Outer | `x` must be private | |
>> | Yes | Yes | No | Inner | Inner | Outer | |
>> | Yes | Yes | Yes | Inner | Inner | Outer | |
>> 
>> So it looks like we can say this about an (presumably valid) expression of the form `N.this.x`:
>> * If `N` = `C`, then we are always referring to the "inner" `x` - therefore, it's an early reference to `C` if and only if we are in an early construction context for `C`
>> * If `N` ≠ `C`, then we are always referring to the "outer" `x` - therefore, an early reference if and only if we are in an early construction context for `N`
>> 
>> So according to that logic, you are correct in saying, with regards to expressions like `N.this.x`, that "the only thing that matters is whether `N` is in early construction context".
>> 
>> Please check my work...
>
> Yes, the spec is clear in saying:
> 
>>  A class or interface O is the zeroth lexically enclosing class or interface declaration of itself. 
> 
> And
> 
>> Let n be an integer such that TypeName denotes the n'th lexically enclosing class or interface declaration of the class or interface whose declaration immediately encloses the qualified this expression. The value of a qualified this expression TypeName.this is the n'th lexically enclosing instance of this. 
> 
> So, if we are inside N and we see N.this, we have to assume it's the 0-th enclosing instance we're talking about (e.g. the class being declared).
> 
> Your table looks correct - when you say `x must be private`, I'd add that this is true only for the implicit case where access is just `x`. E.g. if `x` is not inherited, then it's resolved to the Outer, otherwise it's resolved to the class being declared (which might create early construction issues).
> 
> This situation is not dissimilar from the one we have encountered here:
> 
> https://git.openjdk.org/jdk/pull/19904
> 
> Where my initial prototype was "skipping" over early references if non-early enclosing references were available. While in that case it was deemed too subtle (for good reasons), I think in this case we're bound by the existing meaning of `x` -- which depends on whether `x` is, or is not a member of the current class.

OK thanks. In all the discussion I've lost track of what needs to actually change here. Is there a further simplification of `Resolve.isEarlyReference()` possible?

You previously said:
> I guess in terms of code, what this means is that I'm surprised that we're handling the implicit and explicit cases in the same test. E.g. when using a qualified `X.this`, the membership check seems irrelevant (the only thing that matters is whether `X` is in early construction context).

But this method is testing whether the variable reference is an early reference for the current class, not just whatever class it's a member of... so isn't the membership test appropriate?

Having said that, there's probably some refactoring of these checks that would make more sense. `Resolve.isEarlyReference()` is probably being invoked from too many places.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23545#discussion_r1958693623