From ffe61834bf8b83fb3d276b5e4c1f9bdf0af4e9c2 Mon Sep 17 00:00:00 2001 From: Colin Alworth Date: Wed, 29 Nov 2023 16:15:32 -0600 Subject: [PATCH] Handle ternary intersection types (#9799) 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 --- .../src/com/google/gwt/dev/javac/JdtUtil.java | 2 +- .../gwt/dev/jjs/impl/GwtAstBuilder.java | 38 ++++++++++++++++++- user/src/com/google/gwt/junit/JUnitShell.java | 4 +- .../google/gwt/dev/jjs/test/Java10Test.java | 27 ++++++++++++- .../google/gwt/dev/jjs/test/CompilerTest.java | 3 ++ .../google/gwt/dev/jjs/test/Java10Test.java | 4 ++ 6 files changed, 72 insertions(+), 6 deletions(-) diff --git a/dev/core/src/com/google/gwt/dev/javac/JdtUtil.java b/dev/core/src/com/google/gwt/dev/javac/JdtUtil.java index 4091260de7a..911169e53e0 100644 --- a/dev/core/src/com/google/gwt/dev/javac/JdtUtil.java +++ b/dev/core/src/com/google/gwt/dev/javac/JdtUtil.java @@ -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 { diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java index 240860bee06..58334327c2b 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GwtAstBuilder.java @@ -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); @@ -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. + *

+ * 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 { @@ -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; diff --git a/user/src/com/google/gwt/junit/JUnitShell.java b/user/src/com/google/gwt/junit/JUnitShell.java index 733e54413cf..a11b1452834 100644 --- a/user/src/com/google/gwt/junit/JUnitShell.java +++ b/user/src/com/google/gwt/junit/JUnitShell.java @@ -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; @@ -1092,7 +1091,8 @@ void compileForWebMode(ModuleDef module, Set 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(); diff --git a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java10Test.java b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java10Test.java index 295da178631..165815343b8 100644 --- a/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java10Test.java +++ b/user/test-super/com/google/gwt/dev/jjs/super/com/google/gwt/dev/jjs/test/Java10Test.java @@ -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. @@ -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 supplier = () -> s; diff --git a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java index f9bb6b92cd7..ad1f54285e1 100644 --- a/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java +++ b/user/test/com/google/gwt/dev/jjs/test/CompilerTest.java @@ -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')); } /** diff --git a/user/test/com/google/gwt/dev/jjs/test/Java10Test.java b/user/test/com/google/gwt/dev/jjs/test/Java10Test.java index 1f22f3c5a91..07b6bd72ca8 100644 --- a/user/test/com/google/gwt/dev/jjs/test/Java10Test.java +++ b/user/test/com/google/gwt/dev/jjs/test/Java10Test.java @@ -50,6 +50,10 @@ public void testLocalVarType_Anonymous() { assertFalse(isGwtSourceLevel10()); } + public void testLocalVarType_Ternary() { + assertFalse(isGwtSourceLevel10()); + } + public void testLocalVarType_LambdaCapture() { assertFalse(isGwtSourceLevel10()); }