From 607cb99bbe3c4efa8bc76d22bdcb8b934c5f7805 Mon Sep 17 00:00:00 2001 From: Srikanth Sankaran <131454720+srikanth-sankaran@users.noreply.github.com> Date: Wed, 10 Apr 2024 17:29:26 +0530 Subject: [PATCH] [Switch-expression] Internal inconsistency warning at compile time & verify error at runtime (#2313) * Fixes https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2228 --- .../ast/AbstractMethodDeclaration.java | 1 - .../jdt/internal/compiler/ast/Clinit.java | 2 + .../ast/CompactConstructorDeclaration.java | 1 - .../compiler/ast/ConstructorDeclaration.java | 3 +- .../compiler/ast/MethodDeclaration.java | 1 - .../compiler/ast/SwitchExpression.java | 16 ++ .../internal/compiler/ast/TryStatement.java | 2 - .../compiler/ast/TypeDeclaration.java | 3 + .../internal/compiler/ast/YieldStatement.java | 1 - .../internal/compiler/codegen/CodeStream.java | 28 ++- .../jdt/internal/compiler/parser/Parser.java | 9 - .../SwitchExpressionsYieldTest.java | 237 ++++++++++++++++++ 12 files changed, 283 insertions(+), 21 deletions(-) diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java index 63dc8001abc..f72fad74cd2 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/AbstractMethodDeclaration.java @@ -83,7 +83,6 @@ public abstract class AbstractMethodDeclaration AbstractMethodDeclaration(CompilationResult compilationResult){ this.compilationResult = compilationResult; - this.containsSwitchWithTry = false; } /* diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Clinit.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Clinit.java index 7804c8b6437..d9b03a39b7c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Clinit.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Clinit.java @@ -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; diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompactConstructorDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompactConstructorDeclaration.java index 17e7d75f063..c2f8b6a347e 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompactConstructorDeclaration.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/CompactConstructorDeclaration.java @@ -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) { diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java index 4bd1828ab05..e32f0294d3c 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/ConstructorDeclaration.java @@ -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 { @@ -601,7 +603,6 @@ public void parseStatements(Parser parser, CompilationUnitDeclaration unit) { return; } parser.parse(this, unit, false); - this.containsSwitchWithTry = parser.switchWithTry; } @Override diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MethodDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MethodDeclaration.java index 8eb56abc74e..43001895853 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MethodDeclaration.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/MethodDeclaration.java @@ -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 diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java index dcae3ca09fc..e3dce374aa4 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java @@ -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; @@ -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) { diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TryStatement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TryStatement.java index e58ec2eb945..472b49e125a 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TryStatement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TryStatement.java @@ -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) { diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java index 25469cb4350..1168577e875 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/TypeDeclaration.java @@ -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$ diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java index d401dc77c90..ee6d2449770 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java @@ -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$ diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java index c524eb51013..78c0ae83cd7 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/codegen/CodeStream.java @@ -7730,6 +7730,26 @@ 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) { @@ -7737,10 +7757,6 @@ private void pushTypeBinding(int resolvedPosition) { 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() { @@ -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); } } diff --git a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java index f711f3c4c54..36ed00679ed 100644 --- a/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java +++ b/org.eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/parser/Parser.java @@ -888,7 +888,6 @@ protected int actFromTokenOrSynthetic(int previousAct) { // protected int casePtr; protected Map caseStartMap = new HashMap<>(); ASTNode [] noAstNodes = new ASTNode[AstStackIncrement]; - public boolean switchWithTry = false; Expression [] noExpressions = new Expression[ExpressionStackIncrement]; //modifiers dimensions nestedType etc....... @@ -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 @@ -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 @@ -9659,7 +9655,6 @@ protected void consumeSwitchExpression() { problemReporter().switchExpressionsNotSupported(s); } collectResultExpressionsYield(s); - this.switchWithTry |= s.containsTry; pushOnExpressionStack(s); } } @@ -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; @@ -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. @@ -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; @@ -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; diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java index c0d0538b431..71e6446c96a 100644 --- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java +++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java @@ -6862,4 +6862,241 @@ public static void main(String[] args) { }, "Entry = Hello"); } + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2228 + // [Switch-expression] Internal inconsistency warning at compile time & verify error at runtime + public void testIssue2228() { + if (this.complianceLevel < ClassFileConstants.JDK21) + return; + this.runConformTest( + new String[] { + "X.java", + """ + public class X { + int k; + void foo() { + new X() { + { + System.out.println ("Switch Expr = " + switch (X.this.k) { + default -> { + try { + yield throwing(); + } catch (NumberFormatException nfe) { + yield 10; + } + finally { + System.out.println("Finally"); + } + } + }); + } + + private Object throwing() { + throw new NumberFormatException(); + } + }; + } + + public static void main(String[] args) { + new X().foo(); + } + } + """ + }, + "Finally\n" + + "Switch Expr = 10"); + } + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2228 + // [Switch-expression] Internal inconsistency warning at compile time & verify error at runtime + public void testIssue2228_2() { + if (this.complianceLevel < ClassFileConstants.JDK21) + return; + this.runConformTest( + new String[] { + "X.java", + """ + public class X { + int k; + void foo() { + int fooLocal = 10; + class Local { + Local() { + System.out.println("Switch result = " + switch(X.this.k) { + default -> { + try { + System.out.println("Try"); + yield 10; + } catch (Exception e) { + System.out.println("Catch"); + yield 20; + } finally { + System.out.println("Finally"); + } + } + }); + } + } + new Local(); + } + public static void main(String[] args) { + new X().foo(); + } + } + """ + }, + "Try\n" + + "Finally\n" + + "Switch result = 10"); + } + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2228 + // [Switch-expression] Internal inconsistency warning at compile time & verify error at runtime + public void testIssue2228_3() { + if (this.complianceLevel < ClassFileConstants.JDK21) + return; + this.runConformTest( + new String[] { + "X.java", + """ + public class X { + + int k; + + { + System.out.println ("Switch Expr = " + switch (k) { + default -> { + try { + yield throwing(); + } catch (NumberFormatException nfe) { + yield 10; + } + finally { + System.out.println("Finally"); + } + } + }); + } + private Object throwing() { + throw new NumberFormatException(); + } + public static void main(String[] args) { + new X(); + } + } + """ + }, + "Finally\n" + + "Switch Expr = 10"); + } + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2228 + // [Switch-expression] Internal inconsistency warning at compile time & verify error at runtime + public void testIssue2228_4() { + if (this.complianceLevel < ClassFileConstants.JDK21) + return; + this.runConformTest( + new String[] { + "X.java", + """ + public class X { + public static void main(String[] args) { + args = new String [] { "one" , "two"}; + System.out.println(switch (args) { + case null -> 0; + default -> switch(args.length) { + case 0 -> 0; + case 1 -> "One"; + default -> new X(); + }; + }); + } + public String toString() { + return "some X()"; + } + } + """ + }, + "some X()"); + } + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2228 + // [Switch-expression] Internal inconsistency warning at compile time & verify error at runtime + public void testIssue2228_5() { + if (this.complianceLevel < ClassFileConstants.JDK21) + return; + this.runConformTest( + new String[] { + "X.java", + """ + public class X { + int k; + void foo() { + String val = "123"; + new X() { + { + System.out.println ("Switch Expr = " + switch (X.this.k) { + default -> { + try { + yield throwing(); + } catch (NumberFormatException nfe) { + yield val; + } + finally { + System.out.println("Finally"); + } + } + }); + } + + private Object throwing() { + throw new NumberFormatException(); + } + }; + } + + public static void main(String[] args) { + new X().foo(); + } + } + """ + }, + "Finally\n" + + "Switch Expr = 123"); + } + // https://github.com/eclipse-jdt/eclipse.jdt.core/issues/2228 + // [Switch-expression] Internal inconsistency warning at compile time & verify error at runtime + public void testIssue2228_6() { + if (this.complianceLevel < ClassFileConstants.JDK21) + return; + this.runConformTest( + new String[] { + "X.java", + """ + public class X { + + static int k; + + static { + System.out.println ("Switch Expr = " + switch (k) { + default -> { + try { + yield throwing(); + } catch (NumberFormatException nfe) { + yield 10; + } + finally { + System.out.println("Finally"); + } + } + }); + } + private static Object throwing() { + throw new NumberFormatException(); + } + public static void main(String[] args) { + new X(); + } + } + """ + }, + "Finally\n" + + "Switch Expr = 10"); + } + }