Skip to content

Commit

Permalink
Attempting to improve reported errors.
Browse files Browse the repository at this point in the history
  • Loading branch information
@lahodaj
lahodaj committed Oct 6, 2023
1 parent ff07aef commit 2021d16
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 69 deletions.
75 changes: 23 additions & 52 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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);
Expand All @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand All @@ -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);
}

Expand All @@ -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));
Expand Down
27 changes: 18 additions & 9 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand Down
1 change: 0 additions & 1 deletion src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Lower.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

package com.sun.tools.javac.comp;

import java.lang.reflect.Method;
import java.util.*;
import java.util.stream.Collectors;

Expand Down
7 changes: 1 addition & 6 deletions src/jdk.compiler/share/classes/com/sun/tools/javac/comp/TransPatterns.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,13 @@
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;
import java.util.Map.Entry;
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;
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion test/langtools/tools/javac/patterns/CastConversionMatch.out
View file
Original file line number Diff line number Diff line change
@@ -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
2 changes: 2 additions & 0 deletions test/langtools/tools/javac/patterns/DeconstructionPatternErrors.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> r1 = null;
if (r1 instanceof GenRecord(String s)) {}
switch (r1) {
Expand Down

0 comments on commit 2021d16

Please sign in to comment.