Skip to content

Commit

Permalink
[23] JEP 455 - Exhaustiveness of Switch #2869 (#2878)
Browse files Browse the repository at this point in the history
+ true + false = exhaustive over boolean :)
+ leverage PrimitiveConversionRoute for Pattern.coversType()
+ fine-tune combinations of primitive and boxing types
+ fix one omission in BaseTypeBinding.isExactWidening()
+ code gen to respect exhaustiveness of switch over primitives
+ fix code gen for bootstrap in the case of boxing+widening conversion
+ simplify condition for generating a throwing default
+ UNBOXING_CONVERSION is exhaustive (trivial case from 14.11.1.1)
+ implement WIDENING_REFERENCE_AND_UNBOXING_COVERSION and
  ..._AND_WIDENING_PRIMITIVE_CONVERSION
  - detect in Pattern.findPrimitiveConversionRoute()
  - adjust SwitchStatement.typeSwitchSignature()
  - generate in TypePattern.generateTestingConversion()
clarify code gen for BOXING_CONVERSION_AND_WIDENING_REFERENCE_CONVERSION
implement NARROWING_AND_UNBOXING_CONVERSION
+ InstanceOfExpression.generateTypeCheck()
+ TypePattern.generateTestingConversion()
+ 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

Additional clean-up:
* clarify terminology for pairs of types
+ prefer 'provided' / 'expected' where possible
+ disentangle Pattern.outerExpressionType out ofExpression.expectedType
+ also connect to terms runtimeType vs. compileTimeType
* + tiny clean-up removing dead code

specific correct error message for incompatible case constant
+ fix bogus expectation in existing tests

fixes #2869
  • Loading branch information
