RFR: 8359145: Implement JEP 530: Primitive Types in Patterns, instanceof, and switch (Fourth Preview) [v2]

[Jan Lahoda]

On Tue, 28 Oct 2025 12:46:30 GMT, Aggelos Biboudis <abimpoudis at openjdk.org> wrote:

>> PR for Primitive Types in Patterns, instanceof, and switch (Fourth Preview).
>> 
>> spec: https://cr.openjdk.org/~abimpoudis/instanceof/latest/
>
> Aggelos Biboudis has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision:
> 
>   8359145: JEP 530: Implement Primitive Types in Patterns, instanceof, and switch (Fourth Preview)

I think the direction is good, there are some comments inline.

src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java line 5099:

> 5097:         }
> 5098:     }
> 5099:     // where

Nit:
Suggestion:

src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java line 4743:

> 4741:                     boolean dominated = false;
> 4742: 
> 4743:                     if (unconditionalCaseLabel == testCaseLabel) unconditionalFound = true;

I think the code here is correct, but it is very hard to read (to me, at least). I wonder if there's a chance to try to tweak the code so that it is more readable.

At least: do `{}` around `unconditionalFound`, and move the `dominatedCandidate` into the `if (!currentType.hasTag(ERROR) && !testType.hasTag(ERROR)) {`. I suspect it might be possible to replace `dominatedCandidate` with `continue` in `if (currentType.constValue() instanceof Number) {` (would need to `if (allowPrimitivePatterns && unconditionalFound && unconditionalCaseLabel != label) {` check upfront, so not sure if that would improve things).

test/langtools/tools/javac/patterns/PrimitiveUnconditionallyExactInAssignability.java line 93:

> 91:     }
> 92:     void assertOriginalAssignmentNarrowingAndUnconditionality() {
> 93:         // byte b = <constant short> vs ExactConversionsSupport::isIntToByteExact

If I understand it correctly, the updated and original isAssignable should give the same results, right? I think it would be better to have one invocation, like `assertOriginaAndUpdatedAssignable`, that would check that for the given input, both methods return the same value, and the expected value. Having to lists of cases is likely to lead to omissions/mistakes, I suspect.

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

Marked as reviewed by jlahoda (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/27637#pullrequestreview-3393466239
PR Review Comment: https://git.openjdk.org/jdk/pull/27637#discussion_r2473119289
PR Review Comment: https://git.openjdk.org/jdk/pull/27637#discussion_r2473182215
PR Review Comment: https://git.openjdk.org/jdk/pull/27637#discussion_r2473117111