Proposed patch for review (JCTree.Tag)

[maurizio cimadamore]

Great work!
Only nitpick comment: watch out for import order (as in Attr.java) - we 
tend to order them alphabetically.

Maurizio

On 02/11/2011 20:41, Jonathan Gibbons wrote:
> 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
>