<div dir="ltr"><div>Another nice cleanup ... RAR = 2.29!!</div><div><br></div><div>Combining this patch with your first patch and the other PR's already submitted fixes all the test cases I've been playing with.</div><div><br></div><div>Your change to the semantics of <span style="font-family:monospace">hasOuterInstance()</span> happens to invalidate my <a href="https://github.com/openjdk/jdk/pull/19705/files#diff-ce037c979a568d769d3064d15f94a75cd0311e3e0a9e7899f59d4f9283ad6ea8" target="_blank">fix for JDK-8334248</a> but that's OK, the new semantics are more correct and I've rewritten my fix to instead check for NOOUTERTHIS directly.<br></div><div><br></div><div>-Archie<br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jun 14, 2024 at 9:53 AM Maurizio Cimadamore <<a href="mailto:maurizio.cimadamore@oracle.com" target="_blank">maurizio.cimadamore@oracle.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><u></u>
<div>
<p>Ok.</p>
<p>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:</p>
<p><a href="https://github.com/mcimadamore/jdk/compare/cleanup_capture...mcimadamore:jdk:cleanup_capture+encl_fix?expand=1" target="_blank">https://github.com/mcimadamore/jdk/compare/cleanup_capture...mcimadamore:jdk:cleanup_capture+encl_fix?expand=1</a></p>
<p>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.</p>
<p>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.</p>
<p>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).</p>
<p>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`
(!!).</p>
<p>This seems to solve issues like:
<a href="https://bugs.openjdk.org/browse/JDK-8334121" target="_blank">https://bugs.openjdk.org/browse/JDK-8334121</a></p>
<p>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.</p>
<p>Cheers<br>
Maurizio<br>
</p>
<div>On 14/06/2024 13:57, Archie Cobbs
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>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.</div>
<div><br>
</div>
<div>Thanks!<br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Jun 13, 2024 at
3:34 PM Maurizio Cimadamore <<a href="mailto:maurizio.cimadamore@oracle.com" target="_blank">maurizio.cimadamore@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p>Heh. Thanks :-)<br>
</p>
<p>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).</p>
<p>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).</p>
<p>Cheers<br>
Maurizio<br>
</p>
<p><br>
</p>
<div>On 13/06/2024 19:36, Archie Cobbs wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">
<div>Refactoring Awesomeness Ratio (RAR) = # lines
removed / # lines added.</div>
<div><br>
</div>
<div>Congrats on an RAR of 1.59 :)<br>
</div>
<div><br>
</div>
<div>-Archie<br>
</div>
<div><br>
</div>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Jun 13,
2024 at 12:05 PM Maurizio Cimadamore <<a href="mailto:maurizio.cimadamore@oracle.com" target="_blank">maurizio.cimadamore@oracle.com</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>Here's a first draft of the work to move
LambdaToMethod after Lower: </div>
</blockquote>
<div><br>
</div>
</div>
<span class="gmail_signature_prefix">-- </span><br>
<div dir="ltr" class="gmail_signature">Archie L. Cobbs<br>
</div>
</div>
</blockquote>
</div>
</blockquote>
</div>
<br clear="all">
<br>
<span class="gmail_signature_prefix">-- </span><br>
<div dir="ltr" class="gmail_signature">Archie L. Cobbs<br>
</div>
</div>
</blockquote>
</div>
</blockquote></div><br clear="all"><br><span class="gmail_signature_prefix">-- </span><br><div dir="ltr" class="gmail_signature">Archie L. Cobbs<br></div>