Revision: 10303
Author: [email protected]
Date: Wed Jun 8 16:12:42 2011
Log: 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
http://code.google.com/p/google-web-toolkit/source/detail?r=10303
Modified:
/trunk/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java
/trunk/dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java
/trunk/dev/core/src/com/google/gwt/dev/util/HttpHeaders.java
/trunk/dev/core/test/com/google/gwt/core/ext/linker/impl/SelectionScriptJavaScriptTest.java
/trunk/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java
/trunk/dev/core/test/com/google/gwt/lang/LongLibTestBase.java
/trunk/user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java
/trunk/user/src/com/google/gwt/i18n/client/LocalizableResource.java
/trunk/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
/trunk/user/src/com/google/gwt/user/rebind/ui/ImageBundleGenerator.java
/trunk/user/super/com/google/gwt/emul/java/lang/Object.java
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java Thu
May 19 06:11:59 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jdt/AbstractCompiler.java Wed
Jun 8 16:12:42 2011
@@ -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) {
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java Tue
Jun 7 11:03:06 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/CastNormalizer.java Wed
Jun 8 16:12:42 2011
@@ -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);
}
}
-
-}
+}
=======================================
---
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
Thu Jun 2 11:49:52 2011
+++
/trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ControlFlowAnalyzer.java
Wed Jun 8 16:12:42 2011
@@ -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());
@@ -960,15 +963,6 @@
public void traverseFromRunAsync(JRunAsync runAsync) {
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>>();
@@ -985,4 +979,13 @@
}
}
}
-}
+
+ /**
+ * Traverse the fragments for all runAsyncs.
+ */
+ private void traverseFromRunAsyncs() {
+ for (JRunAsync runAsync : program.getRunAsyncs()) {
+ traverseFromRunAsync(runAsync);
+ }
+ }
+}
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
Thu Jun 2 11:49:52 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/ReplaceRunAsyncs.java
Wed Jun 8 16:12:42 2011
@@ -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;
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java Tue
Jun 7 11:03:06 2011
+++ /trunk/dev/core/src/com/google/gwt/dev/jjs/impl/TypeTightener.java Wed
Jun 8 16:12:42 2011
@@ -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);
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java Thu
Sep 23 06:33:21 2010
+++ /trunk/dev/core/src/com/google/gwt/dev/shell/DispatchClassInfo.java Wed
Jun 8 16:12:42 2011
@@ -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");
}
=======================================
--- /trunk/dev/core/src/com/google/gwt/dev/util/HttpHeaders.java Thu May 17
09:56:58 2007
+++ /trunk/dev/core/src/com/google/gwt/dev/util/HttpHeaders.java Wed Jun 8
16:12:42 2011
@@ -23,7 +23,7 @@
import java.util.Locale;
/**
- * Groups predefined magic HTTP header strings.
+ * HTTP header strings defined by RFCs.
*/
public final class HttpHeaders {
=======================================
---
/trunk/dev/core/test/com/google/gwt/core/ext/linker/impl/SelectionScriptJavaScriptTest.java
Mon Nov 1 10:01:11 2010
+++
/trunk/dev/core/test/com/google/gwt/core/ext/linker/impl/SelectionScriptJavaScriptTest.java
Wed Jun 8 16:12:42 2011
@@ -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);
}
=======================================
--- /trunk/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java Thu
Jun 2 11:49:52 2011
+++ /trunk/dev/core/test/com/google/gwt/dev/jjs/JavaAstConstructor.java Wed
Jun 8 16:12:42 2011
@@ -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);
}
=======================================
--- /trunk/dev/core/test/com/google/gwt/lang/LongLibTestBase.java Thu Jul
29 09:26:40 2010
+++ /trunk/dev/core/test/com/google/gwt/lang/LongLibTestBase.java Wed Jun
8 16:12:42 2011
@@ -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 {
=======================================
---
/trunk/user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java
Thu Jun 2 11:49:52 2011
+++
/trunk/user/src/com/google/gwt/core/client/impl/AsyncFragmentLoader.java
Wed Jun 8 16:12:42 2011
@@ -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[]{});
=======================================
--- /trunk/user/src/com/google/gwt/i18n/client/LocalizableResource.java Fri
Mar 11 11:18:03 2011
+++ /trunk/user/src/com/google/gwt/i18n/client/LocalizableResource.java Wed
Jun 8 16:12:42 2011
@@ -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]";
=======================================
--- /trunk/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
Thu May 12 14:09:51 2011
+++ /trunk/user/src/com/google/gwt/resources/rg/CssResourceGenerator.java
Wed Jun 8 16:12:42 2011
@@ -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();
=======================================
--- /trunk/user/src/com/google/gwt/user/rebind/ui/ImageBundleGenerator.java
Fri Mar 19 08:12:41 2010
+++ /trunk/user/src/com/google/gwt/user/rebind/ui/ImageBundleGenerator.java
Wed Jun 8 16:12:42 2011
@@ -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();
}
=======================================
--- /trunk/user/super/com/google/gwt/emul/java/lang/Object.java Mon Aug 30
04:31:11 2010
+++ /trunk/user/super/com/google/gwt/emul/java/lang/Object.java Wed Jun 8
16:12:42 2011
@@ -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
*/
--
http://groups.google.com/group/Google-Web-Toolkit-Contributors
