LGTM, with a few nits. Also, if there is no specific reason to have the annotations white list, I think we could simplify our code and drop it for now.
http://gwt-code-reviews.appspot.com/134810/diff/1/2 File dev/core/src/com/google/gwt/dev/jjs/ast/HasAnnotations.java (right): http://gwt-code-reviews.appspot.com/134810/diff/1/2#newcode26 Line 26: List<JAnnotation> getAnnotations(); getAnnotations(String className) would seem good to add here. That's the method that would almost always be used. http://gwt-code-reviews.appspot.com/134810/diff/1/3 File dev/core/src/com/google/gwt/dev/jjs/ast/JAnnotation.java (right): http://gwt-code-reviews.appspot.com/134810/diff/1/3#newcode32 Line 32: public class JAnnotation extends JLiteral { Extending literal means it extends expression, which means this class will show up whenever someone contemplates an operation across all expressions. However, most places that expressions can be used, an annotation cannot. How about instead an interface AnnotationArgument? JLiteral and JAnnotation would implement it. http://gwt-code-reviews.appspot.com/134810/diff/1/3#newcode56 Line 56: assert values.size() == 1 : "Expecting single-valued property, found " This seems like a method developers would expect to always do the check, not only when assertions are enabled. http://gwt-code-reviews.appspot.com/134810/diff/1/3#newcode145 Line 145: private Object reifyValue(JLiteral value) throws ClassNotFoundException { This is more precisely "evaluation": it converts an AST to a value. http://gwt-code-reviews.appspot.com/134810/diff/1/3#newcode186 Line 186: // Crete the annotation as a reflective Proxy instance Create http://gwt-code-reviews.appspot.com/134810/diff/1/13 File dev/core/src/com/google/gwt/dev/jjs/impl/GenerateJavaAST.java (right): http://gwt-code-reviews.appspot.com/134810/diff/1/13#newcode2689 Line 2689: * @param annotations Noise in the Javadoc. http://gwt-code-reviews.appspot.com/134810/diff/1/13#newcode2711 Line 2711: JInterfaceType annotationType = (JInterfaceType) typeMap.tryGet(binding); Add TypeMap.getOrExternal? This logic appears twice in GenerateJavaAST. http://gwt-code-reviews.appspot.com/134810/diff/1/13#newcode3120 Line 3120: "com.google.gwt.dev.jjs", "test")); Do we actually need an explicit white list? There's already an implicit white list that the only annotations that will have any effect are those that the compiler includes special support for. http://gwt-code-reviews.appspot.com/134810 -- http://groups.google.com/group/Google-Web-Toolkit-Contributors
