On Tue, 2 Jun 2026 11:27:06 GMT, Maurizio Cimadamore <[email protected]> 
wrote:

>> This PR removes some unnecessary coupling between Lower, Gen and 
>> LocalProxyVarsGen.
>> 
>> It does so by making LocalProxyVarsGen no longer a standalone pass but, 
>> rather, a helper for Gen::normalizeMethod.
>> 
>> The main idea is that we can make things more regular, by having Gen always 
>> inserting variable initializer in the correct place.
>> Then, LocalProxyVarGen will create a blank proxy, and use its visitor to 
>> inspect the rest of the constructor body.
>> Since the visitor _already_ rewires assignments to real fields as 
>> assignments to proxies, this new arrangement has the desired effect of 
>> generating the same code as before, but w/o too much coupling.
>> 
>> Some massaging to `Gen::normalizeMethod` was needed because now we need to 
>> make sure it calls the proxy step for all constructors, not just in case 
>> there's some pending var initializers.
>> 
>> Finally, when cleaning up `Lower` I noticed a likely bug: `freevardefs` was 
>> no longer preserving the `LOCAL_CAPTURE_FIELD` -- sometimes it was replacing 
>> it with `STRICT`. But `LOCAL_CAPTURE_FIELD` is used by LambdaToMethod, so 
>> changing this probably results in bad downstream lowering.
>> I've fixed this by adding both `STRICT` _and_ `LOCAL_CAPTURE_FIELD` to the 
>> captured sym.
>> 
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the 
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> Maurizio Cimadamore has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Tighten code

I've tightened the code to make sure `Gen` doesn't rewrite more than it should:
* I've added a new explicit flag for the`this$0` field (`OUTER_THIS_FIELD`)
* This means that the rewriter can check whether the sym being accessed is 
either `OUTER_THIS_FIELD` or `CAPTURED_LOCAL_FIELD`
* If there's a match we look into `md.sym.extraParams` or 
`md.sym.capturredLocals` for the symbol to replace
* additionally, `Gen::rewriteEarlyInitializersIfNeeded` doesn't do anything if 
the above two lists of synthetic params are empty

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

PR Comment: https://git.openjdk.org/valhalla/pull/2488#issuecomment-4601943796

Reply via email to