RFR: 8325805: Compiler Implementation for Flexible Constructor Bodies (Second Preview) [v5]

[Jan Lahoda]

On Tue, 30 Apr 2024 03:01:36 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> This patch changes the compiler to allow constructors to make assignments to fields prior to the `super()` invocation, i.e., in the pre-construction context.  This is part of the work associated with [JDK-8325803 - Flexible Constructor Bodies (Second Preview)](https://bugs.openjdk.org/browse/JDK-8325803).
>> 
>> This patch is based the relevant bits from @vicente-romero-oracle's commit [1b99b5cf](https://github.com/openjdk/valhalla/commit/1b99b5cfd9a5d484b9e7bdfc284daad9ad6535bf) to the valhalla repo.
>
> Archie Cobbs has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 20 commits:
> 
>  - Allow declaring and instantiating local classes prior to super().
>  - Merge branch 'master' into JDK-8325805
>  - Merge branch 'master' into JDK-8325805
>  - Disallow assignments to fields with initializers in constructor prologues.
>  - Merge branch 'master' into JDK-8325805
>  - Merge branch 'master' into JDK-8325805
>  - Fix cut & paste of bug ID and summary.
>  - Avoid possible unwanted side effects from method Symbol.flags().
>  - Add a few more test cases.
>  - Fix bug where early assignments in lambda's were being allowed.
>  - ... and 10 more: https://git.openjdk.org/jdk/compare/9b423a85...7e6a6a0d

Overall, looks reasonable to me. Some suggestions inline.

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 3853:

> 3851: 
> 3852:         // The symbol must appear in the LHS of an assignment statement
> 3853:         if (!env.tree.hasTag(ASSIGN))

I think you should be able to use pattern matching, to avoid the manual cast below. Something like:
Suggestion:

        if (!(env.tree instanceof JCAssign at))

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Resolve.java line 3865:

> 3863: 
> 3864:         // The symbol must not be an instance field inherited from a superclass
> 3865:         if (v.owner.kind == TYP &&

The conditions here are eerily similar to those in `checkAssignable`. I wonder if there's a way to unify the codepaths, so that we only have these conditions on one place.

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

PR Review: https://git.openjdk.org/jdk/pull/18088#pullrequestreview-2044995826
PR Review Comment: https://git.openjdk.org/jdk/pull/18088#discussion_r1593636566
PR Review Comment: https://git.openjdk.org/jdk/pull/18088#discussion_r1593634900