RFR: 8334248: Invalid error for early construction local class constructor method reference [v3]

[Maurizio Cimadamore]

On Sat, 15 Jun 2024 15:28:40 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> The new "flexible constructors" JEP 482 specifies that a local class declared in an early construction context does not have an immediate outer instance, i.e., it has the same behavior as an anonymous class would in that situation.
>> 
>> However, the compiler still (incorrectly) tries to verify that an outer instance exists in the case of a method reference to the local class' constructor.
>> 
>> Example:
>> 
>> 
>> 
>> import java.util.function.Supplier;
>> 
>> class EarlyLocalCtorRef {
>> 
>>     EarlyLocalCtorRef() {
>>         class InnerLocal { }
>>         this(InnerLocal::new);
>>     }
>> 
>>     EarlyLocalCtorRef(Supplier<Object> s) {
>>     }
>> }
>> 
>> 
>> Expected output: Successful compilation.
>> 
>> Actual output:
>> 
>> 
>> EarlyLocalCtorRef.java:5: error: cannot reference this before supertype constructor has been called
>>         this(InnerLocal::new);
>>              ^ 
>> 
>> 
>> The fix is to not look for an implicit outer instance for classes that don't have one when encountering a constructor method reference.
>> 
>> **NOTE:** The test added here will fail until [JDK-8333313](https://bugs.openjdk.org/browse/JDK-8333313) is fixed.
>
> Archie Cobbs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Refactor resolveImplicitThis() to respect actual outer instance.
>  - Add Maurizio's innermostAccessibleEnclosingClass() and hasOuterInstance().
>  - Merge branch 'master' into JDK-8334248
>  - Future-proof bug fix vs. new & improved semantics of hasOuterInstance().
>  - Don't require outer instance for early construction local class method ref.

The patch seems to be doing something sensible (e.g. take into account that not all enclosing instances are accessible). However, as I mentioned last week, I believe we need to wait for the spec to weigh in on this matter. The checks we're tweaking correspond to this statement in the JLS (15.9.2):


 If C is an inner local class, then:
 ...
Otherwise, let O be the immediately enclosing class or interface declaration of C, and let U be the immediately enclosing class or interface declaration of the class instance creation expression. 
 If U is not an inner class of O or O itself, then a compile-time error occurs. 
 ```
 
 Now, if C is a local class, is there ever a case where "new C" can appear from _outside_ the lexical context in which C is defined? I don't think that's possible (Dan and Gavin seems to agree). If that's the case, then the only thing we need to check for local/anon classes is that the creation expression does not occur in a static context. This is already covered by the existing rules:


If C occurs in a static context, then i has no immediately enclosing instance.

Otherwise, if the class instance creation expression occurs in a static context, then a compile-time error occurs.


It seems to me that these rules override any other rule when it comes to local/anon classes. If so, maybe the code in `visitNew` and `visitReference` should just add a static check when the target class is a local/anon class, and leave `resolveImplicitThis` alone.

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

PR Comment: https://git.openjdk.org/jdk/pull/19705#issuecomment-2172854828