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

[Jan Lahoda]

On Wed, 8 May 2024 17:58:22 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>> 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))
>
> I agree in principle, however the code base uses `hasTag()` instead of `instanceof` everywhere - presumably for a good (although possibly obsolete) reason - so I'm just being conservative/consistent.
> 
> As a separate discussion, it's worth considering getting rid of 'tags' in a giant wholesale cleanup... maybe someone could write a clever script to make the changes automatically. But first someone would need to confirm that what originally motivated this quirk no longer applies.
> 
> Personally I've always found that `hasTag()` instead of `instanceof` just makes the code harder to read. These days I'd guess it's probably not saving any time either.

We are already using pattern matching `instanceof` for trees, like here:
https://github.com/openjdk/jdk/blob/ad78b7fa67ba30cab2e8f496e4c765be15deeca6/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java#L1215
https://github.com/openjdk/jdk/blob/ad78b7fa67ba30cab2e8f496e4c765be15deeca6/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java#L1517

While I am not a bit fan of doing whole code-base rewrite to introduce pattern matching (even though technically we could do that), mostly because that obfuscates the history, and makes backports more difficult, I don't see a big reason to not use the feature for new code.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18088#discussion_r1594441635