From 2021d168b2a19baf7a6e454e71bf5db68f9b86ed Mon Sep 17 00:00:00 2001 From: Jan Lahoda Date: Fri, 6 Oct 2023 15:24:02 +0200 Subject: [PATCH] Attempting to improve reported errors. --- .../com/sun/tools/javac/comp/Attr.java | 75 ++++++------------- .../com/sun/tools/javac/comp/Flow.java | 27 ++++--- .../com/sun/tools/javac/comp/Lower.java | 1 - .../sun/tools/javac/comp/TransPatterns.java | 7 +- .../javac/patterns/CastConversionMatch.out | 2 +- .../patterns/DeconstructionPatternErrors.java | 2 + 6 files changed, 45 insertions(+), 69 deletions(-) diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java index cc656ce7511e9..6c9cb1155d34f 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java @@ -172,8 +172,6 @@ protected Attr(Context context) { Feature.PATTERN_SWITCH.allowedInSource(source); allowUnconditionalPatternsInstanceOf = Feature.UNCONDITIONAL_PATTERN_IN_INSTANCEOF.allowedInSource(source); - allowPrimitivePatterns = (preview.isEnabled() || !preview.isPreview(Feature.PRIMITIVE_PATTERNS)) && - Feature.PRIMITIVE_PATTERNS.allowedInSource(source); sourceName = source.name; useBeforeDeclarationWarning = options.isSet("useBeforeDeclarationWarning"); @@ -202,10 +200,6 @@ protected Attr(Context context) { */ private final boolean allowUnconditionalPatternsInstanceOf; - /** Are primitive patterns in instanceof allowed - */ - private final boolean allowPrimitivePatterns; - /** * Switch: warn about use of variable before declaration? * RFE: 6425594 @@ -1687,18 +1681,17 @@ private void handleSwitch(JCTree switchTree, try { boolean enumSwitch = (seltype.tsym.flags() & Flags.ENUM) != 0; boolean stringSwitch = types.isSameType(seltype, syms.stringType); - boolean booleanSwitch = types.isAssignable(seltype, syms.booleanType); boolean errorEnumSwitch = TreeInfo.isErrorEnumSwitch(selector, cases); boolean intSwitch = types.isAssignable(seltype, syms.intType); - boolean errorPrimitiveSwitch = seltype.isPrimitive() && !intSwitch && !allowPrimitivePatterns; boolean patternSwitch; if (!enumSwitch && !stringSwitch && !errorEnumSwitch && - !intSwitch && !errorPrimitiveSwitch) { + !intSwitch) { preview.checkSourceLevel(selector.pos(), Feature.PATTERN_SWITCH); patternSwitch = true; } else { - if (errorPrimitiveSwitch) { - log.error(selector.pos(), Errors.SelectorTypeNotAllowed(seltype)); + if (seltype.isPrimitive() && !intSwitch) { + preview.checkSourceLevel(selector.pos(), Feature.PRIMITIVE_PATTERNS); + patternSwitch = true; } patternSwitch = cases.stream() .flatMap(c -> c.labels.stream()) @@ -1784,21 +1777,9 @@ private void handleSwitch(JCTree switchTree, log.error(label.pos(), Errors.DuplicateCaseLabel); } } - else if (allowPrimitivePatterns) { - if (!stringSwitch && !intSwitch && !booleanSwitch && - !(types.isSameType(pattype, syms.longType) && types.isAssignable(seltype, syms.longType)) && - !(types.isSameType(pattype, syms.floatType) && types.isAssignable(seltype, syms.floatType)) && - !(types.isSameType(pattype, syms.doubleType) && types.isAssignable(seltype, syms.doubleType))) { - log.error(label.pos(), Errors.ConstantLabelNotCompatible(pattype, seltype)); - } else if (booleanSwitch && - !(types.isAssignable(pattype, syms.booleanType))) { - log.error(label.pos(), Errors.ConstantLabelNotCompatible(pattype, seltype)); - } else if (!constants.add(pattype.constValue())) { - log.error(c.pos(), Errors.DuplicateCaseLabel); - } - } else { - if (!stringSwitch && !intSwitch && !errorPrimitiveSwitch) { + if (seltype.isPrimitive() && !intSwitch && + !types.isAssignable(seltype, pattype)) { log.error(label.pos(), Errors.ConstantLabelNotCompatible(pattype, seltype)); } else if (!constants.add(pattype.constValue())) { log.error(c.pos(), Errors.DuplicateCaseLabel); @@ -1820,14 +1801,12 @@ else if (allowPrimitivePatterns) { attribExpr(pat, switchEnv, seltype); Type primaryType = TreeInfo.primaryPatternType(pat); - if (!primaryType.hasTag(TYPEVAR) && !(primaryType.isPrimitive() && allowPrimitivePatterns)) { + if (primaryType.isPrimitive()) { + preview.checkSourceLevel(pat.pos(), Feature.PRIMITIVE_PATTERNS); + } else if (!primaryType.hasTag(TYPEVAR)) { primaryType = chk.checkClassOrArrayType(pat.pos(), primaryType); - } else if (primaryType.isPrimitive() && allowPrimitivePatterns) { - preview.warnPreview(pat.pos(), Feature.PRIMITIVE_PATTERNS); - } - if (!errorPrimitiveSwitch) { - checkCastablePattern(pat.pos(), seltype, primaryType); } + checkCastablePattern(pat.pos(), seltype, primaryType); Type patternType = types.erasure(primaryType); JCExpression guard = c.guard; if (guardBindings == null && guard != null) { @@ -4151,11 +4130,11 @@ public boolean compatible(Type found, Type req, Warner warn) { public void visitTypeTest(JCInstanceOf tree) { Type exprtype = attribExpr(tree.expr, env); - if (!allowPrimitivePatterns) { + if (exprtype.isPrimitive()) { + preview.checkSourceLevel(tree.expr.pos(), Feature.PRIMITIVE_PATTERNS); + } else { exprtype = chk.checkNullOrRefType( tree.expr.pos(), exprtype); - } else if (tree.pattern.type != null && exprtype.isPrimitive() && preview.isPreview(Feature.PRIMITIVE_PATTERNS)) { - preview.warnPreview(tree.pattern.pos(), Feature.PRIMITIVE_PATTERNS); } Type clazztype; JCTree typeTree; @@ -4175,14 +4154,14 @@ public void visitTypeTest(JCInstanceOf tree) { } else { clazztype = attribType(tree.pattern, env); typeTree = tree.pattern; - if (!clazztype.isPrimitive() || !allowPrimitivePatterns) { - chk.validate(typeTree, env, false); - } - } - if (!clazztype.hasTag(TYPEVAR) && !allowPrimitivePatterns) { - clazztype = chk.checkClassOrArrayType(typeTree.pos(), clazztype); + chk.validate(typeTree, env, false); } - if(!clazztype.isPrimitive() || !allowPrimitivePatterns) { + if (clazztype.isPrimitive()) { + preview.checkSourceLevel(tree.pattern.pos(), Feature.PRIMITIVE_PATTERNS); + } else { + if (!clazztype.hasTag(TYPEVAR)) { + clazztype = chk.checkClassOrArrayType(typeTree.pos(), clazztype); + } if (!clazztype.isErroneous() && !types.isReifiable(clazztype)) { boolean valid = false; if (allowReifiableTypesInInstanceof) { @@ -4196,11 +4175,8 @@ public void visitTypeTest(JCInstanceOf tree) { clazztype = types.createErrorType(clazztype); } } - } else if (clazztype.isPrimitive() && preview.isPreview(Feature.PRIMITIVE_PATTERNS)) { - preview.warnPreview(tree.pattern.pos(), Feature.PRIMITIVE_PATTERNS); } chk.checkCastable(tree.expr.pos(), exprtype, clazztype); - result = check(tree, syms.booleanType, KindSelector.VAL, resultInfo); } @@ -4218,14 +4194,9 @@ private boolean checkCastablePattern(DiagnosticPosition pos, return false; } else if ((exprType.isPrimitive() || pattType.isPrimitive()) && (!exprType.isPrimitive() || !pattType.isPrimitive() || !types.isSameType(exprType, pattType))) { - if (!allowPrimitivePatterns) { - chk.basicHandler.report(pos, - diags.fragment(Fragments.NotApplicableTypes(exprType, pattType))); - return false; - } else { - preview.warnPreview(pos, Feature.PRIMITIVE_PATTERNS); - return true; - } + //TODO: double check + preview.checkSourceLevel(pos, Feature.PRIMITIVE_PATTERNS); + return false; } else if (warner.hasLint(LintCategory.UNCHECKED)) { log.error(pos, Errors.InstanceofReifiableNotSafe(exprType, pattType)); diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java index 785a78b5ae40f..5657d6bef6dda 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java @@ -698,7 +698,15 @@ public void visitSwitch(JCSwitch tree) { tree.isExhaustive = tree.hasUnconditionalPattern || TreeInfo.isErrorEnumSwitch(tree.selector, tree.cases); if (exhaustiveSwitch) { - tree.isExhaustive |= exhausts(tree.selector, tree.cases); + if (tree.selector.type.hasTag(TypeTag.BOOLEAN)) { + if (tree.hasUnconditionalPattern && + exhausts(tree.selector, tree.cases)) { + log.error(tree, Errors.DefaultLabelNotAllowed); + } + tree.isExhaustive = true; + } else { + tree.isExhaustive |= exhausts(tree.selector, tree.cases); + } if (!tree.isExhaustive) { log.error(tree, Errors.NotExhaustiveStatement); } @@ -733,16 +741,17 @@ public void visitSwitchExpression(JCSwitchExpression tree) { } } - boolean exhaustive = exhausts(tree.selector, tree.cases); - - if (tree.selector.type.hasTag(TypeTag.BOOLEAN) && exhaustive && tree.hasUnconditionalPattern) { - log.error(tree, Errors.DefaultLabelNotAllowed); + if (tree.hasUnconditionalPattern || + TreeInfo.isErrorEnumSwitch(tree.selector, tree.cases)) { + if (tree.selector.type.hasTag(TypeTag.BOOLEAN) && + exhausts(tree.selector, tree.cases)) { + log.error(tree, Errors.DefaultLabelNotAllowed); + } + tree.isExhaustive = true; + } else { + tree.isExhaustive = exhausts(tree.selector, tree.cases); } - tree.isExhaustive = tree.hasUnconditionalPattern || - TreeInfo.isErrorEnumSwitch(tree.selector, tree.cases) || - exhaustive; - if (!tree.isExhaustive) { log.error(tree, Errors.NotExhaustive); } diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java index 995d81f60a914..ba357b9cdefb6 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java @@ -25,7 +25,6 @@ package com.sun.tools.javac.comp; -import java.lang.reflect.Method; import java.util.*; import java.util.stream.Collectors; diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java index 6a754221daec1..6761010b10d12 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java @@ -65,7 +65,6 @@ import com.sun.tools.javac.util.ListBuffer; import com.sun.tools.javac.util.Name; import com.sun.tools.javac.util.Names; -import com.sun.tools.javac.util.JCDiagnostic.DiagnosticPosition; import java.util.Collections; import java.util.Map; @@ -73,7 +72,6 @@ import java.util.HashMap; import java.util.IdentityHashMap; import java.util.LinkedHashMap; -import java.util.stream.Stream; import java.util.Set; import com.sun.tools.javac.code.Symbol.MethodSymbol; @@ -990,10 +988,7 @@ public void resolve(VarSymbol commonBinding, } private Type principalType(JCTree p) { - if (p instanceof JCPattern jcp && jcp.type.isPrimitive()) { - return jcp.type; - } - return types.boxedTypeOrType(types.erasure(TreeInfo.primaryPatternType(p))); + return types.erasure(TreeInfo.primaryPatternType(p)); } private LoadableConstant toLoadableConstant(JCCaseLabel l, Type selector) { diff --git a/test/langtools/tools/javac/patterns/CastConversionMatch.out b/test/langtools/tools/javac/patterns/CastConversionMatch.out index 8f747810aff25..93385942d8b86 100644 --- a/test/langtools/tools/javac/patterns/CastConversionMatch.out +++ b/test/langtools/tools/javac/patterns/CastConversionMatch.out @@ -1,2 +1,2 @@ -CastConversionMatch.java:11:26: compiler.err.type.found.req: int, (compiler.misc.type.req.class.array) +CastConversionMatch.java:11:26: compiler.err.preview.feature.disabled.plural: (compiler.misc.feature.primitive.patterns) 1 error diff --git a/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.java b/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.java index b7671a86f44ef..d3c419194ca17 100644 --- a/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.java +++ b/test/langtools/tools/javac/patterns/DeconstructionPatternErrors.java @@ -28,12 +28,14 @@ public static void meth() throws Throwable { if (p instanceof P2(var v, var v)); //duplicated variables if (p instanceof P6(P2(var v1, var v2), P2(var v1, var v2))); //duplicated variables if (p instanceof P7(byte b)); //incorrect pattern type +//TODO: either remove (probably OK), or re-write to one testcase per compilation: if (p instanceof P7(long l)); //incorrect pattern type switch (p) { case P7(byte b) -> {} //incorrect pattern type - no exception should occur case P7(long l) -> {} //incorrect pattern type - no exception should occur default -> {} } +//till here GenRecord r1 = null; if (r1 instanceof GenRecord(String s)) {} switch (r1) {