Skip to content

Commit

Permalink
Alternate fixes for https://bugs.eclipse.org/bugs/show_bug.cgi?id=576861
Browse files Browse the repository at this point in the history 
 (#3208)

* Also back out its follow on bug fix #520 as it loses relevance
  • Loading branch information
@srikanth-sankaran
srikanth-sankaran authored Nov 1, 2024
1 parent e8ce5b1 commit 02d9d0c
Show file tree
Hide file tree
Showing 6 changed files with 156 additions and 35 deletions.
4 changes: 4 additions & 0 deletions ...eclipse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/Expression.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1100,6 +1100,10 @@ public StringBuilder print(int indent, StringBuilder output) {

public abstract StringBuilder printExpression(int indent, StringBuilder output);

public StringBuilder printExpression(int tab, StringBuilder output, boolean makeShort) {
return printExpression(tab, output);
}

@Override
public StringBuilder printStatement(int indent, StringBuilder output) {
return print(indent, output).append(";"); //$NON-NLS-1$
Expand Down
1 change: 1 addition & 0 deletions ...e.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/LambdaExpression.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -734,6 +734,7 @@ public StringBuilder printExpression(int tab, StringBuilder output) {
return printExpression(tab, output, false);
}

@Override
public StringBuilder printExpression(int tab, StringBuilder output, boolean makeShort) {
int parenthesesCount = (this.bits & ASTNode.ParenthesizedMASK) >> ASTNode.ParenthesizedSHIFT;
String suffix = ""; //$NON-NLS-1$
Expand Down
47 changes: 28 additions & 19 deletions ...e.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/SwitchExpression.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,15 @@
import org.eclipse.jdt.internal.compiler.flow.FlowInfo;
import org.eclipse.jdt.internal.compiler.impl.CompilerOptions;
import org.eclipse.jdt.internal.compiler.impl.Constant;
import org.eclipse.jdt.internal.compiler.lookup.*;
import org.eclipse.jdt.internal.compiler.lookup.BlockScope;
import org.eclipse.jdt.internal.compiler.lookup.FieldBinding;
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.PolyTypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.Scope;
import org.eclipse.jdt.internal.compiler.lookup.TypeBinding;
import org.eclipse.jdt.internal.compiler.lookup.TypeIds;

public class SwitchExpression extends SwitchStatement implements IPolyExpression {

Expand All @@ -40,8 +48,6 @@ public class SwitchExpression extends SwitchStatement implements IPolyExpression
private boolean isPolyExpression = false;
private TypeBinding[] originalValueResultExpressionTypes;
private TypeBinding[] finalValueResultExpressionTypes;
/* package */ Map<Expression, TypeBinding> originalTypeMap = new HashMap<>();


private int nullStatus = FlowInfo.UNKNOWN;
public List<Expression> resultExpressions = new ArrayList<>(0);
Expand Down Expand Up @@ -330,9 +336,6 @@ public TypeBinding resolveType(BlockScope upperScope) {
if (this.constant != Constant.NotAConstant) {
this.constant = Constant.NotAConstant;

if (this.originalTypeMap == null)
this.originalTypeMap = new HashMap<>();

super.resolve(upperScope);

resultExpressionsCount = this.resultExpressions.size();
Expand All @@ -341,31 +344,26 @@ public TypeBinding resolveType(BlockScope upperScope) {
return this.resolvedType = null;
}

if (this.originalValueResultExpressionTypes == null) {
this.originalValueResultExpressionTypes = new TypeBinding[resultExpressionsCount];
this.finalValueResultExpressionTypes = new TypeBinding[resultExpressionsCount];
for (int i = 0; i < resultExpressionsCount; ++i) {
this.finalValueResultExpressionTypes[i] = this.originalValueResultExpressionTypes[i] =
this.resultExpressions.get(i).resolvedType;
}
this.originalValueResultExpressionTypes = new TypeBinding[resultExpressionsCount];
this.finalValueResultExpressionTypes = new TypeBinding[resultExpressionsCount];
for (int i = 0; i < resultExpressionsCount; ++i) {
this.finalValueResultExpressionTypes[i] = this.originalValueResultExpressionTypes[i] = this.resultExpressions.get(i).resolvedType;
}

if (isPolyExpression()) { //The type of a poly switch expression is the same as its target type.
if (this.expectedType == null || !this.expectedType.isProperType(true)) {
return new PolyTypeBinding(this);
}
return this.resolvedType = computeConversions(this.scope, this.expectedType) ? this.expectedType : null;
}
} else {
// re-resolving of poly expression:
} else { // re-resolving of poly expression:
resultExpressionsCount = this.resultExpressions.size();
if (resultExpressionsCount == 0)
return this.resolvedType = null; // error flagging would have been done during the earlier phase.
boolean yieldErrors = false;
for (int i = 0; i < resultExpressionsCount; i++) {
Expression resultExpr = this.resultExpressions.get(i);
TypeBinding origType = this.originalTypeMap.get(resultExpr);
// NB: if origType == null we assume that initial resolving failed hard, rendering re-resolving impossible
if (origType != null && origType.kind() == Binding.POLY_TYPE) {
if (resultExpr.isPolyExpression()) {
this.finalValueResultExpressionTypes[i] = this.originalValueResultExpressionTypes[i] =
resultExpr.resolveTypeExpecting(upperScope, this.expectedType);
}
Expand All @@ -374,7 +372,7 @@ public TypeBinding resolveType(BlockScope upperScope) {
}
if (yieldErrors)
return this.resolvedType = null;
this.resolvedType = computeConversions(this.scope, this.expectedType) ? this.expectedType : null;
return this.resolvedType = computeConversions(this.scope, this.expectedType) ? this.expectedType : null;
}

boolean uniformYield = true;
Expand Down Expand Up @@ -641,6 +639,17 @@ public boolean sIsMoreSpecific(TypeBinding s, TypeBinding t, Scope skope) {
}
return true;
}

@Override
public StringBuilder printExpression(int tab, StringBuilder output, boolean makeShort) {
if (!makeShort) {
return super.printExpression(tab, output);
} else {
printIndent(tab, output).append("switch ("); //$NON-NLS-1$
return this.expression.printExpression(0, output).append(") { ... }"); //$NON-NLS-1$
}
}

@Override
public TypeBinding expectedType() {
return this.expectedType;
Expand Down
4 changes: 1 addition & 3 deletions ...pse.jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/ast/YieldStatement.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,9 +222,7 @@ public void resolve(BlockScope scope) {
} else if (this.switchExpression == null) {
scope.problemReporter().yieldOutsideSwitchExpression(this);
}
TypeBinding type = this.expression.resolveType(scope);
if (this.switchExpression != null && type != null)
this.switchExpression.originalTypeMap.put(this.expression, type);
this.expression.resolveType(scope);
}

@Override
Expand Down
4 changes: 1 addition & 3 deletions ...jdt.core.compiler.batch/src/org/eclipse/jdt/internal/compiler/lookup/PolyTypeBinding.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.eclipse.jdt.internal.compiler.lookup;

import org.eclipse.jdt.internal.compiler.ast.Expression;
import org.eclipse.jdt.internal.compiler.ast.LambdaExpression;

public class PolyTypeBinding extends TypeBinding {

Expand Down Expand Up @@ -75,8 +74,7 @@ public char[] readableName() {

@Override
public char[] shortReadableName() {
return this.expression instanceof LambdaExpression ?
((LambdaExpression) this.expression).printExpression(0, new StringBuilder(), true).toString().toCharArray() : readableName();
return this.expression.printExpression(0, new StringBuilder(), true).toString().toCharArray();
}

@Override
Expand Down
131 changes: 121 additions & 10 deletions ...mpiler/src/org/eclipse/jdt/core/tests/compiler/regression/SwitchExpressionsYieldTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6021,7 +6021,7 @@ public void testBug566125_06() {
"1. ERROR in X.java (at line 7)\n" +
" boolean b = foo( switch(i+1) {\n" +
" ^^^\n" +
"The method foo(short) in the type X is not applicable for the arguments (double)\n" +
"The method foo(short) in the type X is not applicable for the arguments (switch ((i + 1)) { ... })\n" +
"----------\n" +
"2. ERROR in X.java (at line 9)\n" +
" default -> Double.valueOf(2.0d);\n" +
Expand Down Expand Up @@ -6057,7 +6057,7 @@ public void testBug566125_07() {
"1. ERROR in X.java (at line 7)\n" +
" boolean b = foo( switch(i+1) {\n" +
" ^^^\n" +
"The method foo(short) in the type X is not applicable for the arguments (double)\n" +
"The method foo(short) in the type X is not applicable for the arguments (switch ((i + 1)) { ... })\n" +
"----------\n" +
"2. ERROR in X.java (at line 9)\n" +
" default -> 2.0d;\n" +
Expand Down Expand Up @@ -6094,7 +6094,7 @@ public void testBug566125_08() {
"1. ERROR in X.java (at line 7)\n" +
" boolean b = foo( switch(i+1) {\n" +
" ^^^\n" +
"The method foo(short) in the type X is not applicable for the arguments (double)\n" +
"The method foo(short) in the type X is not applicable for the arguments (switch ((i + 1)) { ... })\n" +
"----------\n" +
"2. ERROR in X.java (at line 9)\n" +
" default : yield 2.0d;\n" +
Expand Down Expand Up @@ -6341,12 +6341,7 @@ public void testGH520() throws Exception {
"1. ERROR in X.java (at line 3)\n" +
" foo(switch (i) {\n" +
" ^^^\n" +
"The method foo(T) in the type X is not applicable for the arguments (switch (i) {\n" +
"case 0 ->\n" +
" m.call();\n" +
"default ->\n" +
" null;\n" +
"})\n" +
"The method foo(T) in the type X is not applicable for the arguments (switch (i) { ... })\n" +
"----------\n" +
"2. ERROR in X.java (at line 4)\n" +
" case 0 -> m.call();\n" +
Expand Down Expand Up @@ -8087,4 +8082,120 @@ public static void main(String[] args) {
"----------\n");
}

}
// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3205
// [Switch Expressions] ECJ accepts ambiguous method invocation involving switch expressions with poly type result expression
public void testIssue3205() {
this.runConformTest(
new String[] {
"X.java",
"""
interface I {
void foo();
}
interface J {
void foo();
}
public class X implements I, J {
public void foo() {
}
static void doit() {}
public static void foo(Object o, J j) {
System.out.println("Object");
}
public static void foo(String s, I i) {
System.out.println(s);
}
public static void main(String[] args) {
// Compiles with both ECJ and javac
foo("OK", () -> {});
// Compiles with both ECJ and javac
foo("OK", args == null ? () -> {} : () -> {});
// Compiles with both ECJ and javac
foo("OK", X::doit);
// Rejected by both ECJ and javac.
//foo("OK", new X());
// Rejected by javac, accepted by ECJ.
foo("OK", switch (0) {
case 0 -> () -> {};
default -> () -> {};
});
}
}
"""
},
"OK\nOK\nOK\nOK");
}

// https://github.com/eclipse-jdt/eclipse.jdt.core/issues/3205
// [Switch Expressions] ECJ accepts ambiguous method invocation involving switch expressions with poly type result expression
public void testIssue3205_2() {
this.runNegativeTest(
new String[] {
"X.java",
"""
interface I {
void foo();
}
interface J {
void foo();
}
public class X implements I, J {
public void foo() {
}
static void doit() {}
public static void foo(Object o, J j) {
System.out.println("Object");
}
public static void foo(String s, I i) {
System.out.println(s);
}
public static void main(String[] args) {
// Compiles with both ECJ and javac
foo("OK", () -> {});
// Compiles with both ECJ and javac
foo("OK", args == null ? () -> {} : () -> {});
// Compiles with both ECJ and javac
foo("OK", X::doit);
// Rejected by both ECJ and javac.
foo("OK", new X());
// Rejected by javac, accepted by ECJ.
foo("OK", switch (0) {
case 0 -> () -> {};
default -> () -> {};
});
}
}
"""
},
"----------\n" +
"1. ERROR in X.java (at line 36)\n" +
" foo(\"OK\", new X());\n" +
" ^^^\n" +
"The method foo(Object, J) is ambiguous for the type X\n" +
"----------\n");
}
}

0 comments on commit 02d9d0c

Please sign in to comment.