[23] JEP 455: Primitive Types in Patterns, instanceof, and switch (Preview)

[mpalat]

ref: https://openjdk.org/jeps/455

Summary Enhance pattern matching by allowing primitive type patterns in all pattern contexts, and extend instanceof and switch to work with all primitive types.

spec delta: https://cr.openjdk.org/~abimpoudis/instanceof/latest/

[srikanth-sankaran]

Please include me in all design and implementation discussions and reviews.

As a first step, I suggest we figure out if parser/scanner changes are required by capturing all snippets from the JEP document into test cases - do we have any parse failures if we do that ?

[jarthana]

As a first step, I suggest we figure out if parser/scanner changes are required by capturing all snippets from the JEP document into test cases - do we have any parse failures if we do that ?

Is there an easy/standard way to do that or are you saying we just look for syntax errors?

[srikanth-sankaran]

As a first step, I suggest we figure out if parser/scanner changes are required by capturing all snippets from the JEP document into test cases - do we have any parse failures if we do that ?

Is there an easy/standard way to do that or are you saying we just look for syntax errors?

We can see what errors we get, or better yet whether diagnose parse gets entered while compiling the snippets.

[stephan-herrmann]

Please include me in all design and implementation discussions and reviews.

... and include me on all spec issues :)

[srikanth-sankaran]

When this is scheduled for standardization I will assume ownership - for now, please feel free to engage me for any design or implementation consultation and code review as needed. Thanks

[stephan-herrmann]

When this is scheduled for standardization I will assume ownership

by "standardization" you mean having a stable spec, or maturing from preview to standard?

[srikanth-sankaran]

When this is scheduled for standardization I will assume ownership

by "standardization" you mean having a stable spec, or maturing from preview to standard?

I meant the latter.

[mpalat]

@stephan-herrmann - Would you be able to take this up (partly/completely rest of it?) - I am off for the next few days on an important personal work and cannot spend time on work until I am back [Aug 5th] and may have to take a few days later too. In case you are able to spare time to work on this next week, it would be of great help- Please feel free to take it up partly/completely depending on your availability. [@srikanth-sankaran - open to you as well but since you are out I am not sure about your availability]. Thanks.

[mpalat]

Additional issues:

• [ ] https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2868
• [ ] https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2869

[stephan-herrmann]

Some - perhaps academic - observations:

• for implementing patterns like case float f -> javac uses SwitchBootstraps.typeSwitch()
  • that method has changes specifically for JEP 455 (previously it would work only for a very limited set of types).
  • javadoc only mentions that IllegalArgumentException is no longer thrown for types Long, Float etc.
  • the main javadoc text explains no such changes
  • those changes can be traced back to https://bugs.openjdk.org/browse/JDK-8303374
  • when I follow the link to the corresponding commit in openjdk, the method body looks different than what I have in the JDK 23 release candidate, ups? looks like I got lost in github navigation, status in master and jdk23 branches corresponds to the release candidate alright.
• the same commit also introduced java.lang.runtime.ExactConversionsSupport
  • we already use this class since 988d0b8c, @mpalat how did you find out about that class, has it been mentioned in some document or post on the mailing list?

Let's hope that such issues will clear up when the feature leaves preview!

[mpalat]

Some - perhaps academic - observations:

• we already use this class since 988d0b8, @mpalat how did you find out about that class, has it been mentioned in some document or post on the mailing list?

@stephan-herrmann initially I tried implementing this by myself in the generated code which met with limited success. And I should confess that on a comparative check with javac generated code I saw this method and hence decided to use the same since it is already available at runtime.

[stephan-herrmann]

Here's a first attempt of a mapping between spec changes and implementation already done:

• §5.7 conversions: the part concerning primitives is represented by PrimitiveConversionRoute added in #2499 Details see next comment :heavy_check_mark:
• §5.7.1 test for exact conversion implemented in CodeStream.invokeExactConversionsSupport() using ExactConversionsSupport (#2499) :heavy_check_mark:
  • interestingly this implementation doesn't look as sophisticated as required by the spec, I see none of the required projections to BigDecimal, but since this is what javac uses, too, so be it.
• §5.7.2 unconditionally exact conversions are detected by BaseTypeBinding.isExactWidening() (#2499)
  • remaining cases covered by #2869 :heavy_check_mark:
• §14.11.1 newly allowed switch types (long,float,double,boolean + their boxes) supported since(#2866) :heavy_check_mark:
• §14.11.1.1 checking exhaustiveness -> #2869 :heavy_check_mark:
• §14.11.1.2 runtime evaluation is largely delegated to SwitchBootstraps.typeSwitch() (since JDK 17) usage expanded in #2866 :heavy_check_mark:

[stephan-herrmann]

Let's check for completeness of individual parts:

§5.7 conversions: the part concerning primitives is represented by PrimitiveConversionRoute added in https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2298

Enum PrimitiveConversionRoute :heavy_check_mark: Covers all conversions relating to primitives

Computation in Pattern.findPrimitiveConversionRoute() :heavy_check_mark: Had 3 TODOs, which I'm fleshing out in my current PR #2878. Note that "a widening reference conversion followed by an unboxing conversion" is more an academic thing, see https://mail.openjdk.org/pipermail/amber-spec-experts/2024-August/004204.html and answers by Brian and Rémi.

Code gen for all conversion routes :heavy_check_mark:

• all handled in TypePattern.generateTestingConversion()
• all handeld in InstanceOfExpression.generateTypeCheck() since 7f70a8577247e8095fe83377f0f5c1d18c20d8d2

Test coverage :heavy_check_mark: Findings from running all compiler regression tests at compliance 23 with coverage enabled:

• I inspected the following classes and found no relevant not-covered code sections among those touched for JEP 455
  • Pattern, TypePattern, CastStatement, SwitchStatement, InstanceOfExpression, ClassFile
• One code section in InstanceOfExpression.checkRefForPrimitivesAndAddSecretVariable() turned out to be dead code: called only when the destination type is non-reifiable, but then wants that destination type to be primitive, which can't be.

[stephan-herrmann]

While working on all those type tests and conversions, I repeatedly got entangled in confusing terminology, particularly where the type of a switch selector was named the "expectedType" for each case statement. One such confusion actually surfaced in bogus error messages like this:

        final short s = 100;
        Integer i1 = ...;
        switch (i1)  {
            case s:
        }

ecj reported:

    case s:
         ^
Type mismatch: cannot convert from short to Integer

Nobody has any business converting any short value to Integer. An Integer value is provided and to be matched against (not even converted to) a short value. Taking inspiration from javac I'm correcting the error to

Case constant of type short is incompatible with switch selector type Integer

[stephan-herrmann]

Much has been argued in this issue and in #2869.

At this point I've run out of obvious things to implement or fix, so let's call victory for now - until playing with the feature will reveal shortcomings, but those will live in their own issues.

[mpalat]

Much has been argued in this issue and in #2869.

At this point I've run out of obvious things to implement or fix, so let's call victory for now - until playing with the feature will reveal shortcomings, but those will live in their own issues.

Thanks a lot @stephan-herrmann for stepping in for this issue and also for the exceptionally fast code implementations. [maybe I should fall sick more often :)]. Going through the code and the comments - In case of any (odd) cases found, will raise individual issues.

[stephan-herrmann]

Mentioning #2891 for the story of nested patterns (primitive type pattern nested in record pattern).