Skip to content

Commit

Permalink
[20] Switch statement with record pattern illegally reported as non-e…
Browse files Browse the repository at this point in the history 
…xhaustive (eclipse-jdt#1228)

Fixed issue  eclipse-jdt#1224

Now, a total record pattern, which from now on called as a pattern that covers the expression type, can be mixed with an optional default case. The absence of such default should now be allowed.
  • Loading branch information
@jarthana
jarthana authored Jul 18, 2023
1 parent 2841fde commit bbef232
Show file tree
Hide file tree
Showing 7 changed files with 263 additions and 40 deletions.
13 changes: 8 additions & 5 deletions ...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CaseStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -502,15 +502,18 @@ private Constant resolveConstantExpression(BlockScope scope,
return Constant.NotAConstant;
}
}
if (e.isTotalForType(expressionType)) {
if (e.coversType(expressionType)) {
if ((switchStatement.switchBits & SwitchStatement.TotalPattern) != 0) {
scope.problemReporter().duplicateTotalPattern(e);
return IntConstant.fromValue(-1);
}
switchStatement.switchBits |= (SwitchStatement.TotalPattern | SwitchStatement.Exhaustive);
if (switchStatement.defaultCase != null)
scope.problemReporter().illegalTotalPatternWithDefault(this);
switchStatement.totalPattern = e;
switchStatement.switchBits |= SwitchStatement.Exhaustive;
if (e.isAlwaysTrue()) {
switchStatement.switchBits |= SwitchStatement.TotalPattern;
if (switchStatement.defaultCase != null && !(e instanceof RecordPattern))
scope.problemReporter().illegalTotalPatternWithDefault(this);
switchStatement.totalPattern = e;
}
e.isTotalTypeNode = true;
if (switchStatement.nullCase == null)
constant = IntConstant.fromValue(-1);
Expand Down
6 changes: 3 additions & 3 deletions ...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/GuardedPattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ public boolean isAlwaysTrue() {
return cst != Constant.NotAConstant && cst.booleanValue() == true;
}
@Override
public boolean isTotalForType(TypeBinding type) {
return this.primaryPattern.isTotalForType(type) && isAlwaysTrue();
public boolean coversType(TypeBinding type) {
return this.primaryPattern.coversType(type) && isAlwaysTrue();
}
@Override
public Pattern primary() {
Expand Down Expand Up @@ -157,7 +157,7 @@ public boolean visit(
public TypeBinding resolveAtType(BlockScope scope, TypeBinding u) {
if (this.resolvedType == null || this.primaryPattern == null)
return null;
if (this.primaryPattern.isTotalForType(u))
if (this.primaryPattern.coversType(u))
return this.primaryPattern.resolveAtType(scope, u);

return this.resolvedType; //else leave the pattern untouched for now.
Expand Down
9 changes: 7 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 @@ -77,8 +77,13 @@ public void setEnclosingPattern(Pattern enclosingPattern) {
this.enclosingPattern = enclosingPattern;
this.nestingLevel = enclosingPattern.nestingLevel+1;
}

public boolean isTotalForType(TypeBinding type) {
/**
* Implement the rules in the spec under 14.11.1.1 Exhaustive Switch Blocks
*
* @param type
* @return whether pattern covers the given type or not
*/
public boolean coversType(TypeBinding type) {
return false;
}
public TypeBinding resolveAtType(BlockScope scope, TypeBinding type) {
Expand Down
65 changes: 37 additions & 28 deletions ...ipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/RecordPattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,25 @@ public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, Fl
return flowInfo;
}
@Override
public boolean isTotalForType(TypeBinding t) {
return false;
public boolean coversType(TypeBinding t) {
if (TypeBinding.equalsEquals(t, this.resolvedType)) {
// return the already computed value
return this.isTotalTypeNode;
}
if (!t.isRecord())
return false;
RecordComponentBinding[] components = t.components();
if (components == null || components.length != this.patterns.length) {
return false;
}
for (int i = 0; i < components.length; i++) {
Pattern p = this.patterns[i];
RecordComponentBinding componentBinding = components[i];
if (!p.coversType(componentBinding.type)) {
return false;
}
}
return true;
}
@Override
public void resolveWithExpression(BlockScope scope, Expression exp) {
Expand Down Expand Up @@ -172,7 +189,7 @@ public TypeBinding resolveType(BlockScope scope, boolean isPatternVariable) {
return this.resolvedType;
}
private void setAccessorsPlusInfuseInferredType(BlockScope scope) {
this.isTotalTypeNode = isTotalForType(this.resolvedType);
this.isTotalTypeNode = super.coversType(this.resolvedType);
RecordComponentBinding[] components = this.resolvedType.components();
if (components.length != this.patterns.length) {
scope.problemReporter().recordPatternSignatureMismatch(this.resolvedType, this);
Expand All @@ -191,12 +208,13 @@ private void setAccessorsPlusInfuseInferredType(BlockScope scope) {
p.resolveType(scope, true);
TypeBinding expressionType = componentBinding.type;
if (p.isPatternTypeCompatible(expressionType, scope)) {
p.isTotalTypeNode = p.isTotalForType(componentBinding.type);
p.isTotalTypeNode = p.coversType(componentBinding.type);
MethodBinding[] methods = this.resolvedType.getMethods(componentBinding.name);
if (methods != null && methods.length > 0) {
p.accessorMethod = methods[0];
}
}
this.isTotalTypeNode &= p.isTotalTypeNode;
}
}
}
Expand Down Expand Up @@ -239,7 +257,7 @@ public boolean isAlwaysTrue() {
public boolean dominates(Pattern p) {
if (!this.resolvedType.isValidBinding())
return false;
if (!super.isTotalForType(p.resolvedType)) {
if (!super.coversType(p.resolvedType)) {
return false;
}
if (p instanceof RecordPattern) {
Expand Down Expand Up @@ -282,18 +300,11 @@ public void generateOptimizedBoolean(BlockScope currentScope, CodeStream codeStr
}
@Override
protected void generatePatternVariable(BlockScope currentScope, CodeStream codeStream, BranchLabel trueLabel, BranchLabel falseLabel) {
if (!this.isTotalTypeNode) {
// codeStream.load(this.secretPatternVariable);
// codeStream.instance_of(this.resolvedType);
// BranchLabel target = falseLabel != null ? falseLabel : new BranchLabel(codeStream);
// codeStream.ifeq(target);
}
List<ExceptionLabel> labels = new ArrayList<>();
for (Pattern p : this.patterns) {
if (p.accessorMethod != null) {
codeStream.load(this.secretPatternVariable);
if (!this.isTotalTypeNode)
codeStream.checkcast(this.resolvedType);
codeStream.checkcast(this.resolvedType);

ExceptionLabel exceptionLabel = new ExceptionLabel(codeStream,TypeBinding.wellKnownType(currentScope, T_JavaLangThrowable));
exceptionLabel.placeStart();
Expand All @@ -305,21 +316,19 @@ protected void generatePatternVariable(BlockScope currentScope, CodeStream codeS
if (TypeBinding.notEquals(p.accessorMethod.original().returnType.erasure(),
p.accessorMethod.returnType.erasure()))
codeStream.checkcast(p.accessorMethod.returnType);
if (!p.isTotalTypeNode) {
if (p instanceof TypePattern) {
((TypePattern)p).getSecretVariable(currentScope, p.resolvedType);
((TypePattern)p).initializePatternVariables(currentScope, codeStream);
codeStream.load(p.secretPatternVariable);
codeStream.instance_of(p.resolvedType);
BranchLabel target = falseLabel != null ? falseLabel : new BranchLabel(codeStream);
List<LocalVariableBinding> deepPatternVars = getDeepPatternVariables(currentScope);
recordEndPCDeepPatternVars(deepPatternVars, codeStream.position);
codeStream.ifeq(target);
recordStartPCDeepPatternVars(deepPatternVars, codeStream.position);
p.secretPatternVariable.recordInitializationStartPC(codeStream.position);
codeStream.load(p.secretPatternVariable);
codeStream.removeVariable(p.secretPatternVariable);
}
if (p instanceof RecordPattern || !p.isTotalTypeNode) {
((TypePattern)p).getSecretVariable(currentScope, p.resolvedType);
((TypePattern)p).initializePatternVariables(currentScope, codeStream);
codeStream.load(p.secretPatternVariable);
codeStream.instance_of(p.resolvedType);
BranchLabel target = falseLabel != null ? falseLabel : new BranchLabel(codeStream);
List<LocalVariableBinding> deepPatternVars = getDeepPatternVariables(currentScope);
recordEndPCDeepPatternVars(deepPatternVars, codeStream.position);
codeStream.ifeq(target);
recordStartPCDeepPatternVars(deepPatternVars, codeStream.position);
p.secretPatternVariable.recordInitializationStartPC(codeStream.position);
codeStream.load(p.secretPatternVariable);
codeStream.removeVariable(p.secretPatternVariable);
}
p.generateOptimizedBoolean(currentScope, codeStream, trueLabel, falseLabel);
}
Expand Down
2 changes: 1 addition & 1 deletion ...se.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ public void resolve(BlockScope upperScope) {
if (type.isBaseType()) {
type = this.scope.environment().computeBoxingType(type);
}
if (p1.primary().isTotalForType(type))
if (p1.primary().coversType(type))
this.scope.problemReporter().patternDominatedByAnother(c.e);
}
}
Expand Down
2 changes: 1 addition & 1 deletion ...clipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypePattern.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ public void resolve(BlockScope scope) {
this.resolveType(scope);
}
@Override
public boolean isTotalForType(TypeBinding type) {
public boolean coversType(TypeBinding type) {
if (type == null || this.resolvedType == null)
return false;
return (type.isSubtypeOf(this.resolvedType, false));
Expand Down
Loading

0 comments on commit bbef232

Please sign in to comment.