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.

So, in the context of this PR, I think two things need to happen:
* we need to fix the `env.ctorPrologue` logic so that errors are correctly reported as part of `Attr` and not `Lower`
* we need to remove dependencies from `NOOUTERTHIS`. The idea is that we only need to call `resolveSelfXYZ` if the class being constructed is a member inner class. For local class just checking if it's static would suffice. This logic should be applied in at least three places:
- `Attr::visitNewClass`
- `Attr::visitReference`
- `Resolve::enclosingInstanceMissing`

Then, optional cleanup activities:

* decide whether to keep the `Resolve::enclosingInstanceMissing` check, or the one in `Attr::visitReference` as they are likely redundant.
* instead of the current fix, where you check for early construction context in the new `Resolve::resolveImplicitThis`, it might make sense to add the check in the loops inside `Resolve::resolveSelf` and `Resolve::resolveSelfContainingInternal`, as both methods are looping over `env` anyway.

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

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