Skip to content

Commit

Permalink
[23] JEP 455 - Exhaustiveness of Switch eclipse-jdt#2869
Browse files Browse the repository at this point in the history 
+ improve test coverage

+ Fixes for WIDENING_AND_NARROWING_PRIMITIVE_CONVERSION
  - must be checked before individual narrowing or widening
  - pattern doesn't cover its type
+ Implement remaining routes in InstanceOfExpression.generateTypeCheck()
  + those are unconditionally exact
  • Loading branch information
@stephan-herrmann
stephan-herrmann committed Aug 31, 2024
1 parent bd29a9f commit 7f70a85
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 24 deletions.
44 changes: 26 additions & 18 deletions ...t.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,39 +221,46 @@ public void generateOptimizedBoolean(BlockScope currentScope, CodeStream codeStr

private void generateTypeCheck(BlockScope scope, CodeStream codeStream, BranchLabel internalFalseLabel, PrimitiveConversionRoute route) {
switch (route) {
case IDENTITY_CONVERSION:
case IDENTITY_CONVERSION -> {
storeExpressionValue(codeStream);
codeStream.iconst_1();
setPatternIsTotalType();
break;
case WIDENING_PRIMITIVE_CONVERSION:
case NARROWING_PRIMITVE_CONVERSION:
case WIDENING_AND_NARROWING_PRIMITIVE_CONVERSION:
}
case WIDENING_PRIMITIVE_CONVERSION,
NARROWING_PRIMITVE_CONVERSION,
WIDENING_AND_NARROWING_PRIMITIVE_CONVERSION -> {
generateExactConversions(scope, codeStream);
setPatternIsTotalType();
break;
case BOXING_CONVERSION:
case BOXING_CONVERSION_AND_WIDENING_REFERENCE_CONVERSION:
}
case BOXING_CONVERSION,
BOXING_CONVERSION_AND_WIDENING_REFERENCE_CONVERSION -> {
storeExpressionValue(codeStream);
codeStream.iconst_1();
setPatternIsTotalType();
break;
// TODO: case WIDENING_REFERENCE_AND_UNBOXING_COVERSION:
// TODO: case WIDENING_REFERENCE_AND_UNBOXING_COVERSION_AND_WIDENING_PRIMITIVE_CONVERSION:
case NARROWING_AND_UNBOXING_CONVERSION:
}
case WIDENING_REFERENCE_AND_UNBOXING_COVERSION,
WIDENING_REFERENCE_AND_UNBOXING_COVERSION_AND_WIDENING_PRIMITIVE_CONVERSION -> {
codeStream.ifnull(internalFalseLabel);
codeStream.iconst_1();
setPatternIsTotalType();
}
case NARROWING_AND_UNBOXING_CONVERSION -> {
TypeBinding boxType = scope.environment().computeBoxingType(this.type.resolvedType);
codeStream.instance_of(this.type, boxType);
break;
case UNBOXING_CONVERSION:
case UNBOXING_AND_WIDENING_PRIMITIVE_CONVERSION:
}
case UNBOXING_CONVERSION,
UNBOXING_AND_WIDENING_PRIMITIVE_CONVERSION -> {
codeStream.ifnull(internalFalseLabel);
codeStream.iconst_1();
setPatternIsTotalType();
break;
case NO_CONVERSION_ROUTE:
default:
}
case NO_CONVERSION_ROUTE -> {
codeStream.instance_of(this.type, this.type.resolvedType);
break;
}
default -> {
throw new IllegalArgumentException("Unexpected conversion route "+route); //$NON-NLS-1$
}
}
}

Expand Down Expand Up @@ -302,6 +309,7 @@ public TypeBinding resolveType(BlockScope scope) {
}
TypeBinding expressionType = this.expression.resolveType(scope);
if (this.pattern != null) {
this.expression.computeConversion(scope, expressionType, expressionType); // avoid that a total pattern would skip a checkCast, needed due to generics
this.pattern.setExpressionContext(ExpressionContext.TESTING_CONTEXT);
this.pattern.setOuterExpressionType(this.expression.resolvedType);
this.pattern.resolveType(scope);
Expand Down
6 changes: 4 additions & 2 deletions org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Pattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ public boolean coversType(TypeBinding type, Scope scope) {
return true;
case WIDENING_PRIMITIVE_CONVERSION:
return BaseTypeBinding.isExactWidening(this.resolvedType.id, type.id);
case WIDENING_AND_NARROWING_PRIMITIVE_CONVERSION:
return false; // char->byte
/* §14.11.1.1 "CE contains a type pattern with a primitive type P,
* and T is the wrapper class for the primitive type W,
* and the conversion from type W to type P is unconditionally exact (5.7.2). */
Expand Down Expand Up @@ -214,12 +216,12 @@ public static PrimitiveConversionRoute findPrimitiveConversionRoute(TypeBinding
if (TypeBinding.equalsEquals(destinationType, expressionType)) {
return PrimitiveConversionRoute.IDENTITY_CONVERSION;
}
if (BaseTypeBinding.isWideningAndNarrowing(destinationType.id, expressionType.id))
return PrimitiveConversionRoute.WIDENING_AND_NARROWING_PRIMITIVE_CONVERSION;
if (BaseTypeBinding.isWidening(destinationType.id, expressionType.id))
return PrimitiveConversionRoute.WIDENING_PRIMITIVE_CONVERSION;
if (BaseTypeBinding.isNarrowing(destinationType.id, expressionType.id))
return PrimitiveConversionRoute.NARROWING_PRIMITVE_CONVERSION;
if (BaseTypeBinding.isWideningAndNarrowing(destinationType.id, expressionType.id))
return PrimitiveConversionRoute.WIDENING_AND_NARROWING_PRIMITIVE_CONVERSION;
} else {
if (expressionIsBaseType) {
if (expressionType instanceof NullTypeBinding
Expand Down
87 changes: 83 additions & 4 deletions ...ompiler/src/org/eclipse/jdt/core/tests/compiler/regression/PrimitiveInPatternsTestSH.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1493,28 +1493,72 @@ public void testPrimitivePatternInSwitch_widenUnbox() {
runConformTest(new String[] {
"X.java",
"""
import java.util.Optional;
public class X {
static <T extends Integer> int mInteger(T in) {
return switch (in) {
case int v -> v;
default -> -1;
};
}
static <T extends Short> int mShort(T in) {
return switch (in) {
static int mShort(Optional<? extends Short> in) {
return switch (in.get()) {
case int v -> v;
default -> -1;
};
}
public static void main(String... args) {
System.out.print(mInteger(Integer.valueOf(1)));
System.out.print(mShort(Short.valueOf((short) 2)));
System.out.print(mShort(Optional.of(Short.valueOf((short) 2))));
}
}
"""
},
"12");
}
public void testInstanceof_widenUnbox() {
runConformTest(new String[] {
"X.java",
"""
import java.util.Optional;
public class X {
static <T extends Integer> int mInteger(T in) {
if (in instanceof int v) return v;
return -1;
}
static int mShort(Optional<? extends Short> in) {
if (in.get() instanceof int v) return v;
return -1;
}
public static void main(String... args) {
System.out.print(mInteger(Integer.valueOf(1)));
System.out.print(mShort(Optional.of(Short.valueOf((short) 2))));
}
}
"""
},
"12");
}
public void testInstanceof_genericExpression() { // regression test for a checkCast which we failed to generate earlier
runConformTest(new String[] {
"X.java",
"""
import java.util.List;
import java.util.Collections;
public class X {
static int mInteger(List<Integer> in) {
if (in.get(0) instanceof int v) // pattern is total, still the cast to Integer must be generated
return v;
return -1;
}
public static void main(String... args) {
System.out.print(mInteger(Collections.singletonList(Integer.valueOf(1))));
}
}
"""
},
"1");
}
public void testPrimitivePatternInSwitch_more() {
runConformTest(new String[] {
"X.java",
Expand All @@ -1534,6 +1578,13 @@ public static String switchfloatMoreCases(float f) {
case float v -> "v="+String.valueOf(v);
};
}
public static char switchByteToChar(byte b) {
return switch (b) {
case '1' -> 'A';
case char c -> c;
default -> '_';
};
}
public static void main(String... args) {
System.out.print(switchbool(true));
System.out.print("|");
Expand All @@ -1545,10 +1596,38 @@ public static void main(String... args) {
System.out.print("|");
System.out.print(switchfloatMoreCases(1.6f));
System.out.print("|");
System.out.print(switchByteToChar((byte) 49));
System.out.print("|");
System.out.print(switchByteToChar((byte) 50));
System.out.print("|");
System.out.print(switchByteToChar((byte) -1));
}
}
"""},
"true|v=false|1.0|1.5|v=1.6|");
"true|v=false|1.0|1.5|v=1.6|A|2|_");
}

public void testPrimitivePatternInSwitch_byteToChar_notExhaustive() {
runNegativeTest(new String[] {
"X.java",
"""
public class X {
public static char switchByteToChar(byte b) {
return switch (b) {
case '1' -> 'A';
case char c -> c;
};
}
}
"""},
"""
----------
1. ERROR in X.java (at line 3)
return switch (b) {
^
A switch expression should have a default case
----------
""");
}

private void testNarrowingInSwitchFrom(String from, int idx, String expectedOut) {
Expand Down

0 comments on commit 7f70a85

Please sign in to comment.