RFR: 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview) [v3]

[Maurizio Cimadamore]

On Tue, 3 Oct 2023 08:58:55 GMT, Maurizio Cimadamore <mcimadamore at openjdk.org> wrote:

>> Aggelos Biboudis has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - Merge branch 'master' into primitive-patterns
>>  - Implement type pairs to exactnessMethod name
>>  - Apply suggestions from code review
>>    
>>    Co-authored-by: Raffaello Giulietti <raffaello.giulietti at oracle.com>
>>  - 8303374: Compiler Implementation for Primitive types in patterns, instanceof, and switch (Preview)
>
> src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java line 2982:
> 
>> 2980:                     // =>
>> 2981:                     // if (let tmp$123 = v; ExactnessChecks.int_float(tmp$123))
>> 2982:                     JCIdent argument = make.Ident(dollar_s);
> 
> In principle you don't need a synthetic variable here... (but it's ok if you prefer to leave for uniformity)

Aren't there cases here where the type of the expr is unconditional w.r.t. the type of the pattern (and so, no test is required) ? Perhaps the case I'm thinking of is dealt with one of the ifs at the beginning of this method.

I have to say I'm a bit confused by how this method is arranged. From a logical perspective I would expect first and foremost to test if the pattern type is a primitive, because that's the only case where Lower needs to do anything (right?).

Then you can have two cases: the expression has a reference type, in which case you need null check and unboxing. Or the expression has a primitive type.

In both cases you can be in a situation where the conversion is provably exact, so no need to call the exactness routine - or you can be in an inexact situation where a runtime test is required.

While I'm sure that is what happens in the above code, I think perhaps the various tests could be arranged in a way to make the above structure pop out a little more?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15638#discussion_r1343767531