Skip to content

Commit

Permalink
[Switch-expression] Internal inconsistency warning at compile time & …
Browse files Browse the repository at this point in the history
…verify error at runtime (#2313)

* Fixes #2228
  • Loading branch information
srikanth-sankaran authored Apr 10, 2024
1 parent 8cf2f7a commit 607cb99
Show file tree
Hide file tree
Showing 12 changed files with 283 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ public abstract class AbstractMethodDeclaration

AbstractMethodDeclaration(CompilationResult compilationResult){
this.compilationResult = compilationResult;
this.containsSwitchWithTry = false;
}

/*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,8 @@ public void generateCode(ClassScope classScope, ClassFile classFile) {
if (referenceContext != null) {
unitResult = referenceContext.compilationResult();
problemCount = unitResult.problemCount;
if (referenceContext.clinitContainsSwitchWithTry)
this.containsSwitchWithTry = true;
}
}
boolean restart = false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ public CompactConstructorDeclaration(CompilationResult compilationResult) {
public void parseStatements(Parser parser, CompilationUnitDeclaration unit) {
this.constructorCall = SuperReference.implicitSuperConstructorCall();
parser.parse(this, unit, false);
this.containsSwitchWithTry = parser.switchWithTry;
}
@Override
public void analyseCode(ClassScope classScope, InitializationFlowContext initializerFlowContext, FlowInfo flowInfo, int initialReachMode) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ public void generateCode(ClassScope classScope, ClassFile classFile) {
if (referenceContext != null) {
unitResult = referenceContext.compilationResult();
problemCount = unitResult.problemCount;
if (referenceContext.initContainsSwitchWithTry)
this.containsSwitchWithTry = true;
}
}
do {
Expand Down Expand Up @@ -601,7 +603,6 @@ public void parseStatements(Parser parser, CompilationUnitDeclaration unit) {
return;
}
parser.parse(this, unit, false);
this.containsSwitchWithTry = parser.switchWithTry;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,6 @@ public RecordComponent getRecordComponent() {
public void parseStatements(Parser parser, CompilationUnitDeclaration unit) {
//fill up the method body with statement
parser.parse(this, unit);
this.containsSwitchWithTry = parser.switchWithTry;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import org.eclipse.jdt.internal.compiler.lookup.LocalVariableBinding;
import org.eclipse.jdt.internal.compiler.lookup.LookupEnvironment;
import org.eclipse.jdt.internal.compiler.lookup.MethodBinding;
import org.eclipse.jdt.internal.compiler.lookup.MethodScope;
import org.eclipse.jdt.internal.compiler.lookup.PolyTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.Scope;
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
Expand Down Expand Up @@ -396,6 +397,21 @@ public TypeBinding resolveType(BlockScope upperScope) {
if (this.constant != Constant.NotAConstant) {
this.constant = Constant.NotAConstant;

if (this.containsTry) {
MethodScope namedMethodScope = upperScope.namedMethodScope();
if (namedMethodScope != null) {
if (namedMethodScope.referenceContext instanceof AbstractMethodDeclaration amd) {
amd.containsSwitchWithTry = true;
} else if (namedMethodScope.referenceContext instanceof TypeDeclaration typeDecl) {
if (namedMethodScope.isStatic) {
typeDecl.clinitContainsSwitchWithTry = true;
} else {
typeDecl.initContainsSwitchWithTry = true;
}
}
}
}

// A switch expression is a poly expression if it appears in an assignment context or an invocation context (5.2, 5.3).
// Otherwise, it is a standalone expression.
if (this.expressionContext == ASSIGNMENT_CONTEXT || this.expressionContext == INVOCATION_CONTEXT) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,6 @@ public class TryStatement extends SubRoutineStatement {
private ExceptionLabel[] resourceExceptionLabels;
private int[] caughtExceptionsCatchBlocks;

public SwitchExpression enclosingSwitchExpression = null;

@Override
public FlowInfo analyseCode(BlockScope currentScope, FlowContext flowContext, FlowInfo flowInfo) {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,9 @@ public class TypeDeclaration extends Statement implements ProblemSeverities, Ref
// TEST ONLY: disable one fix here to challenge another related fix (in TypeSystem):
public static boolean TESTING_GH_2158 = false;

public boolean initContainsSwitchWithTry = false;
public boolean clinitContainsSwitchWithTry = false;

static {
disallowedComponentNames = new HashSet<>(6);
disallowedComponentNames.add("clone"); //$NON-NLS-1$
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ public class YieldStatement extends BranchStatement {

public Expression expression;
public SwitchExpression switchExpression;
public TryStatement tryStatement;

public boolean isImplicit;
static final char[] SECRET_YIELD_RESULT_VALUE_NAME = " secretYieldValue".toCharArray(); //$NON-NLS-1$
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7730,17 +7730,33 @@ private TypeBinding retrieveLocalType(int currentPC, int resolvedPosition) {
}
}
}
ReferenceBinding declaringClass = this.methodDeclaration.binding.declaringClass;
if (resolvedPosition == 0 && !this.methodDeclaration.isStatic()) {
return declaringClass;
}
int enumOffset = declaringClass.isEnum() ? 2 : 0; // String name, int ordinal
int argSlotSize = 1 + enumOffset; // this==aload0
if (this.methodDeclaration.binding.isConstructor()) {
if (declaringClass.isNestedType()) {
ReferenceBinding[] enclosingTypes = declaringClass.syntheticEnclosingInstanceTypes();
if (enclosingTypes != null) {
if (resolvedPosition < argSlotSize + enclosingTypes.length)
return enclosingTypes[resolvedPosition - argSlotSize];
}
for (SyntheticArgumentBinding extraSyntheticArgument : declaringClass.syntheticOuterLocalVariables()) {
if (extraSyntheticArgument.resolvedPosition == resolvedPosition)
return extraSyntheticArgument.type;
}
}
}

return null;
}
private void pushTypeBinding(int resolvedPosition) {
if (!isSwitchStackTrackingActive())
return;
assert resolvedPosition < this.maxLocals;
TypeBinding type = retrieveLocalType(this.position, resolvedPosition);
if (type == null && resolvedPosition == 0 && !this.methodDeclaration.isStatic()) {
type = this.methodDeclaration.binding.declaringClass; // thisReference.resolvedType
}
assert type != null;
pushTypeBinding(type);
}
private void pushTypeBindingArray() {
Expand All @@ -7764,7 +7780,9 @@ private void pushTypeBinding(char[] typeName) {
}
private void pushTypeBinding(TypeBinding typeBinding) {
if (isSwitchStackTrackingActive()) {
assert typeBinding != null;
if (typeBinding == null) {
throw new AssertionError("Operand type is null"); //$NON-NLS-1$
}
this.switchSaveTypeBindings.push(typeBinding);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,6 @@ protected int actFromTokenOrSynthetic(int previousAct) {
// protected int casePtr;
protected Map<Integer, Integer> caseStartMap = new HashMap<>();
ASTNode [] noAstNodes = new ASTNode[AstStackIncrement];
public boolean switchWithTry = false;

Expression [] noExpressions = new Expression[ExpressionStackIncrement];
//modifiers dimensions nestedType etc.......
Expand Down Expand Up @@ -9598,8 +9597,6 @@ public boolean visit(YieldStatement yieldStatement, BlockScope blockScope) {
// flag an error while resolving
yieldStatement.switchExpression = targetSwitchExpression;
}
if (this.tryStatements != null && !this.tryStatements.empty())
yieldStatement.tryStatement = this.tryStatements.peek();
return true;
}
@Override
Expand All @@ -9621,7 +9618,6 @@ public boolean visit(TryStatement stmt, BlockScope blockScope) {
this.tryStatements.push(stmt);
SwitchExpression targetSwitchExpression = this.targetSwitchExpressions.peek();
targetSwitchExpression.containsTry = true;
stmt.enclosingSwitchExpression = targetSwitchExpression;
return true;
}
@Override
Expand Down Expand Up @@ -9659,7 +9655,6 @@ protected void consumeSwitchExpression() {
problemReporter().switchExpressionsNotSupported(s);
}
collectResultExpressionsYield(s);
this.switchWithTry |= s.containsTry;
pushOnExpressionStack(s);
}
}
Expand Down Expand Up @@ -12187,7 +12182,6 @@ public void initialize(boolean parsingCompilationUnit) {
this.nestedMethod[this.nestedType = 0] = 0; // need to reset for further reuse
this.switchNestingLevel = 0;
this.caseStartMap.clear();
this.switchWithTry = false;
this.variablesCounter[this.nestedType] = 0;
this.dimensions = 0 ;
this.realBlockPtr = -1;
Expand Down Expand Up @@ -13411,7 +13405,6 @@ protected void prepareForBlockStatements() {
this.variablesCounter[this.nestedType] = 0;
this.realBlockStack[this.realBlockPtr = 1] = 0;
this.switchNestingLevel = 0;
this.switchWithTry = false;
}
/**
* Returns this parser's problem reporter initialized with its reference context.
Expand Down Expand Up @@ -14007,7 +14000,6 @@ protected void resetStacks() {
this.nestedMethod[this.nestedType = 0] = 0; // need to reset for further reuse
this.variablesCounter[this.nestedType] = 0;
this.switchNestingLevel = 0;
this.switchWithTry = false;

this.dimensions = 0 ;
this.realBlockStack[this.realBlockPtr = 0] = 0;
Expand Down Expand Up @@ -14245,7 +14237,6 @@ public void copyState(Parser from) {
this.intPtr = parser.intPtr;
this.nestedType = parser.nestedType;
this.switchNestingLevel = parser.switchNestingLevel;
this.switchWithTry = parser.switchWithTry;
this.realBlockPtr = parser.realBlockPtr;
this.valueLambdaNestDepth = parser.valueLambdaNestDepth;

Expand Down
Loading

0 comments on commit 607cb99

Please sign in to comment.