Proposed patch for review (JCTree.Tag)
Jonathan Gibbons
jonathan.gibbons at oracle.com
Wed Nov 2 13:41:37 PDT 2011
Vicente,
This is looking great. I like the use of the static imports (where
possible) and the new hasTag method. Code using hasTag(TAGNAME) is
definitely cleaner than before.
However, you can simplify JCTree by changing the definition of
JCTree.hasTag() to the following:
361 /* Returns true if the tag of this node is equals to tag.
362 */
363 public boolean hasTag(Tag tag) {
return tag == getTag();
}
then remove all the (now redundant) overriding methods.
Some additional comments:
(1)
TreeInfo, lines 68-95
Maybe use worker method(s) to simplify the repetitive code:
private void setOpname(Tag tag, String name) {
setOpname(tag, names.fromString(name));
}
private void setOpname(Tag tag, Name name) {
opname[tag.ordinal() - POS.ordinal()] = name;
}
(2)
I found three places where you were using Tag.ordinal(). This is not
wrong, but does imply we are publicly relying on the ordering of the
enum members. I wonder if we can put methods on the enum such that these
uses of values() and ordinal() are kept as implementation details within
the enum. The three uses I found are as follows:
Lower.java
1265 case PREINCcode: case POSTINCcode: case PREDECcode: case POSTDECcode:
1266 expr = makeUnary(
1267 values()[((acode1 - PREINCcode)>> 1) + PREINC.ordinal()], ref);
1268 break;
JavacParser
2847 return (oc.ordinal()> NO_TAG.ordinal()) ? TreeInfo.opPrec(oc) : -1;
Pretty
983 if (tree.getTag().ordinal()<= PREDEC.ordinal()) {
(3)
TreePosTest
Eliminate the static class TagNames entirely and move the get method up
one class-level and rename it as getName or something like that.
(4)
JCTree, 313-324: re-align comments
-- Jon
On 11/02/2011 01:24 PM, Jonathan Gibbons wrote:
> I have posted an updated patch from Vicente, available here:
>
> http://cr.openjdk.java.net/~jjg/6921494/webrev.01/
>
> -- Jon
More information about the compiler-dev
mailing list