Hi Jochen
> On 29. feb. 2016, at 11.39, Jochen Theodorou <blackd...@gmx.org> wrote:
> 
> On 28.02.2016 12:55, Jesper Steen Møller wrote:
> [...]
>>  * Memory: Is this an issue I should be focusing on — and is there a
>>    test to baseline against?
> 
> no test you can baseline against imho. The current approach is keeping 
> everything in memory and uses that for the cst part, which is build in the 
> parsing phase of the compiler. The conversion to an AST is done in the next 
> step, the conversion step.
> 

I’ll find a way to benchmark this with the existing test suite using 
ThreadMXBean.getThreadAllocatedBytes, such as total bytes allocated for some 
big Groovy files, once the grammar is complete. Antlr4 is pretty lean on 
resource use, if used correctly.

When compiling several files, are multiple AST’s in memory, or how does that 
work? If so, we’d also have to ensure that the live set of the AST doesn’t grow.

> I my opinion the cst and the getter for the cst should be deprecated... but 
> this may require some changes in the compiler and its infrastructure… so in 
> total, the tree approach might be easier for now.
> 
You read my mind, I was wondering if it’d be OK to get rid of the CST part. I 
don’t see why we can’t parse straight to AST.
Unfortunately, it’s not possible to eliminate the CSTNode hierarchy without 
changing the AST’s API (for Token, used for differentiating between binary ops) 
and LocatedMessage API (for CSTNode), and that would break compatibility.
I’m not sure if Groovy 3.0 would allow such API breakage, so I’m assuming ’no’.

>>  * I’ve discovered a small issue with unary syntax. Currently, nested
>>    unary expressions are not supported without parenthesis: Try e.g. -
>>    -1 or + -1. Is this intentional, or just an artifact of the
>>    precedence-refactored Java grammar?
> 
> 
> you mean "- -1" as in -(-1)? I think there was a grammar reason for this. If 
> it there is no trouble with this in the new grammar, then you can support it, 
> np.. I see that as an optional

Yes, exactly. New grammar supports this.

Thanks for your answers!

-Jesper

Reply via email to