A new model for external types
http://gwt-code-reviews.appspot.com/589801/show
Patch by: tobyr
Review by: me, bobv
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@8255 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ArtificialRescueRecorder.java b/dev/core/src/com/google/gwt/dev/jjs/ArtificialRescueRecorder.java
index 5d3e9bd..7720935 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ArtificialRescueRecorder.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ArtificialRescueRecorder.java
@@ -22,6 +22,7 @@
import com.google.gwt.dev.jjs.ast.JAnnotation;
import com.google.gwt.dev.jjs.ast.JDeclaredType;
import com.google.gwt.dev.jjs.ast.JField;
+import com.google.gwt.dev.jjs.ast.JInterfaceType;
import com.google.gwt.dev.jjs.ast.JNode;
import com.google.gwt.dev.jjs.ast.JProgram;
import com.google.gwt.dev.jjs.ast.JReferenceType;
@@ -124,12 +125,12 @@
new ArtificialRescueRecorder(program).execImpl();
}
- private final JDeclaredType artificialRescueType;
+ private final JInterfaceType artificialRescueType;
private final JProgram program;
private ArtificialRescueRecorder(JProgram program) {
this.program = program;
- artificialRescueType = program.getFromTypeMap(ArtificialRescue.class.getName());
+ artificialRescueType = (JInterfaceType) program.getFromTypeMap(ArtificialRescue.class.getName());
}
private void execImpl() {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java
index 1d2876b..4133976 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java
@@ -240,14 +240,9 @@
return null;
}
- private final JType type;
+ private final JInterfaceType type;
private List<Property> properties = Lists.create();
- public JAnnotation(SourceInfo sourceInfo, JExternalType type) {
- super(sourceInfo);
- this.type = type;
- }
-
public JAnnotation(SourceInfo sourceInfo, JInterfaceType type) {
super(sourceInfo);
this.type = type;
@@ -277,7 +272,7 @@
return null;
}
- public JType getType() {
+ public JInterfaceType getType() {
return type;
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JArrayType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JArrayType.java
index 8fd0b19..3890392 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JArrayType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JArrayType.java
@@ -80,6 +80,11 @@
return false;
}
+ @Override
+ public boolean isExternal() {
+ return elementType.isExternal();
+ }
+
public boolean isFinal() {
return leafType.isFinal();
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java
index 92ba61c..a3ff614 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JClassLiteral.java
@@ -131,7 +131,7 @@
JClassLiteral componentLiteral = program.getLiteralClass(arrayType.getElementType());
call.addArg(componentLiteral);
} else {
- assert (type instanceof JExternalType || type instanceof JInterfaceType || type instanceof JPrimitiveType);
+ assert (type instanceof JInterfaceType || type instanceof JPrimitiveType);
}
assert call.getArgs().size() == method.getParams().size() : "Argument / param mismatch "
+ call.toString() + " versus " + method.toString();
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
index b539640..97964e7 100755
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JDeclaredType.java
@@ -67,6 +67,14 @@
private JDeclaredType enclosingType;
/**
+ * True if this class is provided externally to the program by the program's
+ * host execution environment. For example, while compiling for the JVM, JRE
+ * types are external types. External types definitions are provided by class
+ * files which are considered opaque by the GWT compiler.
+ */
+ private boolean isExternal;
+
+ /**
* This type's super class.
*/
private JClassType superClass;
@@ -226,10 +234,16 @@
return clinitTarget != null;
}
+ @Override
+ public boolean isExternal() {
+ return isExternal;
+ }
+
/**
* Removes the field at the specified index.
*/
public void removeField(int i) {
+ assert !isExternal() : "External types can not be modiified.";
fields = Lists.remove(fields, i);
}
@@ -237,6 +251,7 @@
* Removes the method at the specified index.
*/
public void removeMethod(int i) {
+ assert !isExternal() : "External types can not be modiified.";
methods = Lists.remove(methods, i);
}
@@ -249,6 +264,10 @@
this.enclosingType = enclosingType;
}
+ public void setExternal(boolean isExternal) {
+ this.isExternal = isExternal;
+ }
+
/**
* Sets this type's super class.
*/
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JExternalType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JExternalType.java
deleted file mode 100644
index 86d69a7..0000000
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JExternalType.java
+++ /dev/null
@@ -1,49 +0,0 @@
-/*
- * Copyright 2010 Google Inc.
- *
- * Licensed under the Apache License, Version 2.0 (the "License"); you may not
- * use this file except in compliance with the License. You may obtain a copy of
- * the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
- * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
- * License for the specific language governing permissions and limitations under
- * the License.
- */
-package com.google.gwt.dev.jjs.ast;
-
-import com.google.gwt.dev.jjs.SourceInfo;
-
-/**
- * Represents a type outside of the client type system, usually a binary-only
- * annotation reference.
- */
-public class JExternalType extends JDeclaredType {
-
- JExternalType(SourceInfo info, String name) {
- super(info, name);
- }
-
- @Override
- public String getClassLiteralFactoryMethod() {
- return "Class.createForInterface";
- }
-
- public boolean isAbstract() {
- return true;
- }
-
- public boolean isFinal() {
- return false;
- }
-
- public void traverse(JVisitor visitor, Context ctx) {
- if (visitor.visit(this, ctx)) {
- annotations = visitor.acceptImmutable(annotations);
- }
- visitor.endVisit(this, ctx);
- }
-}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
index 3feb55d..7e1660e 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JMethod.java
@@ -148,6 +148,7 @@
}
public JAbstractMethodBody getBody() {
+ assert !enclosingType.isExternal() : "External types do not have method bodies.";
return body;
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
index 92f4f0f..ea9d226 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JProgram.java
@@ -303,8 +303,6 @@
*/
private final ArrayList<HashMap<JType, JArrayType>> dimensions = new ArrayList<HashMap<JType, JArrayType>>();
- private final Map<String, JExternalType> externalTypes = new HashMap<String, JExternalType>();
-
private final Map<String, JField> indexedFields = new HashMap<String, JField>();
private final Map<String, JMethod> indexedMethods = new HashMap<String, JMethod>();
@@ -481,20 +479,6 @@
return x;
}
- public JExternalType createExternalType(SourceInfo info, String name) {
- JExternalType x;
- x = externalTypes.get(name);
- if (x != null) {
- return x;
- }
-
- x = new JExternalType(info, name);
- if (INDEX_TYPES_SET.contains(name)) {
- indexedTypes.put(x.getShortName(), x);
- }
- return x;
- }
-
public JField createField(SourceInfo info, String name,
JDeclaredType enclosingType, JType type, boolean isStatic,
Disposition disposition) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JType.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JType.java
index 837e727..7391010 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JType.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JType.java
@@ -45,4 +45,8 @@
return name;
}
+ public boolean isExternal() {
+ return false;
+ }
+
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java b/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
index 225c932..73c6ff1 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/ast/JTypeOracle.java
@@ -611,7 +611,7 @@
private void computeClinitTarget(JDeclaredType type,
Set<JDeclaredType> computed) {
- if (!type.hasClinit() || computed.contains(type)) {
+ if (type.isExternal() || !type.hasClinit() || computed.contains(type)) {
return;
}
if (type.getSuperClass() != null) {
@@ -876,6 +876,18 @@
Set<JReferenceType> instantiatedTypes) {
type = program.getRunTimeType(type);
+ if (type.isExternal()) {
+ // TODO(tobyr) I don't know under what situations it is safe to assume
+ // that an external type won't be instantiated. For example, if we
+ // assumed that an external exception weren't instantiated, because we
+ // didn't see it constructed in our code, dead code elimination would
+ // incorrectly elide any catch blocks for that exception.
+ //
+ // We should see how this effects optimization and if we can limit its
+ // impact if necessary.
+ return true;
+ }
+
if (instantiatedTypes == null) {
return true;
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
index 66b6b5a..a90b297 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/DeadCodeElimination.java
@@ -259,11 +259,6 @@
}
@Override
- public void endVisit(JClassType x, Context ctx) {
- // previously set currentClass = null;
- }
-
- @Override
public void endVisit(JConditional x, Context ctx) {
JExpression updated = simplifier.conditional(x, x.getSourceInfo(),
x.getType(), x.getIfTest(), x.getThenExpr(), x.getElseExpr());
@@ -614,8 +609,8 @@
@Override
public boolean visit(JClassType x, Context ctx) {
- // previously set currentClass = x;
- return true;
+ // We can't eliminate code from an external type
+ return !x.isExternal();
}
@Override
@@ -1463,8 +1458,8 @@
/**
* Simplify the expression <code>-exp</code>.
*
+ * @param original An expression equivalent to <code>-exp</code>.
* @param exp The expression to negate.
- * @param An expression equivalent to <code>-exp</code>.
*
* @return A simplified expression equivalent to <code>- exp</code>.
*/
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/Finalizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/Finalizer.java
index 60b4250..e15b90b 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/Finalizer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/Finalizer.java
@@ -103,6 +103,13 @@
}
@Override
+ public boolean visit(JClassType x, Context ctx) {
+ // Don't visit external types, because we can't change their final
+ // specifiers.
+ return !x.isExternal();
+ }
+
+ @Override
public boolean visit(JMethodBody x, Context ctx) {
for (JLocal local : x.getLocals()) {
maybeFinalize(local);
@@ -180,6 +187,13 @@
}
}
+ @Override
+ public boolean visit(JClassType x, Context ctx) {
+ // Don't visit external types, because we can't change their final
+ // specifiers.
+ return !x.isExternal();
+ }
+
private void recordAssignment(JExpression lhs) {
if (lhs instanceof JVariableRef) {
JVariableRef variableRef = (JVariableRef) lhs;
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
index bf45e4c..658c1cb 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java
@@ -48,7 +48,6 @@
import com.google.gwt.dev.jjs.ast.JEnumType;
import com.google.gwt.dev.jjs.ast.JExpression;
import com.google.gwt.dev.jjs.ast.JExpressionStatement;
-import com.google.gwt.dev.jjs.ast.JExternalType;
import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JFieldRef;
import com.google.gwt.dev.jjs.ast.JFloatLiteral;
@@ -2011,12 +2010,9 @@
}
}
- private void addThrownExceptions(MethodBinding methodBinding,
- JMethod method) {
- for (ReferenceBinding exceptionReference :
- methodBinding.thrownExceptions) {
- method.addThrownException((JClassType)
- typeMap.get(exceptionReference.erasure()));
+ private void addThrownExceptions(MethodBinding methodBinding, JMethod method) {
+ for (ReferenceBinding exceptionReference : methodBinding.thrownExceptions) {
+ method.addThrownException((JClassType) typeMap.get(exceptionReference.erasure()));
}
}
@@ -2239,6 +2235,17 @@
return (SourceTypeBinding) typeBinding;
}
+ private JInterfaceType getOrCreateExternalType(SourceInfo info,
+ char[][] compoundName) {
+ String name = BuildTypeMap.dotify(compoundName);
+ JInterfaceType external = (JInterfaceType) program.getFromTypeMap(name);
+ if (external == null) {
+ external = program.createInterface(info, name);
+ external.setExternal(true);
+ }
+ return external;
+ }
+
/**
* Get a new label of a particular name, or create a new one if it doesn't
* exist already.
@@ -2450,25 +2457,26 @@
for (ElementValuePair pair : elementValuePairs) {
String name = CharOperation.charToString(pair.getName());
- List<JAnnotationArgument> values = processAnnotationPropertyValue(sourceInfo,
- pair.getValue());
+ List<JAnnotationArgument> values = processAnnotationPropertyValue(
+ sourceInfo, pair.getValue());
annotation.addValue(new Property(sourceInfo, name, values));
}
}
- private List<JAnnotationArgument> processAnnotationPropertyValue(SourceInfo info,
- Object value) {
+ private List<JAnnotationArgument> processAnnotationPropertyValue(
+ SourceInfo info, Object value) {
if (value instanceof TypeBinding) {
JType type = (JType) typeMap.tryGet((TypeBinding) value);
if (type == null) {
// Indicates a binary-only class literal
- type = program.createExternalType(info,
- BuildTypeMap.dotify(((ReferenceBinding) value).compoundName));
+ type = getOrCreateExternalType(info,
+ ((ReferenceBinding) value).compoundName);
}
return Lists.<JAnnotationArgument> create(program.getLiteralClass(type));
} else if (value instanceof Constant) {
- return Lists.create((JAnnotationArgument) dispatch("processConstant", value));
+ return Lists.create((JAnnotationArgument) dispatch("processConstant",
+ value));
} else if (value instanceof Object[]) {
Object[] array = (Object[]) value;
@@ -2487,8 +2495,8 @@
if (type != null) {
toReturn = new JAnnotation(info, type);
} else {
- JExternalType external = program.createExternalType(info,
- BuildTypeMap.dotify(annotationType.compoundName));
+ JInterfaceType external = getOrCreateExternalType(info,
+ annotationType.compoundName);
toReturn = new JAnnotation(info, external);
}
@@ -2554,8 +2562,8 @@
annotation = new JAnnotation(x.getSourceInfo(), annotationType);
} else {
// Indicates a binary-only annotation type
- JExternalType externalType = program.createExternalType(
- x.getSourceInfo(), BuildTypeMap.dotify(binding.compoundName));
+ JInterfaceType externalType = getOrCreateExternalType(
+ x.getSourceInfo(), binding.compoundName);
annotation = new JAnnotation(x.getSourceInfo(), externalType);
}
processAnnotationProperties(x.getSourceInfo(), annotation,
@@ -2805,7 +2813,7 @@
JDeclarationStatement declStmt = new JDeclarationStatement(sourceInfo,
valuesRef, newExpr);
JBlock clinitBlock = ((JMethodBody) type.getMethods().get(0).getBody()).getBlock();
-
+
/*
* HACKY: the $VALUES array must be initialized immediately after all of
* the enum fields, but before any user initialization (which might rely
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java b/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java
index 5cd64e3..4cf8632 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/MakeCallsStatic.java
@@ -250,6 +250,15 @@
public void endVisit(JMethodCall x, Context ctx) {
JMethod method = x.getTarget();
+ if (method.getEnclosingType() != null) {
+ if (method.getEnclosingType().isExternal()) {
+ // Staticifying a method requires modifying the type, which we can't
+ // do for external types. Theoretically we could put the static method
+ // in some generated code, but what does that really buy us?
+ return;
+ }
+ }
+
// Did we already do this one?
if (program.getStaticImpl(method) != null
|| toBeMadeStatic.contains(method)) {
diff --git a/dev/core/test/com/google/gwt/dev/jjs/impl/JAnnotationTest.java b/dev/core/test/com/google/gwt/dev/jjs/impl/JAnnotationTest.java
index c0990b1..129981c 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/impl/JAnnotationTest.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/impl/JAnnotationTest.java
@@ -20,7 +20,6 @@
import com.google.gwt.dev.jjs.ast.JAnnotation;
import com.google.gwt.dev.jjs.ast.JClassLiteral;
import com.google.gwt.dev.jjs.ast.JDeclaredType;
-import com.google.gwt.dev.jjs.ast.JExternalType;
import com.google.gwt.dev.jjs.ast.JField;
import com.google.gwt.dev.jjs.ast.JLocal;
import com.google.gwt.dev.jjs.ast.JMethod;
@@ -231,7 +230,7 @@
JAnnotation a = JAnnotation.findAnnotation(useDefaults,
BinaryAnnotation.class.getName());
assertNotNull(a);
- assertTrue(a.getType() instanceof JExternalType);
+ assertTrue(a.getType().isExternal());
BinaryAnnotation instance = JAnnotation.createAnnotation(
BinaryAnnotation.class, a);
@@ -265,7 +264,8 @@
Property p = a.getProperty("value");
JClassLiteral literal = (JClassLiteral) p.getSingleValue();
- JExternalType externalType = (JExternalType) literal.getRefType();
- assertEquals(JAnnotationTest.class.getName(), externalType.getName());
+ JDeclaredType type = (JDeclaredType) literal.getRefType();
+ assertTrue(type.isExternal());
+ assertEquals(JAnnotationTest.class.getName(), type.getName());
}
}
