Skip to content

Commit

Permalink
[23] exhaustiveness signalling difference between javac and ecj
Browse files Browse the repository at this point in the history
+ clarify that flagDuplicateDefault() flags only conditionally,
  renamed to checkDuplicateDefault()
  - inside this method clarify that only one error is reported per loc.
+ remove all conflict reporting from CaseStatement.resolveCasePattern
  - will be done within SwitchStatement.resolve() anyway
+ remove IProblem.DuplicateTotalPattern and related code
  - all errors reported here coincided with a dominance error
+ adjust tests: no longer expect secondary errors

fixes eclipse-jdt#2915
  • Loading branch information
stephan-herrmann committed Sep 7, 2024
1 parent 5892326 commit f508a96
Show file tree
Hide file tree
Showing 10 changed files with 89 additions and 59 deletions.
6 changes: 6 additions & 0 deletions org.eclipse.jdt.core.compiler.batch/.settings/.api_filters
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@
<message_argument value="DisallowedStatementInPrologue"/>
</message_arguments>
</filter>
<filter comment="this preview constant (marked @noreference) is unnecesary" id="405864542">
<message_arguments>
<message_argument value="org.eclipse.jdt.core.compiler.IProblem"/>
<message_argument value="DuplicateTotalPattern"/>
</message_arguments>
</filter>
<filter comment="renamed constant for JEP 482" id="405864542">
<message_arguments>
<message_argument value="org.eclipse.jdt.core.compiler.IProblem"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2537,9 +2537,6 @@ public interface IProblem {
/** @since 3.28
* @noreference preview feature error */
int EnhancedSwitchMissingDefault = PreviewRelated + 1908;
/** @since 3.28
* @noreference preview feature error */
int DuplicateTotalPattern = PreviewRelated + 1909;

/** @since 3.34
* @noreference preview feature error */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ public ResolvedCase[] resolveCase(BlockScope scope, TypeBinding switchExpression
this.swich = switchStatement;
scope.enclosingCase = this; // record entering in a switch case block
if (this.constantExpressions == Expression.NO_EXPRESSIONS) {
flagDuplicateDefault(scope, switchStatement, this);
checkDuplicateDefault(scope, switchStatement, this);
return ResolvedCase.UnresolvedCase;
}

Expand All @@ -192,7 +192,7 @@ public ResolvedCase[] resolveCase(BlockScope scope, TypeBinding switchExpression
for (Expression e : this.constantExpressions) {
count++;
if (e instanceof FakeDefaultLiteral) {
flagDuplicateDefault(scope, switchStatement, this.constantExpressions.length > 1 ? e : this);
checkDuplicateDefault(scope, switchStatement, this.constantExpressions.length > 1 ? e : this);
if (count != 2 || nullCaseCount < 1) {
scope.problemReporter().patternSwitchCaseDefaultOnlyAsSecond(e);
}
Expand Down Expand Up @@ -267,16 +267,20 @@ public ResolvedCase[] resolveCase(BlockScope scope, TypeBinding switchExpression
return hasResolveErrors ? ResolvedCase.UnresolvedCase : cases.toArray(new ResolvedCase[cases.size()]);
}

private void flagDuplicateDefault(BlockScope scope, SwitchStatement switchStatement, ASTNode node) {
// remember the default case into the associated switch statement
if (switchStatement.defaultCase != null)
/**
* Precondition: this is a default case.
* Check if (a) another default or (b) a total type pattern has been recorded already
*/
private void checkDuplicateDefault(BlockScope scope, SwitchStatement switchStatement, ASTNode node) {
if (switchStatement.defaultCase != null) {
scope.problemReporter().duplicateDefaultCase(node);
} else if ((switchStatement.switchBits & SwitchStatement.TotalPattern) != 0) {
scope.problemReporter().illegalTotalPatternWithDefault(this);
}

// remember the default case into the associated switch statement
// on error the last default will be the selected one ...
switchStatement.defaultCase = this;
if ((switchStatement.switchBits & SwitchStatement.TotalPattern) != 0) {
scope.problemReporter().illegalTotalPatternWithDefault(this);
}
}

@Override
Expand Down Expand Up @@ -394,17 +398,10 @@ private Constant resolveCasePattern(BlockScope scope, TypeBinding caseType, Type
}
}
if (e.coversType(expressionType, scope)) {
if ((switchStatement.switchBits & SwitchStatement.TotalPattern) != 0) {
scope.problemReporter().duplicateTotalPattern(e);
return IntConstant.fromValue(-1);
}
switchStatement.switchBits |= SwitchStatement.Exhaustive;
if (e.isUnconditional(expressionType, scope)) {
switchStatement.switchBits |= SwitchStatement.TotalPattern;
if (switchStatement.defaultCase != null && !(e instanceof RecordPattern))
scope.problemReporter().illegalTotalPatternWithDefault(this);
}
e.isTotalTypeNode = true;
if (e.isUnconditional(expressionType, scope)) // unguarded is implied from 'coversType()' above
switchStatement.switchBits |= SwitchStatement.TotalPattern;
if (switchStatement.nullCase == null)
constant = IntConstant.fromValue(-1);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1192,8 +1192,7 @@ public void resolve(BlockScope upperScope) {
ResolvedCase[] constantsList;
final Statement statement = this.statements[i];
if (statement instanceof CaseStatement caseStmt) {
caseNullDefaultFound = caseNullDefaultFound ? caseNullDefaultFound
: isCaseStmtNullDefault(caseStmt);
caseNullDefaultFound |= isCaseStmtNullDefault(caseStmt);
defaultFound |= caseStmt.constantExpressions == Expression.NO_EXPRESSIONS;
constantsList = caseStmt.resolveCase(this.scope, expressionType, this);
patternVariables = statement.bindingsWhenTrue();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12597,14 +12597,6 @@ public void enhancedSwitchMissingDefaultCase(ASTNode element) {
element.sourceStart,
element.sourceEnd);
}
public void duplicateTotalPattern(ASTNode element) {
this.handle(
IProblem.DuplicateTotalPattern,
NoArgument,
NoArgument,
element.sourceStart,
element.sourceEnd);
}
public void unexpectedTypeinSwitchPattern(TypeBinding type, ASTNode element) {
this.handle(
IProblem.UnexpectedTypeinSwitchPattern,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,7 @@
1906 = This case label is dominated by one of the preceding case labels
1907 = Switch case cannot have both unconditional pattern and default label
1908 = An enhanced switch statement should be exhaustive; a default label expected
1909 = The switch statement cannot have more than one unconditional pattern
# 1909 removed
1910 = Unnecessary 'null' pattern, the switch selector expression cannot be null
1911 = Unexpected type {0}, expected class or array type
1920 = A null case label has to be either the only expression in a case label or the first expression followed only by a default
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1340,7 +1340,6 @@ class ProblemAttributes {
expectedProblemAttributes.put("PatternDominated", new ProblemAttributes(true));
expectedProblemAttributes.put("IllegalTotalPatternWithDefault", new ProblemAttributes(true));
expectedProblemAttributes.put("EnhancedSwitchMissingDefault", new ProblemAttributes(true));
expectedProblemAttributes.put("DuplicateTotalPattern", new ProblemAttributes(true));
expectedProblemAttributes.put("UnexpectedTypeinSwitchPattern", new ProblemAttributes(true));
expectedProblemAttributes.put("UnexpectedTypeinRecordPattern", new ProblemAttributes(true));
expectedProblemAttributes.put("RecordPatternMismatch", new ProblemAttributes(true));
Expand Down Expand Up @@ -2474,7 +2473,6 @@ class ProblemAttributes {
expectedProblemAttributes.put("PatternDominated", SKIP);
expectedProblemAttributes.put("IllegalTotalPatternWithDefault", SKIP);
expectedProblemAttributes.put("EnhancedSwitchMissingDefault", SKIP);
expectedProblemAttributes.put("DuplicateTotalPattern", SKIP);
expectedProblemAttributes.put("UnexpectedTypeinSwitchPattern", SKIP);
expectedProblemAttributes.put("UnexpectedTypeinRecordPattern", SKIP);
expectedProblemAttributes.put("RecordPatternMismatch", SKIP);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2221,6 +2221,7 @@ public static void main(String... args) {
}

public void testCoversTypePlusDefault() {
// case int i "covers" type Integer but is not unconditional
runConformTest(new String[] {
"X.java",
"""
Expand All @@ -2241,6 +2242,72 @@ public static void main(String argv[]) {
},
"true");
}

public void testUnconditionPlusDefault() {
// case int i "covers" type int and is unconditional
// various combinations of dominance with/without default
runNegativeTest(new String[] {
"X.java",
"""
public class X {
int foo1(int myInt) {
return switch (myInt) {
case int i -> i;
default -> 0; // conflict with preceding total pattern (unguarded and unconditional)
};
}
int foo2(int myInt) {
return switch (myInt) { // swapped order of cases
default -> 0;
case int i -> i; // conflict with preceding default
};
}
int foo3(int myInt) {
return switch (myInt) {
default -> 0;
case int i -> i; // conflict with preceding default
case short s -> s; // additionally dominated by int i
};
}
int foo4(int myInt) {
return switch (myInt) {
case int i -> i;
case short s -> s; // dominated by int i
};
}
}
"""
},
"""
----------
1. ERROR in X.java (at line 5)
default -> 0; // conflict with preceding total pattern (unguarded and unconditional)
^^^^^^^
Switch case cannot have both unconditional pattern and default label
----------
2. ERROR in X.java (at line 11)
case int i -> i; // conflict with preceding default
^^^^^
This case label is dominated by one of the preceding case labels
----------
3. ERROR in X.java (at line 17)
case int i -> i; // conflict with preceding default
^^^^^
This case label is dominated by one of the preceding case labels
----------
4. ERROR in X.java (at line 18)
case short s -> s; // additionally dominated by int i
^^^^^^^
This case label is dominated by one of the preceding case labels
----------
5. ERROR in X.java (at line 24)
case short s -> s; // dominated by int i
^^^^^^^
This case label is dominated by one of the preceding case labels
----------
""");
}

// test from spec
public void _testSpec001() {
runConformTest(new String[] {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -367,12 +367,6 @@ public void test009() {
" case Rectangle(ColoredPoint(Point(int x, int y), Color c),\n" +
" ColoredPoint(Point(int x1, int y1), Color c1)) -> {\n" +
" ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
"The switch statement cannot have more than one unconditional pattern\n" +
"----------\n" +
"2. ERROR in X.java (at line 7)\n" +
" case Rectangle(ColoredPoint(Point(int x, int y), Color c),\n" +
" ColoredPoint(Point(int x1, int y1), Color c1)) -> {\n" +
" ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n" +
"This case label is dominated by one of the preceding case labels\n" +
"----------\n");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2876,11 +2876,6 @@ public void testBug575048_01() {
"----------\n" +
"1. ERROR in X.java (at line 5)\n" +
" case Integer i1 -> 0;\n" +
" ^^^^^^^^^^^^^^^\n" +
"Switch case cannot have both unconditional pattern and default label\n" +
"----------\n" +
"2. ERROR in X.java (at line 5)\n" +
" case Integer i1 -> 0;\n" +
" ^^^^^^^^^^\n" +
"This case label is dominated by one of the preceding case labels\n" +
"----------\n");
Expand Down Expand Up @@ -3539,11 +3534,6 @@ public void testBug575047_06() {
"1. ERROR in X.java (at line 6)\n" +
" case String s -> -1;\n" +
" ^^^^^^^^\n" +
"The switch statement cannot have more than one unconditional pattern\n" +
"----------\n" +
"2. ERROR in X.java (at line 6)\n" +
" case String s -> -1;\n" +
" ^^^^^^^^\n" +
"This case label is dominated by one of the preceding case labels\n" +
"----------\n");
}
Expand Down Expand Up @@ -5751,11 +5741,6 @@ public void testIssue742_1() {
"1. ERROR in X.java (at line 7)\n" +
" case Integer i -> // Error - dominated case label\n" +
" ^^^^^^^^^\n" +
"The switch statement cannot have more than one unconditional pattern\n" +
"----------\n" +
"2. ERROR in X.java (at line 7)\n" +
" case Integer i -> // Error - dominated case label\n" +
" ^^^^^^^^^\n" +
"This case label is dominated by one of the preceding case labels\n" +
"----------\n");
}
Expand All @@ -5779,11 +5764,6 @@ public void testIssue742_2() {
"1. ERROR in X.java (at line 7)\n" +
" case Integer i when true -> // Error - dominated case label\n" +
" ^^^^^^^^^\n" +
"The switch statement cannot have more than one unconditional pattern\n" +
"----------\n" +
"2. ERROR in X.java (at line 7)\n" +
" case Integer i when true -> // Error - dominated case label\n" +
" ^^^^^^^^^\n" +
"This case label is dominated by one of the preceding case labels\n" +
"----------\n");
}
Expand Down

0 comments on commit f508a96

Please sign in to comment.