Error reporting for UnifyAst. Report errors cleanly and don't blow up. http://gwt-code-reviews.appspot.com/1451814/ Review by: zundel@google.com git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10314 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/CompileTaskRunner.java b/dev/core/src/com/google/gwt/dev/CompileTaskRunner.java index 60ba4ba..7c65f6e 100644 --- a/dev/core/src/com/google/gwt/dev/CompileTaskRunner.java +++ b/dev/core/src/com/google/gwt/dev/CompileTaskRunner.java
@@ -17,6 +17,7 @@ import com.google.gwt.core.ext.TreeLogger; import com.google.gwt.core.ext.UnableToCompleteException; +import com.google.gwt.dev.javac.CompilationProblemReporter; import com.google.gwt.dev.shell.log.SwingLoggerPanel; import com.google.gwt.dev.util.log.PrintWriterTreeLogger; @@ -88,7 +89,7 @@ } catch (UnableToCompleteException e) { // Assume logged. } catch (Throwable e) { - logger.log(TreeLogger.ERROR, "Unexpected", e); + CompilationProblemReporter.logAndTranslateException(logger, e); } return false; }
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java index b29358a..fff563d 100644 --- a/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java +++ b/dev/core/src/com/google/gwt/dev/jjs/impl/UnifyAst.java
@@ -17,6 +17,7 @@ import com.google.gwt.core.ext.TreeLogger; import com.google.gwt.core.ext.UnableToCompleteException; +import com.google.gwt.dev.javac.CompilationProblemReporter; import com.google.gwt.dev.javac.CompilationUnit; import com.google.gwt.dev.javac.CompiledClass; import com.google.gwt.dev.jdt.RebindPermutationOracle; @@ -70,6 +71,7 @@ import com.google.gwt.dev.js.ast.JsProgram; import com.google.gwt.dev.js.ast.JsRootScope; import com.google.gwt.dev.util.JsniRef; +import com.google.gwt.dev.util.Name; import com.google.gwt.dev.util.Name.BinaryName; import com.google.gwt.dev.util.Name.InternalName; import com.google.gwt.dev.util.collect.IdentityHashSet; @@ -189,7 +191,7 @@ @Override public void endVisit(JExpression x, Context ctx) { - assert !x.getType().isExternal(); + assert !x.getType().isExternal() || errorsFound; } @Override @@ -226,7 +228,10 @@ public void endVisit(JMethodCall x, Context ctx) { // Already resolved during visit(). JMethod target = x.getTarget(); - assert !target.isExternal(); + if (target.isExternal()) { + assert errorsFound; + return; + } if (magicMethodCalls.contains(target)) { JExpression result = handleMagicMethodCall(x); if (result == null) { @@ -399,7 +404,9 @@ JsniRef ref = JsniRef.parse(stringValue); if (ref != null) { - searchForType(ref.className()); + if (Name.isBinaryName(ref.className())) { + searchForType(ref.className()); + } node = JsniRefLookup.findJsniRefTarget(ref, program, new JsniRefLookup.ErrorReporter() { public void reportError(String errMsg) { error(x, errMsg); @@ -463,6 +470,7 @@ private final Map<String, CompiledClass> classFileMap; private boolean errorsFound = false; + private final Set<CompilationUnit> failedUnits = new IdentityHashSet<CompilationUnit>(); private final Map<String, JField> fieldMap = new HashMap<String, JField>(); /** @@ -518,7 +526,7 @@ } assimilateUnit(cc.getUnit()); type = program.getFromTypeMap(sourceTypeName); - assert type != null; + assert type != null || errorsFound; } } } @@ -650,8 +658,16 @@ } private void assimilateUnit(CompilationUnit unit) { - // TODO: error checking. + if (unit.isError()) { + if (failedUnits.add(unit)) { + CompilationProblemReporter.reportErrors(logger, unit, false); + errorsFound = true; + } + return; + } + // TODO(zundel): ask for a recompile if deserialization fails? List<JDeclaredType> types = unit.getTypes(); + assert containsAllTypes(unit, types); for (JDeclaredType t : types) { program.addType(t); } @@ -712,6 +728,19 @@ } } + private boolean containsAllTypes(CompilationUnit unit, List<JDeclaredType> types) { + Set<String> binaryTypeNames = new HashSet<String>(); + for (JDeclaredType type : types) { + binaryTypeNames.add(type.getName()); + } + for (CompiledClass cc : unit.getCompiledClasses()) { + if (!binaryTypeNames.contains(InternalName.toBinaryName(cc.getInternalName()))) { + return false; + } + } + return true; + } + private void error(JNode x, String errorMessage) { errorsFound = true; TreeLogger branch = @@ -730,7 +759,11 @@ } private void flowInto(JField field) { - if (field == JField.NULL_FIELD || field.isExternal()) { + if (field.isExternal()) { + assert errorsFound; + return; + } + if (field == JField.NULL_FIELD) { return; } if (!liveFieldsAndMethods.contains(field)) { @@ -743,7 +776,11 @@ } private void flowInto(JMethod method) { - if (method == JMethod.NULL_METHOD || method.isExternal()) { + if (method.isExternal()) { + assert errorsFound; + return; + } + if (method == JMethod.NULL_METHOD) { return; } if (!liveFieldsAndMethods.contains(method)) { @@ -794,7 +831,10 @@ } private void instantiate(JDeclaredType type) { - assert !type.isExternal(); + if (type.isExternal()) { + assert errorsFound; + return; + } if (!instantiatedTypes.contains(type)) { instantiatedTypes.add(type); if (type.getSuperClass() != null) { @@ -909,13 +949,16 @@ } assimilateUnit(cc.getUnit()); type = program.getFromTypeMap(binaryTypeName); + assert type != null || errorsFound; } return type; } private void staticInitialize(JDeclaredType type) { - // Make sure the type is already resolved. - assert !type.isExternal(); + if (type.isExternal()) { + assert errorsFound; + return; + } JMethod clinit = type.getMethods().get(0); if (!liveFieldsAndMethods.contains(clinit)) { flowInto(clinit); @@ -953,13 +996,13 @@ String typeName = type.getName(); JDeclaredType newType = searchForType(typeName); - if (newType != null) { - assert !newType.isExternal(); - return newType; + if (newType == null) { + assert errorsFound; + return type; } - // Error condition, should be logged. - errorsFound = true; - return type; + + assert !newType.isExternal(); + return newType; } private JField translate(JField field) { @@ -976,7 +1019,7 @@ enclosingType = translate(enclosingType); if (enclosingType.isExternal()) { - // Error condition, should be logged. + assert errorsFound; return field; } mapApi(enclosingType); @@ -1006,7 +1049,7 @@ enclosingType = translate(enclosingType); if (enclosingType.isExternal()) { - // Error condition, should be logged. + assert errorsFound; return method; } mapApi(enclosingType);
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/AdditionalTypeProviderDelegateTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/AdditionalTypeProviderDelegateTest.java index 03c9120..1962d88 100644 --- a/dev/core/test/com/google/gwt/dev/jjs/impl/AdditionalTypeProviderDelegateTest.java +++ b/dev/core/test/com/google/gwt/dev/jjs/impl/AdditionalTypeProviderDelegateTest.java
@@ -123,40 +123,6 @@ return code; } }); - - // Create a source file containing a reference to an unknown class. - sourceOracle.addOrReplace(new MockJavaResource("test.C") { - @Override - public CharSequence getContent() { - StringBuffer code = new StringBuffer(); - code.append("package test;\n"); - code.append("import myPackage.UnknownClass;"); - code.append("class C {\n"); - code.append(" int func() {\n"); - // Reference an unknown class. - code.append(" return myPackage.UnknownClass.getSmallNumber();\n"); - code.append(" }\n"); - code.append("}\n"); - return code; - } - }); - - // Create a final example with a reference to an unknown class and no - // import statement. - sourceOracle.addOrReplace(new MockJavaResource("test.D") { - @Override - public CharSequence getContent() { - StringBuffer code = new StringBuffer(); - code.append("package test;\n"); - code.append("class D {\n"); - code.append(" int func() {\n"); - // Reference an unknown class. - code.append(" return myPackage.UnknownClass.getSmallNumber();\n"); - code.append(" }\n"); - code.append("}\n"); - return code; - } - }); } public void testInsertedClass() throws UnableToCompleteException { @@ -187,14 +153,46 @@ } public void testUnknownClass() { + // Create a source file containing a reference to an unknown class. + sourceOracle.addOrReplace(new MockJavaResource("test.C") { + @Override + public CharSequence getContent() { + StringBuffer code = new StringBuffer(); + code.append("package test;\n"); + code.append("import myPackage.UnknownClass;"); + code.append("class C {\n"); + code.append(" int func() {\n"); + // Reference an unknown class. + code.append(" return myPackage.UnknownClass.getSmallNumber();\n"); + code.append(" }\n"); + code.append("}\n"); + return code; + } + }); try { compileSnippet("void", "new test.C();"); fail("Shouldn't have compiled"); } catch (UnableToCompleteException expected) { } } - + public void testUnknownClassNoImport() { + // Create a source file with a reference to an unknown class and no + // import statement. + sourceOracle.addOrReplace(new MockJavaResource("test.D") { + @Override + public CharSequence getContent() { + StringBuffer code = new StringBuffer(); + code.append("package test;\n"); + code.append("class D {\n"); + code.append(" int func() {\n"); + // Reference an unknown class. + code.append(" return myPackage.UnknownClass.getSmallNumber();\n"); + code.append(" }\n"); + code.append("}\n"); + return code; + } + }); try { compileSnippet("void", "new test.D();"); fail("Shouldn't have compiled");