[prev in list] [next in list] [prev in thread] [next in thread] 

List:       groovy-jsr
Subject:    Re: [groovy-jsr] First pass at 1.5 based grammar
From:       John Rose <rose00 () mac ! com>
Date:       2005-01-19 22:23:08
Message-ID: B230E0DD-6A68-11D9-A156-000D93505406 () mac ! com
[Download RAW message or body]

On Jan 18, 2005, at 13:39, Jeremy Rayner wrote:
> Here are the changes I've made, if anyone would care to review...
> http://cvs.groovy.codehaus.org/viewrep/groovy/groovy/jsr/ideas/ 
> parsers/antlr/misc/studmanPlusGroovy.g?r1=1.2&r2=MAIN&u=3&ignore=&k=k
> ...
> TODO
> o Remove nondeterminism (antlr warns about all these cases)
Yes, and we must also audit the various occurrences of warning  
suppressions like "options { greedy = true; }".

> o Check changes with John Rose, I've commented the trickiest bits of
> the port to 1.5
My comments follow below.  Over all, thanks very much for doing the  
heavy lifting of this port.

> I must say that the antlr plugin for eclipse has been a joy to use, it
> really helps now that I've completed the diff/patch.
(I'm envious:  There doesn't appear to be a corresponding plugin for  
IDEA.
Failing that, I've been using the Mac's project builder as a text  
editor for the *.g files.)

 From the four files (java.g, groovy.g, Studman's java15.g, and  
studmanPlusGroovy.g) I have built a second-order diff to help locate  
the relevant changes.

All uses of the new nonterminal annotation should by accompanied by  
'nls!', by analogy with modifier.

The lookahead in typeDeclarationStart needs to skip annotations, not  
just stop at '@', because variable and method declarations can also be  
annotated.
> typeDeclarationStart!
>     :   (modifier!)* ("class" | "interface" | "enum" | AT )
S.B. something like
>     :   (modifier! | annotationTokens!)* ("class" | "interface" |  
> "enum" )
(And maybe @interface, if Java 5 allows nested annotation types?  Don't  
know offhand.)
Where annotationTokens can be a quick paren-skipper, as in other  
places:  '@' ident '(' balancedTokens ')'.

The declarationStart production needs to be strengthened to recognize  
things like {List<String> foo}.
Right now it only knows how to skip square brackets after the type, not  
angle brackets.
This probably turns out to be tricky because of >> vs. > >.  If so,  
just put a TO DO comment in.

(The arrayOrTypeArgs thing in 1.4 groovy.g is a placeholder which  
anticipates the trouble of integrating Java 5 type arguments.)

> // TODO - verify that 't' is useful/correct here, used to be 'rt'
Yes, that's correct.  The effect of declaratorBrackets would be a no-op  
if the brackets are not present.

There's a lookahead (declaration)=> which should probably be  
(declarationStart)=>.

The 'for' syntax is too back-tracky for my taste; it probably hides  
ambiguities.
I don't favor supporting both {for (x in y)} and {for (x : y)}.
The former is what the Java designers would have used, if they had been  
able to introduce the key word 'in'; let's do it right without worrying  
about sideways compatibility.
The rule I propose is "if there are semicolons inside the for-head, we  
support Java 1.4-type loops, else it's a Groovy x-in-y loop.  And  
change colon to 'in' when porting from Java."
Therefore, there's no need at all for squeezing in the new Java 5 "for"  
syntax, since Groovy's is a suitable alternative.
Even if I get outvoted and we support the Java 5 syntax, I'd prefer for  
it to be stuck onto the side of the grammar, later, not irretrievably  
wedged now into the middle.

The argumentLabelStart rule has a typo:  too many COLON tokens.

There's a regression on numeral tokens in your changes:
The lexical rules for numbers should not allow the Java numerals ".1"  
and "1.".
See the lexical chapter in the draft GLS.  Since numbers are objects in  
Groovy, they may be accompanied by dots.
Therefore, their spellings must not include outlying dots.
This means DOT (with double and triple dot) is an ordinary operator  
token; it doesn't need to be commented out.
Also, '.' ('0'..'9')* should be '.' ('0'..'9')+ instead.

Again, great work.  Apart from the above comments, ship it, and we'll  
improve it from there.

-- John


[prev in list] [next in list] [prev in thread] [next in thread]