Bug 562101 - [14] Wrong bytecode when switch expression is embedded
inside a pattern matching instanceof statement
Change-Id: I3881a7708981e0c915ec970d0e05e0e8a2265bc2
Signed-off-by: Manoj Palat <manpalat@in.ibm.com>
Also-by: Jay Arthanareeswaran <jarthana@in.ibm.com>
diff --git a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PatternMatching14Test.java b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PatternMatching14Test.java
index 016414f..7473aaf 100644
--- a/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PatternMatching14Test.java
+++ b/org.eclipse.jdt.core.tests.compiler/src/org/eclipse/jdt/core/tests/compiler/regression/PatternMatching14Test.java
@@ -1290,6 +1290,27 @@
"one",
options);
}
+ public void test024b() {
+ Map<String, String> options = getCompilerOptions(true);
+ runConformTest(
+ new String[] {
+ "X24a.java",
+ "@SuppressWarnings(\"preview\")\n" +
+ "public class X24a {\n" +
+ " public static void main(String[] o) {\n" +
+ " foo(\"one\");\n" +
+ " }\n" +
+ " public static void foo(Object o) {\n" +
+ " for (;!(o instanceof String s);) {\n" +
+ " throw new IllegalArgumentException();\n" +
+ " }\n" +
+ " System.out.println(s);\n" +
+ " }\n" +
+ "}\n",
+ },
+ "one",
+ options);
+ }
/*
* It's not a problem to define the same var in two operands of a binary expression,
* but then it is not in scope below.
@@ -1695,7 +1716,7 @@
/*
* Failing with VerifyError
*/
- public void _test039() {
+ public void test039() {
runConformTest(
new String[] {
"X39.java",
@@ -1984,7 +2005,7 @@
compilerOptions);
compilerOptions.put(CompilerOptions.OPTION_PreserveUnusedLocal, old);
}
- public void _test051() {
+ public void test051() {
Map<String, String> compilerOptions = getCompilerOptions(true);
String old = compilerOptions.get(CompilerOptions.OPTION_PreserveUnusedLocal);
compilerOptions.put(CompilerOptions.OPTION_PreserveUnusedLocal, CompilerOptions.OPTIMIZE_OUT);
@@ -2583,4 +2604,71 @@
true,
compilerOptions);
}
+ public void test059() {
+ Map<String, String> compilerOptions = getCompilerOptions(true);
+ runConformTest(
+ new String[] {
+ "X.java",
+ "public class X {\n"+
+ " static int count;\n"+
+ " public static void main(String[] args) {\n"+
+ " int i = 10;\n"+
+ " if (foo() instanceof String s) {\n"+
+ " ++i;\n"+
+ " }\n"+
+ " System.out.println(\"count:\"+X.count+\" i:\"+i);\n"+
+ " }\n"+
+ " public static Object foo() {\n"+
+ " ++X.count;\n"+
+ " return new Object();\n"+
+ " } \n"+
+ "}",
+ },
+ "count:1 i:10",
+ compilerOptions);
+ }
+ public void test060() {
+ Map<String, String> compilerOptions = getCompilerOptions(true);
+ runConformTest(
+ new String[] {
+ "X.java",
+ "public class X {\n"+
+ " static int count;\n"+
+ " public static void main(String[] args) {\n"+
+ " int i = 10;\n"+
+ " if (foo() instanceof String s) {\n"+
+ " ++i;\n"+
+ " }\n"+
+ " System.out.println(\"count:\"+X.count+\" i:\"+i);\n"+
+ " }\n"+
+ " public static Object foo() {\n"+
+ " ++X.count;\n"+
+ " return new String(\"hello\");\n"+
+ " } \n"+
+ "}",
+ },
+ "count:1 i:11",
+ compilerOptions);
+ }
+ public void test061() {
+ Map<String, String> compilerOptions = getCompilerOptions(true);
+ runConformTest(
+ new String[] {
+ "X.java",
+ "public class X {\n"+
+ " static int count;\n"+
+ " static String STR = \"FAIL\";\n"+
+ " @SuppressWarnings(\"preview\")\n"+
+ " public static void main(String[] args) {\n"+
+ " if ( switch(STR) {\n"+
+ " default -> \"PASS\";\n"+
+ " } instanceof String s) {\n"+
+ " System.out.println(s);\n"+
+ " }\n"+
+ " }\n"+
+ "}",
+ },
+ "PASS",
+ compilerOptions);
+ }
}
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BinaryExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BinaryExpression.java
index 4a38f32..6736fcd 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BinaryExpression.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/BinaryExpression.java
@@ -1812,9 +1812,9 @@
}
@Override
-public void initializePatternVariables(BlockScope scope, CodeStream codeStream) {
- this.left.initializePatternVariables(scope, codeStream);
- this.right.initializePatternVariables(scope, codeStream);
+public void addPatternVariables(BlockScope scope, CodeStream codeStream) {
+ this.left.addPatternVariables(scope, codeStream);
+ this.right.addPatternVariables(scope, codeStream);
}
@Override
public boolean containsPatternVariable() {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java
index 3f9d3cf..1685e69 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ConditionalExpression.java
@@ -448,10 +448,10 @@
}
@Override
- public void initializePatternVariables(BlockScope scope, CodeStream codeStream) {
- this.condition.initializePatternVariables(scope, codeStream);
- this.valueIfTrue.initializePatternVariables(scope, codeStream);
- this.valueIfFalse.initializePatternVariables(scope, codeStream);
+ public void addPatternVariables(BlockScope scope, CodeStream codeStream) {
+ this.condition.addPatternVariables(scope, codeStream);
+ this.valueIfTrue.addPatternVariables(scope, codeStream);
+ this.valueIfFalse.addPatternVariables(scope, codeStream);
}
@Override
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java
index 0828b50..0192774 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/Expression.java
@@ -785,7 +785,7 @@
throw new ShouldNotImplement(Messages.ast_missingCode);
}
}
-public void initializePatternVariables(BlockScope scope, CodeStream codeStream) {
+public void addPatternVariables(BlockScope scope, CodeStream codeStream) {
// Nothing by default
}
public boolean containsPatternVariable() {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FieldDeclaration.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FieldDeclaration.java
index c38ba90..94e6096 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FieldDeclaration.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/FieldDeclaration.java
@@ -141,17 +141,17 @@
}
// The fields escape CodeStream#exitUserScope(), and as a result end PC wouldn't be set.
// Set this explicitly (unlike a local declaration)
- if (this.initialization != null && this.initialization.containsPatternVariable()) {
- this.initialization.traverse(new ASTVisitor() {
- @Override
- public boolean visit(
- InstanceOfExpression instanceOfExpression,
- BlockScope scope) {
- instanceOfExpression.elementVariable.binding.recordInitializationEndPC(codeStream.position);
- return true;
- }
- }, currentScope);
- }
+// if (this.initialization != null && this.initialization.containsPatternVariable()) {
+// this.initialization.traverse(new ASTVisitor() {
+// @Override
+// public boolean visit(
+// InstanceOfExpression instanceOfExpression,
+// BlockScope scope) {
+// instanceOfExpression.elementVariable.binding.recordInitializationEndPC(codeStream.position);
+// return true;
+// }
+// }, currentScope);
+// }
codeStream.recordPositionsFrom(pc, this.sourceStart);
}
public void getAllAnnotationContexts(int targetType, List allAnnotationContexts) {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForStatement.java
index fc83729..8e059b8 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForStatement.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/ForStatement.java
@@ -266,7 +266,7 @@
}
}
if (this.condition != null && this.condition.containsPatternVariable()) {
- this.condition.initializePatternVariables(currentScope, codeStream);
+ this.condition.addPatternVariables(currentScope, codeStream);
}
Constant cst = this.condition == null ? null : this.condition.optimizedBooleanConstant();
boolean isConditionOptimizedFalse = cst != null && (cst != Constant.NotAConstant && cst.booleanValue() == false);
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java
index 4ce3b69..418fd90 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/InstanceOfExpression.java
@@ -39,7 +39,9 @@
public Expression expression;
public TypeReference type;
public LocalDeclaration elementVariable;
- boolean isInitialized;
+ static final char[] SECRET_INSTANCEOF_PATTERN_EXPRESSION_VALUE = " instanceOfPatternExpressionValue".toCharArray(); //$NON-NLS-1$
+
+ public LocalVariableBinding secretInstanceOfPatternExpressionValue = null;
public InstanceOfExpression(Expression expression, TypeReference type) {
this.expression = expression;
@@ -106,18 +108,28 @@
*/
@Override
public void generateCode(BlockScope currentScope, CodeStream codeStream, boolean valueRequired) {
- initializePatternVariables(currentScope, codeStream);
+ addPatternVariables(currentScope, codeStream);
int pc = codeStream.position;
- this.expression.generateCode(currentScope, codeStream, true);
+
+ if (this.elementVariable != null) {
+ addAssignment(currentScope, codeStream, this.secretInstanceOfPatternExpressionValue);
+ codeStream.load(this.secretInstanceOfPatternExpressionValue);
+ } else {
+ this.expression.generateCode(currentScope, codeStream, true);
+ }
+
codeStream.instance_of(this.type, this.type.resolvedType);
if (this.elementVariable != null) {
BranchLabel actionLabel = new BranchLabel(codeStream);
codeStream.dup();
codeStream.ifeq(actionLabel);
- this.expression.generateCode(currentScope, codeStream, true);
+ codeStream.load(this.secretInstanceOfPatternExpressionValue);
+ codeStream.removeVariable(this.secretInstanceOfPatternExpressionValue);
codeStream.checkcast(this.type, this.type.resolvedType, codeStream.position);
+ this.elementVariable.binding.recordInitializationStartPC(codeStream.position);
codeStream.store(this.elementVariable.binding, false);
+ codeStream.removeVariable(this.elementVariable.binding);
codeStream.recordPositionsFrom(codeStream.position, this.sourceEnd);
actionLabel.place();
}
@@ -128,6 +140,103 @@
}
codeStream.recordPositionsFrom(pc, this.sourceStart);
}
+@Override
+public void generateOptimizedBoolean(BlockScope currentScope, CodeStream codeStream, BranchLabel trueLabel, BranchLabel falseLabel, boolean valueRequired) {
+ // a label valued to nil means: by default we fall through the case...
+ // both nil means we leave the value on the stack
+
+ if (this.elementVariable == null) {
+ super.generateOptimizedBoolean(currentScope, codeStream, trueLabel, falseLabel, valueRequired);
+ return;
+ }
+ Constant cst = optimizedBooleanConstant();
+ addPatternVariables(currentScope, codeStream);
+
+ int pc = codeStream.position;
+
+ addAssignment(currentScope, codeStream, this.secretInstanceOfPatternExpressionValue);
+ codeStream.load(this.secretInstanceOfPatternExpressionValue);
+
+ BranchLabel nextSibling = falseLabel != null ? falseLabel : new BranchLabel(codeStream);
+ codeStream.instance_of(this.type, this.type.resolvedType);
+ if (this.elementVariable != null) {
+ codeStream.ifeq(nextSibling);
+ codeStream.load(this.secretInstanceOfPatternExpressionValue);
+ codeStream.checkcast(this.type, this.type.resolvedType, codeStream.position);
+ codeStream.dup();
+ codeStream.store(this.elementVariable.binding, false);
+
+ codeStream.load(this.secretInstanceOfPatternExpressionValue);
+ codeStream.removeVariable(this.secretInstanceOfPatternExpressionValue);
+ codeStream.checkcast(this.type, this.type.resolvedType, codeStream.position);
+ }
+ if (valueRequired && cst == Constant.NotAConstant) {
+ codeStream.generateImplicitConversion(this.implicitConversion);
+ } else {
+ codeStream.pop();
+ }
+ codeStream.recordPositionsFrom(pc, this.sourceStart);
+
+
+ if ((cst != Constant.NotAConstant) && (cst.typeID() == TypeIds.T_boolean)) {
+ pc = codeStream.position;
+ if (cst.booleanValue() == true) {
+ // constant == true
+ if (valueRequired) {
+ if (falseLabel == null) {
+ // implicit falling through the FALSE case
+ if (trueLabel != null) {
+ codeStream.goto_(trueLabel);
+ }
+ }
+ }
+ } else {
+ if (valueRequired) {
+ if (falseLabel != null) {
+ // implicit falling through the TRUE case
+ if (trueLabel == null) {
+ codeStream.goto_(falseLabel);
+ }
+ }
+ }
+ }
+ codeStream.recordPositionsFrom(pc, this.sourceStart);
+ } else {
+ // branching
+ int position = codeStream.position;
+ if (valueRequired) {
+ if (falseLabel == null) {
+ if (trueLabel != null) {
+ // Implicit falling through the FALSE case
+ codeStream.if_acmpeq(trueLabel);
+ }
+ } else {
+ if (trueLabel == null) {
+ // Implicit falling through the TRUE case
+ codeStream.if_acmpne(falseLabel);
+ } else {
+ // No implicit fall through TRUE/FALSE --> should never occur
+ }
+ }
+ }
+ codeStream.recordPositionsFrom(position, this.sourceEnd);
+ }
+ if (nextSibling != falseLabel)
+ nextSibling.place();
+}
+
+private void addAssignment(BlockScope currentScope, CodeStream codeStream, LocalVariableBinding local) {
+ assert local != null;
+ SingleNameReference lhs = new SingleNameReference(local.name, 0);
+ lhs.binding = local;
+ lhs.bits &= ~ASTNode.RestrictiveFlagMASK; // clear bits
+ lhs.bits |= Binding.LOCAL;
+ lhs.bits |= ASTNode.IsSecretYieldValueUsage;
+ ((LocalVariableBinding) lhs.binding).markReferenced(); // TODO : Can be skipped?
+ Assignment assignment = new Assignment(lhs, this.expression, 0);
+ assignment.generateCode(currentScope, codeStream);
+ codeStream.addVariable(this.secretInstanceOfPatternExpressionValue);
+}
@Override
public StringBuffer printExpressionNoParenthesis(int indent, StringBuffer output) {
@@ -136,16 +245,9 @@
}
@Override
-public void initializePatternVariables(BlockScope currentScope, CodeStream codeStream) {
+public void addPatternVariables(BlockScope currentScope, CodeStream codeStream) {
if (this.elementVariable != null) {
- if(!this.isInitialized) {
- this.isInitialized = true;
- codeStream.aconst_null();
- codeStream.store(this.elementVariable.binding, false);
- }
- int position = codeStream.position;
codeStream.addVisibleLocalVariable(this.elementVariable.binding);
- this.elementVariable.binding.recordInitializationStartPC(position);
}
}
public void resolvePatternVariable(BlockScope scope) {
@@ -161,9 +263,25 @@
public boolean containsPatternVariable() {
return this.elementVariable != null;
}
+private void addSecretInstanceOfPatternExpressionValue(BlockScope scope1) {
+ LocalVariableBinding local =
+ new LocalVariableBinding(
+ InstanceOfExpression.SECRET_INSTANCEOF_PATTERN_EXPRESSION_VALUE,
+ TypeBinding.wellKnownType(scope1, T_JavaLangObject),
+ ClassFileConstants.AccDefault,
+ false);
+ local.setConstant(Constant.NotAConstant);
+ local.useFlag = LocalVariableBinding.USED;
+ local.declaration = new LocalDeclaration(InstanceOfExpression.SECRET_INSTANCEOF_PATTERN_EXPRESSION_VALUE, 0, 0);
+ scope1.addLocalVariable(local);
+ this.secretInstanceOfPatternExpressionValue = local;
+}
+
@Override
public TypeBinding resolveType(BlockScope scope) {
this.constant = Constant.NotAConstant;
+ if (this.elementVariable != null)
+ addSecretInstanceOfPatternExpressionValue(scope);
resolvePatternVariable(scope);
TypeBinding checkedType = this.type.resolveType(scope, true /* check bounds*/);
if (this.expression instanceof CastExpression) {
@@ -178,7 +296,10 @@
if (expressionType == null || checkedType == null)
return null;
- if (!checkedType.isReifiable()) {
+ if (this.secretInstanceOfPatternExpressionValue != null && expressionType != TypeBinding.NULL)
+ this.secretInstanceOfPatternExpressionValue.type = expressionType;
+
+ if (!checkedType.isReifiable()) {
CompilerOptions options = scope.compilerOptions();
// If preview is disabled, report same as before, even at Java 14
if (options.complianceLevel < ClassFileConstants.JDK14 || !options.enablePreviewFeatures) {
diff --git a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java
index 3863cde..a02b59d 100644
--- a/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java
+++ b/org.eclipse.jdt.core/compiler/org/eclipse/jdt/internal/compiler/ast/WhileStatement.java
@@ -193,7 +193,7 @@
return;
}
if (this.condition != null && this.condition.containsPatternVariable()) {
- this.condition.initializePatternVariables(currentScope, codeStream);
+ this.condition.addPatternVariables(currentScope, codeStream);
}
int pc = codeStream.position;
Constant cst = this.condition.optimizedBooleanConstant();
