known translation issues related to JEP 482

[Maurizio Cimadamore]

Hi,
I have put together a patch that overhauls the attribution checks for 
local and member inner class creation:

https://github.com/openjdk/jdk/compare/master...mcimadamore:jdk:refactor_encl_checks?expand=1

This was far more complex than I expected, as I faced issues not only in 
the implementation, but also in the JLS (see below). Let me attempt to 
summarize the rationale behind this fix.

Let’s start with member inner classes. Whenever we see |new M()| where 
|M| is a member inner class, we need to /infer/ an expression for |M|‘s 
enclosing instance, given none is provided. This inference problem is 
also present when checking a |super(...)| constructor call: if the 
superclass is a member inner class |M|, then validating the constructor 
call implies /inferring/ a suitable enclosing instance for |M|, as if we 
were checking |new M()|.

How should this inference process work? Well, it should look at /all/ 
the enclosing instances available to us, and pick the innermost 
enclosing instance of type |C| such that:

  * |C.this| is accessible (e.g. not in |C|‘s early construction
    context) and;
  * |C| is a subclass of |M|‘s enclosing class.

The crucial observation here is that there can be /multiple/ valid 
enclosing instances, and the innermost one might not be available due to 
early construction context, so we need to be able to skip that, and jump 
to the next. Consider this example:

|class Outer { class InnerSuperclass { } static class InnerOuter extends 
Outer { class InnerInnerOuter extends Outer { InnerInnerOuter() { class 
InnerSubclass extends InnerSuperclass { InnerSubclass() { super(); // 
(1) } } super(); } } } } |

This program doesn’t compile today. But should it? Yes: while it’s true 
that |InnerInnerOuter.this| cannot be accessed in (1), |InnerOuter.this| 
can, and that’s also a suitable enclosing instance (as |InnerOuter 
extends Outer|).

What about local classes? Should they undergo a similar check? The 
answer is no, as 15.9.2 is silent about this. What matters, for local 
classes, is that the local class creation expression occurs in a context 
where we can access local variables defined in the method in which the 
local class is defined.

With the new checks in place we can finally solve both:

  * https://bugs.openjdk.org/browse/JDK-8334248 and
  * https://bugs.openjdk.org/browse/JDK-8322882

Few notes:

  * |Lower| needs some updates to support programs like the the one
    above. That is, we have to teach |Lower| that the current class’
    |this| cannot be touched if the class is in early construction context;
  * the JLS text for member inner class creation seems lacking (in both
    15.9.2 and in 8.8.7.1). First, the JLS fails to recognize that a
    /subclass/ of the enclosing instance is still a suitable value for
    constructing the member class. Secondly, the JLS doesn’t seem to
    take early construction contexts into account (so might end up
    picking the wrong thing, or reporting an unnecessary error);
  * the JLS text for local inner class creation in 15.9.2 seems also
    lacking. The check for local classes seems too weak, which leaves
    out intersting cases such as JDK-8322882.

I think that this fix, coupled with the translation fixes I shared last 
week should implement JEP 482 “as intended” (of course, for some of the 
fixes described here, some JLS udpate is also required, as these are not 
“just” compiler bugs).

Cheers
Maurizio

On 14/06/2024 20:49, Archie Cobbs wrote:

    Thanks. Let’s transition this particular discussion to that PR. I
    added a rebuttal for you :)

    https://github.com/openjdk/jdk/pull/19705
    https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19705__;!!ACWV5N9M2RV99hQ!IPTJtMcgd06MLFxVOwPycDr63fpHIABTrno8GoNywuev7W-GybSntWK-JnxR6AxfqkAm2yp8w79BYUoeki12fBoD2It17g$

    -Archie

    On Fri, Jun 14, 2024 at 12:56 PM Maurizio Cimadamore

    maurizio.cimadamore at oracle.com
    <http://mailto:maurizio.cimadamore@oracle.com> wrote:

    On 14/06/2024 18:51, Archie Cobbs wrote:

        So yes I also see the compiler crashing on that example but I
        think that may be separate from the issue above…? I don’t fully
        understand this one yet.

    I left an example of code that might defeat your fix in the PR.

    https://github.com/openjdk/jdk/pull/19705#issuecomment-2168497505
    https://urldefense.com/v3/__https://github.com/openjdk/jdk/pull/19705*issuecomment-2168497505__;Iw!!ACWV5N9M2RV99hQ!IPTJtMcgd06MLFxVOwPycDr63fpHIABTrno8GoNywuev7W-GybSntWK-JnxR6AxfqkAm2yp8w79BYUoeki12fBpkHh3GXQ$

    Maurizio

    — Archie L. Cobbs

​
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-dev/attachments/20240620/ee281a90/attachment-0001.htm>