On Tue, 28 Oct 2025 12:46:30 GMT, Aggelos Biboudis <[email protected]> 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
