Skip to content

Commit

Permalink
Handle ternary intersection types (#9799)
Browse files Browse the repository at this point in the history
Starting in GWT 2.9.0 (check jdt vers), JDT changed how it emitted
ternary expressions to result in intersection casts in at least two
cases:
 * When concatenating the result of the ternary to a string
 * When assigning a local field typed with `var`.

This error only previously happened when assertions were enabled - the
existing code seems to behave more or less as expected.

This fix allows for those two cases to use intersection types, but since
the GWT AST doesn't have support for intersection types, the fix selects
the first non-java.lang.Object type in the list of types. This is
consistent with what we used to do here, but by changing these specific
cases rather than altering JdtUtil.signature() we only permit
intersection types in these specific cases.

It is technically possible for a JDT WildcardBinding to show up as an
intersection cast, but because JdtUtil.signature() already calls
erasure() on the TypeBinding, we will never fail the assertion in this
way.

JUnitShell now will log context of where a compilation error occurred,
and the specific assertion that previously failed will also log the
encountered type, to aid future debugging in this area.

Fixes #9694
  • Loading branch information
niloc132 committed Nov 29, 2023
1 parent dd81ecf commit ffe6183
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 6 deletions.
2 changes: 1 addition & 1 deletion dev/core/src/com/google/gwt/dev/javac/JdtUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ public static String signature(MethodBinding binding) {
}

public static String signature(TypeBinding binding) {
assert !binding.isIntersectionType18() && !binding.isIntersectionType();
assert !binding.isIntersectionType18() && !binding.isIntersectionType() : binding.debugName();
if (binding.isBaseType()) {
return String.valueOf(binding.sourceName());
} else {
Expand Down
38 changes: 36 additions & 2 deletions dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,14 @@ public void endVisit(CompoundAssignment x, BlockScope scope) {
public void endVisit(ConditionalExpression x, BlockScope scope) {
try {
SourceInfo info = makeSourceInfo(x);
JType type = typeMap.get(x.resolvedType);
JType type;
if (x.resolvedType instanceof IntersectionTypeBinding18) {
type = typeMap.get(
getFirstNonObjectInIntersection((IntersectionTypeBinding18) x.resolvedType)
);
} else {
type = typeMap.get(x.resolvedType);
}
JExpression valueIfFalse = pop(x.valueIfFalse);
JExpression valueIfTrue = pop(x.valueIfTrue);
JExpression condition = pop(x.condition);
Expand All @@ -648,6 +655,27 @@ public void endVisit(ConditionalExpression x, BlockScope scope) {
}
}

/**
* Returns the first non-Object type in the intersection. As intersections can only contain one
* class, and that class must be first, this ensures that if there is a class it will be the
* returned type, but if there are only interfaces, the first interface will be selected.
* <p></p>
* This behavior is consistent with ReferenceMapper.get() with assertions disabled - that is,
* where {@code referenceMapper.get(foo)} would fail due to an assertion, if assertions are
* disabled then {@code
* referenceMapper.get(foo).equals(referenceMapper.get(getFirstNonObjectInIntersection(foo))
* } will be true.
*/
private TypeBinding getFirstNonObjectInIntersection(IntersectionTypeBinding18 resolvedType) {
for (ReferenceBinding type : resolvedType.intersectingTypes) {
if (type != curCud.cud.scope.getJavaLangObject()) {
return type;
}
}
throw new IllegalStateException("Type doesn't have a non-java.lang.Object it intersects "
+ resolvedType);
}

@Override
public void endVisit(ConstructorDeclaration x, ClassScope scope) {
try {
Expand Down Expand Up @@ -2941,7 +2969,13 @@ private JLocal createLocal(LocalDeclaration x) {
TypeBinding resolvedType = x.type.resolvedType;
JType localType;
if (resolvedType.constantPoolName() != null) {
localType = typeMap.get(resolvedType);
if (resolvedType instanceof IntersectionTypeBinding18) {
localType = typeMap.get(
getFirstNonObjectInIntersection((IntersectionTypeBinding18) resolvedType)
);
} else {
localType = typeMap.get(resolvedType);
}
} else {
// Special case, a statically unreachable local type.
localType = JReferenceType.NULL_TYPE;
Expand Down
4 changes: 2 additions & 2 deletions user/src/com/google/gwt/junit/JUnitShell.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

import com.google.gwt.core.ext.Linker;
import com.google.gwt.core.ext.TreeLogger;
import com.google.gwt.core.ext.TreeLogger.Type;
import com.google.gwt.core.ext.UnableToCompleteException;
import com.google.gwt.core.ext.linker.impl.StandardLinkerContext;
import com.google.gwt.core.ext.typeinfo.JClassType;
Expand Down Expand Up @@ -1092,7 +1091,8 @@ void compileForWebMode(ModuleDef module, Set<String> userAgents)
try {
success = Compiler.compile(getTopLogger(), options, module);
} catch (Exception e) {
getTopLogger().log(Type.ERROR, "Compiler aborted with an exception ", e);
// noinspection ThrowableNotThrown
CompilationProblemReporter.logAndTranslateException(getTopLogger(), e);
}
if (!success) {
throw new UnableToCompleteException();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@
import java.util.ArrayList;
import java.util.function.Supplier;

import java.io.Serializable;

/**
* Tests Java 10 features. It is super sourced so that gwt can be compiles under Java 7.
* Tests Java 10 features. It is super sourced so that gwt can be compiled under Java 7.
*
* IMPORTANT: For each test here there must exist the corresponding method in the non super sourced
* version.
Expand Down Expand Up @@ -65,6 +67,29 @@ public void testLocalVarType_Anonymous() {
assertEquals("42", o.s);
}

public void testLocalVarType_Ternary() {
var value = true ? "a" : 'c';
checkSerializableDispatch(value);
checkComparableDispatch(value);
assertEquals("a", value.toString());
}

private void checkSerializableDispatch(Object fail) {
fail("should not be treated as object");
}

private void checkSerializableDispatch(Serializable pass) {
// pass
}

private void checkComparableDispatch(Object fail) {
fail("should not be treated as object");
}

private void checkComparableDispatch(Comparable<?> pass) {
// pass
}

public void testLocalVarType_LambdaCapture() {
var s = "42";
Supplier<String> supplier = () -> s;
Expand Down
3 changes: 3 additions & 0 deletions user/test/com/google/gwt/dev/jjs/test/CompilerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,9 @@ public void testConditionals() {
assertFalse(FALSE ? TRUE : false);
assertFalse(TRUE ? FALSE : true);
assertTrue(FALSE ? FALSE : true);

assertEquals("abc", "ab" + (TRUE ? "c" : 'z'));
assertEquals("abz", "ab" + (FALSE ? "c" : 'z'));
}

/**
Expand Down
4 changes: 4 additions & 0 deletions user/test/com/google/gwt/dev/jjs/test/Java10Test.java
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ public void testLocalVarType_Anonymous() {
assertFalse(isGwtSourceLevel10());
}

public void testLocalVarType_Ternary() {
assertFalse(isGwtSourceLevel10());
}

public void testLocalVarType_LambdaCapture() {
assertFalse(isGwtSourceLevel10());
}
Expand Down

0 comments on commit ffe6183

Please sign in to comment.