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