I don't believe in magic, but as Scott points out, there are consistent
uses of the term to mean "compiler substitution of types", specifically
in java.lang.String and com.google.gwt.util.*. I took a stab at replacing
the word "magic" in some other contexts where I think different terminology
makes the comments or names clearer.
A grey area is the com.google.gwt.core.GWT class. Some of its methods are
re-written by the compiler, but in a way that is pretty clearly documented
and not as behind the scenes as the other uses.
Review at http://gwt-code-reviews.appspot.com/1453804
git-svn-id: https://google-web-toolkit.googlecode.com/svn/trunk@10303 8db76d5a-ed1c-0410-87a9-c151d255dfc7
diff --git a/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java b/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java
index 3daf323..d555625 100644
--- a/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java
+++ b/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java
@@ -242,8 +242,9 @@
TreeLogger branch =
logger.branch(TreeLogger.SPAM, "Scanning for additional dependencies: " + loc, null);
- // Examine the cud for magic types.
- //
+ // Examine the cud for types outside of the flow of the original Java
+ // source.
+ //
String[] typeNames = outer.doFindAdditionalTypesUsingJsni(branch, unit);
addAdditionalTypes(branch, typeNames);
@@ -266,9 +267,9 @@
}
/**
- * Helper method for process() that receives the types found by magic.
- * This causes the compiler to find the additional type, possibly winding
- * its back to ask for the compilation unit from the source oracle.
+ * Helper method for process() that receives the types found outside the
+ * flow of the original Java source. This causes the compiler to find the
+ * additional type, possibly causing the type to be compiled from source.
*/
private void addAdditionalTypes(TreeLogger logger, String[] typeNames) {
for (String typeName : typeNames) {
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
index 9a41672..85bc2b6 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
@@ -431,17 +431,20 @@
}
SourceInfo info = x.getSourceInfo();
if (toType instanceof JNullType) {
- /*
- * Magic: a null type cast means the user tried a cast that couldn't
- * possibly work. Typically this means either the statically resolvable
- * arg type is incompatible with the target type, or the target type was
- * globally uninstantiable. We handle this cast by throwing a
- * ClassCastException, unless the argument is null.
+ /**
+ * A null type cast is used as a placeholder value to indicate that the
+ * user tried a cast that couldn't possibly work. Typically this means
+ * either the statically resolvable arg type is incompatible with the
+ * target type, or the target type was globally uninstantiable.
+ *
+ * See {@link com.google.gwt.dev.jjs.impl.TypeTightener.TightenTypesVisitor#endVisit(JCastOperation,
+ * Context)}
+ *
+ * We handle this cast by throwing a ClassCastException, unless the
+ * argument is null.
*/
JMethod method = program.getIndexedMethod("Cast.throwClassCastExceptionUnlessNull");
- /*
- * Override the type of the magic method with the null type.
- */
+ // Note, we must update the method call to return the null type.
JMethodCall call = new JMethodCall(info, null, method, toType);
call.addArg(expr);
replaceExpr = call;
@@ -455,6 +458,7 @@
// just remove the cast
replaceExpr = curExpr;
} else {
+ // A cast is still needed. Substitute the appropriate Cast implementation.
JMethod method;
boolean isJsoCast = program.typeOracle.isEffectivelyJavaScriptObject(refType);
if (isJsoCast) {
@@ -631,5 +635,4 @@
replacer.accept(program);
}
}
-
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
index d42befc..36c534f 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
@@ -368,12 +368,15 @@
JMethod method = call.getTarget();
if (call.isVolatile() && method == runAsyncOnsuccess) {
/*
- * Magic magic magic: don't allow code flow from the AsyncFragmentLoader
- * implementation back into the callback.onSuccess(). If we did, the
- * rescue path would look like JRunAsync ->
- * AsyncFragmentLoader.runAsync() -> callback.onSuccess(). This would
- * completely defeat code splitting as all the code on the other side of
- * the barrier would become reachable.
+ * Note: In order to preserve code splitting, don't allow code flow from the
+ * AsyncFragmentLoader implementation back into the
+ * callback.onSuccess(). If we did, the rescue path would look like
+ * JRunAsync -> AsyncFragmentLoader.runAsync() -> callback.onSuccess().
+ * This would completely defeat code splitting as all the code on the
+ * other side of the barrier would become reachable.
+ *
+ * Code flow analysis is run separately on methods which implement
+ * RunAsyncCallback.onSuccess() as top-level entry points.
*/
return true;
}
@@ -517,7 +520,7 @@
/**
* Subclasses of JavaScriptObject are never instantiated directly. They are
- * created "magically" when a JSNI method passes a reference to an existing
+ * implicitly created when a JSNI method passes a reference to an existing
* JS object into Java code. If any point in the program can pass a value
* from JS into Java which could potentially be cast to JavaScriptObject, we
* must rescue JavaScriptObject.
@@ -643,8 +646,8 @@
* literal initializers.
*
* TODO: Model ClassLiteral access a different way to avoid special
- * magic. See
- * Pruner.transformToNullFieldRef()/transformToNullMethodCall().
+ * handling. See
+ * Pruner.transformToNullFieldRef()/transformToNullMethodCall().
*/
JField field = (JField) var;
accept(field.getInitializer());
@@ -961,15 +964,6 @@
runAsync.traverseOnSuccess(rescuer);
}
- /**
- * Traverse the fragments for all runAsyncs.
- */
- public void traverseFromRunAsyncs() {
- for (JRunAsync runAsync : program.getRunAsyncs()) {
- traverseFromRunAsync(runAsync);
- }
- }
-
private void buildMethodsOverriding() {
methodsThatOverrideMe = new HashMap<JMethod, List<JMethod>>();
for (JDeclaredType type : program.getDeclaredTypes()) {
@@ -985,4 +979,13 @@
}
}
}
+
+ /**
+ * Traverse the fragments for all runAsyncs.
+ */
+ private void traverseFromRunAsyncs() {
+ for (JRunAsync runAsync : program.getRunAsyncs()) {
+ traverseFromRunAsync(runAsync);
+ }
+ }
}
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java b/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
index a1b00f2..af0afd8 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
@@ -63,8 +63,9 @@
&& (currentMethod != null && currentMethod.getEnclosingType() == program
.getIndexedType("AsyncFragmentLoader"))) {
/*
- * Magic magic magic: don't optimize calls from AsyncFragmentLoader
- * to runAsyncCallBack.onSuccess(). This can defeat code splitting!
+ * Note: The volatile marker on the method flags it so that we don't
+ * optimize calls from AsyncFragmentLoader to implementations of
+ * RunAsyncCallback.onSuccess(). This can defeat code splitting.
*/
x.setVolatile();
return;
diff --git a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
index 259e8f2..d93006c 100644
--- a/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
+++ b/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
@@ -362,6 +362,16 @@
*/
public class TightenTypesVisitor extends JModVisitor {
+ /**
+ * Tries to determine a specific concrete type for the cast, then either
+ * removes the cast, or tightens the cast to a narrower type.
+ *
+ * If static analysis determines that a cast is not possible, swap in a cast
+ * to a null type. This will later be normalized into throwing an
+ * Exception.
+ *
+ * @see CastNormalizer
+ */
@Override
public void endVisit(JCastOperation x, Context ctx) {
JType argType = x.getExpr().getType();
@@ -394,7 +404,7 @@
// remove the cast operation
ctx.replaceMe(x.getExpr());
} else if (triviallyFalse && toType != program.getTypeNull()) {
- // replace with a magic NULL cast, unless it's already a cast to NULL
+ // replace with a placeholder cast to NULL, unless it's already a cast to NULL
JCastOperation newOp =
new JCastOperation(x.getSourceInfo(), program.getTypeNull(), x.getExpr());
ctx.replaceMe(newOp);
diff --git a/dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java b/dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java
index d4dbda9..4ab5eb5 100644
--- a/dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java
+++ b/dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java
@@ -147,7 +147,7 @@
addMember(members, field, field.getName());
}
- // Add a magic field to access class literals from JSNI
+ // Add a synthetic field to access class literals from JSNI
addMember(members, new SyntheticClassMember(targetClass), "class");
}
diff --git a/dev/core/src/com/google/gwt/dev/util/HttpHeaders.java b/dev/core/src/com/google/gwt/dev/util/HttpHeaders.java
index 873771b..83e64a8 100644
--- a/dev/core/src/com/google/gwt/dev/util/HttpHeaders.java
+++ b/dev/core/src/com/google/gwt/dev/util/HttpHeaders.java
@@ -23,7 +23,7 @@
import java.util.Locale;
/**
- * Groups predefined magic HTTP header strings.
+ * HTTP header strings defined by RFCs.
*/
public final class HttpHeaders {
diff --git a/dev/core/test/com/google/gwt/core/ext/linker/impl/SelectionScriptJavaScriptTest.java b/dev/core/test/com/google/gwt/core/ext/linker/impl/SelectionScriptJavaScriptTest.java
index e6a392a..e76f84e 100644
--- a/dev/core/test/com/google/gwt/core/ext/linker/impl/SelectionScriptJavaScriptTest.java
+++ b/dev/core/test/com/google/gwt/core/ext/linker/impl/SelectionScriptJavaScriptTest.java
@@ -149,7 +149,7 @@
}
/**
- * Test the fully magic script base with no meta properties.
+ * Test the script base with no meta properties.
*/
public void testScriptBase() throws IOException {
StringBuffer testCode = new StringBuffer();
@@ -269,6 +269,7 @@
final List<String> alerts = new ArrayList<String>();
webClient.setAlertHandler(new AlertHandler() {
+ @Override
public void handleAlert(Page page, String msg) {
alerts.add(msg);
}
diff --git a/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java b/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
index 39b2f51..a333f54 100644
--- a/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
+++ b/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
@@ -235,10 +235,8 @@
//
JavaToJavaScriptCompiler.checkForErrors(logger, goldenCuds, false);
- /*
- * FindDeferredBindingSitesVisitor detects errors in usage of magic methods
- * in the GWT class.
- */
+
+ // Find errors in usage of GWT.create() calls
for (CompilationUnitDeclaration jdtCud : goldenCuds) {
jdtCud.traverse(new FindDeferredBindingSitesVisitor(), jdtCud.scope);
}
diff --git a/dev/core/test/com/google/gwt/lang/LongLibTestBase.java b/dev/core/test/com/google/gwt/lang/LongLibTestBase.java
index 9c49076..d596e5d 100644
--- a/dev/core/test/com/google/gwt/lang/LongLibTestBase.java
+++ b/dev/core/test/com/google/gwt/lang/LongLibTestBase.java
@@ -21,8 +21,8 @@
import junit.framework.TestCase;
/**
- * Test the LongLib class. The magic expected values were computed by using a
- * Java println on normal Java longs.
+ * Test the LongLib class. The magic numbers being testing against were computed
+ * by using a Java println on normal Java longs.
*/
public class LongLibTestBase extends TestCase {
diff --git a/user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java b/user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java
index 1cc2cf7..8c5e0af 100644
--- a/user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java
+++ b/user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java
@@ -257,11 +257,12 @@
}
/**
- * The standard instance of AsyncFragmentLoader used in a web browser. If not
- * in GWT (i.e our vanilla JUnit tests, or if referenced in a server context),
- * this filed is {@code null}. In GWT, the parameters to this call are
- * rewritten by {@link com.google.gwt.dev.jjs.impl.ReplaceRunAsyncs}. So this
- * must be a method call of exactly two arguments, or that magic fails.
+ * The standard instance of AsyncFragmentLoader used in a web browser. Outside
+ * of GWT generated JavaScript (i.e our vanilla JUnit tests, or if referenced
+ * in a server context), this field is {@code null}. When compiled to
+ * JavaScript, the parameters to this call are rewritten by
+ * {@link com.google.gwt.dev.jjs.impl.ReplaceRunAsyncs}. So this must be a
+ * method call of exactly two arguments to succeed when invoked in web mode.
*/
public static AsyncFragmentLoader BROWSER_LOADER = makeBrowserLoader(1, new int[]{});
diff --git a/user/src/com/google/gwt/i18n/client/LocalizableResource.java b/user/src/com/google/gwt/i18n/client/LocalizableResource.java
index 4e5b20c..d126e6f 100644
--- a/user/src/com/google/gwt/i18n/client/LocalizableResource.java
+++ b/user/src/com/google/gwt/i18n/client/LocalizableResource.java
@@ -86,8 +86,8 @@
public @interface Generate {
/**
- * Constant "magic" default value used to detect that no value was supplied
- * for the fileName parameter.
+ * Placeholder used to detect that no value was supplied for the fileName
+ * parameter.
*/
String DEFAULT = "[default]";
diff --git a/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java b/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
index f4002b4..87e4e52 100644
--- a/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
+++ b/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
@@ -312,7 +312,7 @@
numExpressions = concatOp(numExpressions, b);
} else {
- // This indicates that some magic node is slipping by our visitors
+ // This indicates that some unexpected node is slipping by our visitors
loopLogger.log(TreeLogger.ERROR, "Unhandled substitution "
+ x.getClass());
throw new UnableToCompleteException();
diff --git a/user/src/com/google/gwt/user/rebind/ui/ImageBundleGenerator.java b/user/src/com/google/gwt/user/rebind/ui/ImageBundleGenerator.java
index bf4c0d7..eefb238 100644
--- a/user/src/com/google/gwt/user/rebind/ui/ImageBundleGenerator.java
+++ b/user/src/com/google/gwt/user/rebind/ui/ImageBundleGenerator.java
@@ -363,7 +363,7 @@
JClassType userType = typeOracle.getType(typeName);
// Get the type this generator is designed to support.
- JClassType magicType = typeOracle.findType(IMAGEBUNDLE_QNAME);
+ JClassType markerType = typeOracle.findType(IMAGEBUNDLE_QNAME);
// Ensure it's an interface.
if (userType.isInterface() == null) {
@@ -373,9 +373,9 @@
}
// Ensure proper derivation.
- if (!userType.isAssignableTo(magicType)) {
+ if (!userType.isAssignableTo(markerType)) {
logger.log(TreeLogger.ERROR, userType.getQualifiedSourceName()
- + " must be assignable to " + magicType.getQualifiedSourceName(),
+ + " must be assignable to " + markerType.getQualifiedSourceName(),
null);
throw new UnableToCompleteException();
}
diff --git a/user/super/com/google/gwt/emul/java/lang/Object.java b/user/super/com/google/gwt/emul/java/lang/Object.java
index 23ab9a5..3b8efff 100644
--- a/user/super/com/google/gwt/emul/java/lang/Object.java
+++ b/user/super/com/google/gwt/emul/java/lang/Object.java
@@ -43,7 +43,12 @@
private transient JavaScriptObject castableTypeMap;
/**
- * magic magic magic.
+ * A special marker field used internally to the GWT compiler. For example, it
+ * is used for distinguishing whether an object is a Java object or a
+ * JavaScriptObject. It is also used to differentiate our own Java objects
+ * from foreign objects in a different module on the same page.
+ *
+ * @see com.google.gwt.lang.Cast
*
* @skip
*/
@@ -55,11 +60,11 @@
}
/*
- * Magic; unlike the real JRE, we don't spec this method as final. The
- * compiler will generate a polymorphic override on every other class which
- * will return the correct class object.
+ * Note: Unlike the real JRE, we don't spec this method as final because the
+ * compiler generates a polymorphic override on every other class which will
+ * return the correct class object.
*
- * TODO(scottb): declare this final, but have the compiler fix it up.
+ * TODO(scottb, compiler magician): declare this final, but have the compiler fix it up.
*/
public Class<? extends Object> getClass() {
return Object.class;
@@ -74,7 +79,7 @@
}
/**
- * Never called; here for compatibility.
+ * Never called; here for JRE compatibility.
*
* @skip
*/
