On Sat, 30 May 2026 00:19:23 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:
> 
>   Deal with references to lowered fields from early field initializers

Alternatives considered:
* move `normalizeMethod` earlier. On paper this sounds clean, but it's not so 
straightforward. If the initializer contains nested classes or lambdas there's 
a real risk to generate redundant artifacts, as e.g. Lower might attempt to 
generate the same inner class from different constructors
* Mark access to `this$0` and friends from early initializers as "early 
access". In principle this works but (a) we need to find some way so that an 
early access in a field initializer is an early access _in all constructors_ at 
once (but we already have something like that in `Attr`/`Resolve`). And, we 
need to be mindful of the order in which things appear in the normalized 
constructor -- if early initializers are added at the beginning of the 
constructors, no amount of local proxy patching can save us.

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

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

Reply via email to