known translation issues related to JEP 482

[Archie Cobbs]

Another nice cleanup ... RAR = 2.29!!

Combining this patch with your first patch and the other PR's already
submitted fixes all the test cases I've been playing with.

Your change to the semantics of hasOuterInstance() happens to invalidate my fix
for JDK-8334248
<https://github.com/openjdk/jdk/pull/19705/files#diff-ce037c979a568d769d3064d15f94a75cd0311e3e0a9e7899f59d4f9283ad6ea8>
but that's OK, the new semantics are more correct and I've rewritten my fix
to instead check for NOOUTERTHIS directly.

-Archie

On Fri, Jun 14, 2024 at 9:53 AM Maurizio Cimadamore <
maurizio.cimadamore at oracle.com> wrote:

> Ok.
>
> This is an attempt I made to fix the enclosing instance issue. It sits on
> top of the patch I shared yestreday, but probably could be integrated
> independently:
>
>
> https://github.com/mcimadamore/jdk/compare/cleanup_capture...mcimadamore:jdk:cleanup_capture+encl_fix?expand=1
>
> The core of the fix is the addition of a new function in `Symbol`, namely
> `Symbol::innermostAccessibleEnclosingClass`. As the name implies, the goal
> of this function is to return the class symbol corresponding to the
> innermost enclosing type that is accessible from the symbol's class.
>
> This is used by `Lower` to determine the type of `this$0`, which allows us
> to construct a chain of enclosing instances that skips over all the
> inaccessible types.
>
> I've also updated the `Symbol::hasOuterInstance` method, so that it, too,
> will skip over inaccessible enclosing type (and only return `true` if some
> accessible enclosing type is found).
>
> Again, the cleanup is rather nice. Note how much simpler
> `Lower.FreeVarCollector` now is, as we no longer have to do heroics to
> treat enclosing instances as captured variables (or *freevars* using
> `Lower`'s lingo). As a result, `FreeVarCollector` is now effectively
> stateless - it no longer depends on the contents of `outerThisStack`. As
> such, it is now easier for clients to predict which symbols will be
> captured by a given local class, as this logic now is effectively
> independent from `Lower` (!!).
>
> This seems to solve issues like:
> https://bugs.openjdk.org/browse/JDK-8334121
>
> And, together with the patch I shared yesterday, should put a lid on the
> myriad of translation bugs associated with local classes/lambdas in
> pre-construction contexts.
>
> Cheers
> Maurizio
> On 14/06/2024 13:57, Archie Cobbs wrote:
>
> I'm done with my hacking for now (some of which your changes subsume e.g.
> JDK-8334252) - and I'm sure I'll want to study your improvements for a
> while before trying anything new.
>
> Thanks!
>
> On Thu, Jun 13, 2024 at 3:34 PM Maurizio Cimadamore <
> maurizio.cimadamore at oracle.com> wrote:
>
>> Heh. Thanks :-)
>>
>> So, I think this patch can be used as a basis for further hacking in
>> Lower (e.g. to deal with inaccessible enclosing instances the right way).
>>
>> You seemed to have put some thought into how to do it, so if you want to
>> go ahead that's fine by me (if not, that's fine too, but let's not
>> duplicate work).
>>
>> Cheers
>> Maurizio
>>
>>
>> On 13/06/2024 19:36, Archie Cobbs wrote:
>>
>> Refactoring Awesomeness Ratio (RAR) = # lines removed / # lines added.
>>
>> Congrats on an RAR of 1.59 :)
>>
>> -Archie
>>
>> On Thu, Jun 13, 2024 at 12:05 PM Maurizio Cimadamore <
>> maurizio.cimadamore at oracle.com> wrote:
>>
>>> Here's a first draft of the work to move LambdaToMethod after Lower:
>>>
>>
>> --
>> Archie L. Cobbs
>>
>>
>
> --
> Archie L. Cobbs
>
>

-- 
Archie L. Cobbs
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-dev/attachments/20240614/b8432311/attachment-0001.htm>