stephan-herrmann authored Aug 31, 2024
1 parent 6b7f284 commit 2f5f013
Show file tree
Hide file tree
Showing 23 changed files with 908 additions and 191 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2690,4 +2690,10 @@ public interface IProblem {
*/
int WrongCaseType = PreviewRelated + 2100;

/**
* @since 3.39
* @noreference preview feature
*/
int IncompatibleCaseType = PreviewRelated + 2101;

}
Original file line number Diff line number Diff line change
Expand Up @@ -4019,7 +4019,7 @@ private int addBootStrapTypeSwitchEntry(int localContentsOffset, SwitchStatement
for (CaseStatement.ResolvedCase c : constants) {
if (c.isPattern()) {
int typeOrDynIndex;
if ((switchStatement.switchBits & SwitchStatement.Primitive) != 0) {
if (c.e.resolvedType.isPrimitiveType()) {
// Dynamic for Class.getPrimitiveClass(Z) or such
typeOrDynIndex = this.constantPool.literalIndexForDynamic(c.primitivesBootstrapIdx,
c.t.signature(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,8 @@ public ResolvedCase[] resolveCase(BlockScope scope, TypeBinding switchExpression
continue; // already processed
}
e.setExpressionContext(ExpressionContext.TESTING_CONTEXT);
e.setExpectedType(switchExpressionType);
if (e instanceof Pattern p)
p.setOuterExpressionType(switchExpressionType);

TypeBinding caseType = e.resolveType(scope);

Expand Down Expand Up @@ -301,7 +302,7 @@ public Constant resolveConstantExpression(BlockScope scope,
throw new AssertionError("Unexpected control flow"); //$NON-NLS-1$
} else if (expression instanceof NullLiteral) {
if (!caseType.isCompatibleWith(switchType, scope)) {
scope.problemReporter().typeMismatchError(TypeBinding.NULL, switchType, expression, null);
scope.problemReporter().caseConstantIncompatible(TypeBinding.NULL, switchType, expression);
}
switchStatement.switchBits |= SwitchStatement.NullCase;
return IntConstant.fromValue(-1);
Expand All @@ -310,7 +311,7 @@ public Constant resolveConstantExpression(BlockScope scope,
} else {
if (switchStatement.isNonTraditional) {
if (switchType.isBaseType() && !expression.isConstantValueOfTypeAssignableToType(caseType, switchType)) {
scope.problemReporter().typeMismatchError(caseType, switchType, expression, null);
scope.problemReporter().caseConstantIncompatible(caseType, switchType, expression);
return Constant.NotAConstant;
}
}
Expand Down Expand Up @@ -353,7 +354,7 @@ public Constant resolveConstantExpression(BlockScope scope,
// constantExpression.computeConversion(scope, caseType, switchExpressionType); - do not report boxing/unboxing conversion
return expression.constant;
}
scope.problemReporter().typeMismatchError(expression.resolvedType, switchType, expression, switchStatement.expression);
scope.problemReporter().caseConstantIncompatible(expression.resolvedType, switchType, expression);
return Constant.NotAConstant;
}

Expand Down Expand Up @@ -381,28 +382,24 @@ private Constant resolveCasePattern(BlockScope scope, TypeBinding caseType, Type
}
}
} else if (type.isValidBinding()) {
PrimitiveConversionRoute route = PrimitiveConversionRoute.NO_CONVERSION_ROUTE;
// if not a valid binding, an error has already been reported for unresolved type
if (type.isPrimitiveType()) {
route = Pattern.findPrimitiveConversionRoute(type, expressionType, scope);
if (route == PrimitiveConversionRoute.NO_CONVERSION_ROUTE) {
if (Pattern.findPrimitiveConversionRoute(type, expressionType, scope) == PrimitiveConversionRoute.NO_CONVERSION_ROUTE) {
if (type.isPrimitiveType() && !JavaFeature.PRIMITIVES_IN_PATTERNS.isSupported(scope.compilerOptions())) {
scope.problemReporter().unexpectedTypeinSwitchPattern(type, e);
return Constant.NotAConstant;
} else if (!e.checkCastTypesCompatibility(scope, type, expressionType, null, false)) {
scope.problemReporter().typeMismatchError(expressionType, type, e, null);
return Constant.NotAConstant;
}
}
if ((type.isBaseType() && route == PrimitiveConversionRoute.NO_CONVERSION_ROUTE)
|| !e.checkCastTypesCompatibility(scope, type, expressionType, null, false)) {
scope.problemReporter().typeMismatchError(expressionType, type, e, null);
return Constant.NotAConstant;
}
}
if (e.coversType(expressionType)) {
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)) {
if (e.isUnconditional(expressionType, scope)) {
switchStatement.switchBits |= SwitchStatement.TotalPattern;
if (switchStatement.defaultCase != null && !(e instanceof RecordPattern))
scope.problemReporter().illegalTotalPatternWithDefault(this);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding;
import org.eclipse.jdt.internal.compiler.lookup.Scope;
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;

public class EitherOrMultiPattern extends Pattern {
Expand Down Expand Up @@ -109,11 +110,11 @@ public boolean dominates(Pattern p) {
}

@Override
public boolean coversType(TypeBinding type) {
public boolean coversType(TypeBinding type, Scope scope) {
if (!isUnguarded())
return false;
for (Pattern p : this.patterns) {
if (p.coversType(type))
if (p.coversType(type, scope))
return true;
}
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,9 @@ public boolean checkUnsafeCast(Scope scope, TypeBinding castType, TypeBinding ex
/**
* Base types need that the widening is explicitly done by the compiler using some bytecode like i2f.
* Also check unsafe type operations.
* @param scope a scope
* @param runtimeType this is the type <strong>required</strong> at runtime
* @param compileTimeType this is what the compiler knows about the provided value
*/
public void computeConversion(Scope scope, TypeBinding runtimeType, TypeBinding compileTimeType) {
if (runtimeType == null || compileTimeType == null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.eclipse.jdt.internal.compiler.impl.Constant;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding;
import org.eclipse.jdt.internal.compiler.lookup.Scope;
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeIds;

Expand Down Expand Up @@ -64,8 +65,8 @@ public boolean matchFailurePossible() {
}

@Override
public boolean isUnconditional(TypeBinding t) {
return isUnguarded() && this.primaryPattern.isUnconditional(t);
public boolean isUnconditional(TypeBinding t, Scope scope) {
return isUnguarded() && this.primaryPattern.isUnconditional(t, scope);
}

@Override
Expand All @@ -80,8 +81,8 @@ public void setIsEitherOrPattern() {
}

@Override
public boolean coversType(TypeBinding type) {
return isUnguarded() && this.primaryPattern.coversType(type);
public boolean coversType(TypeBinding type, Scope scope) {
return isUnguarded() && this.primaryPattern.coversType(type, scope);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,36 +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:
// TODO: case NARROWING_AND_UNBOXING_CONVERSION:
case UNBOXING_CONVERSION:
case UNBOXING_AND_WIDENING_PRIMITIVE_CONVERSION:
}
case WIDENING_REFERENCE_AND_UNBOXING_COVERSION,
WIDENING_REFERENCE_AND_UNBOXING_COVERSION_AND_WIDENING_PRIMITIVE_CONVERSION -> {
codeStream.ifnull(internalFalseLabel);
codeStream.iconst_1();
setPatternIsTotalType();
break;
case NO_CONVERSION_ROUTE:
default:
}
case NARROWING_AND_UNBOXING_CONVERSION -> {
TypeBinding boxType = scope.environment().computeBoxingType(this.type.resolvedType);
codeStream.instance_of(this.type, boxType);
}
case UNBOXING_CONVERSION,
UNBOXING_AND_WIDENING_PRIMITIVE_CONVERSION -> {
codeStream.ifnull(internalFalseLabel);
codeStream.iconst_1();
setPatternIsTotalType();
}
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 @@ -299,8 +309,9 @@ 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.setExpectedType(this.expression.resolvedType);
this.pattern.setOuterExpressionType(this.expression.resolvedType);
this.pattern.resolveType(scope);

addSecretExpressionValue(scope, expressionType);
Expand Down Expand Up @@ -360,17 +371,10 @@ private void checkForPrimitives(BlockScope scope, TypeBinding checkedType, TypeB
}

private void checkRefForPrimitivesAndAddSecretVariable(BlockScope scope, TypeBinding checkedType, TypeBinding expressionType) {
if (!(JavaFeature.PRIMITIVES_IN_PATTERNS.isSupported(
scope.compilerOptions().sourceLevel,
scope.compilerOptions().enablePreviewFeatures)))
if (!JavaFeature.PRIMITIVES_IN_PATTERNS.isSupported(scope.compilerOptions()))
return;
PrimitiveConversionRoute route = Pattern.findPrimitiveConversionRoute(checkedType, expressionType, scope);
this.testContextRecord = new TestContextRecord(checkedType, expressionType, route);
if (route == PrimitiveConversionRoute.UNBOXING_CONVERSION
|| route == PrimitiveConversionRoute.UNBOXING_AND_WIDENING_PRIMITIVE_CONVERSION
) {
addSecretExpressionValue(scope, expressionType);
}
}

private void addSecretExpressionValue(BlockScope scope, TypeBinding expressionType) {
Expand Down
Loading

0 comments on commit 2f5f013

Please sign in to comment.