On Tue, 3 Oct 2023 08:58:55 GMT, Maurizio Cimadamore <mcimadam...@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.giulie...@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
