You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@groovy.apache.org by Jochen Theodorou <bl...@gmx.org> on 2016/10/25 20:53:34 UTC
making parser modules for antlr2 and antlr4
Hi,
I am looking a bit into the usage of our old parser and keeping in mind
the new parser we want to eventually make the default.
But one problem I talked about in the past already is the direct usage
of antlr2 types in something else than the parser itself. Moving the
current parser into a subproject has the nice effect of highlighting
those problems, since anltr2 classes are then not automatically part of
the main module anymore.
And then you find things like this:
> if (cause instanceof NoViableAltException) {
> return isEofToken(((NoViableAltException) cause).token);
> } else if (cause instanceof NoViableAltForCharException) {
> char badChar = ((NoViableAltForCharException) cause).foundChar;
> return badChar == CharScanner.EOF_CHAR;
> } else if (cause instanceof MismatchedCharException) {
> char badChar = (char) ((MismatchedCharException) cause).foundChar;
> return badChar == CharScanner.EOF_CHAR;
> } else if (cause instanceof MismatchedTokenException) {
> return isEofToken(((MismatchedTokenException) cause).token);
> }
The cause is from a SyntaxErrorMessage (actually cause is the cause of
the cause). Basically this tells me we are lacking the capability to
handle this case with only groovy-core classes. For the sake of
compatibility I don�t think we should get rid of the cause, but extend
SyntaxError(Message) or SyntaxException to support the transport of the
missing information.
I could now try to make a single class to cover all of the cases above,
which means a token (maybe expecting) and foundChar (maybe expectedChar)
and the problem, that if I have a token I do not have a char and vice
versa. I could work with Char...
if I want to project the given case and use the classic subclassing
approach I would produce two new subclasses of SyntaxException. If I
wanted to support the expected token or char, it would be 4 classes.
Any opinions on that?
bye Jochen
Re: making parser modules for antlr2 and antlr4
Posted by Guillaume Laforge <gl...@gmail.com>.
To me, those seem to be private APIs.
The general AST classes are heavily used by AST transforms, etc, so there's
a real use case for them being "public"... but I don't really see when
someone would try to hook into this area of Groovy. Is there a use case
where a user would do that?
If there's indeed not, I think we should consider they are private APIs,
and we'll be able to make the appropriate changes.
On Thu, Oct 27, 2016 at 1:47 AM, Jochen Theodorou <bl...@gmx.org> wrote:
>
> and found a new problem... org.codehaus.groovy.syntax.Numbers has a
> parseInteger method which takes an AST class from antlr as first parameter.
> The method is used from the parser and I think it is for the parser mainly.
> Of course the new parser uses this method as well. The purpose is to parse
> things like "109G" and "0x1234".
>
> The reason it has a AST parameter is for error reporting. You can use it
> with a null value, then there will be no error reporting. The reporting is
> done using ASTRuntimeException, which is one of our classes in the antlr
> package. For the parser only the error reporting version is
> interesting...but the new parser does not do that.
>
> First of all...do we see that as public API? Because if not, I can change
> it freely... and I would like to change ASTRuntimeException. instead of
> using the AST I would like to use only line and column as parameters
>
> bye Jochen
>
--
Guillaume Laforge
Apache Groovy committer & PMC Vice-President
Developer Advocate @ Google Cloud Platform
Blog: http://glaforge.appspot.com/
Social: @glaforge <http://twitter.com/glaforge> / Google+
<https://plus.google.com/u/0/114130972232398734985/posts>
Re: making parser modules for antlr2 and antlr4
Posted by Jochen Theodorou <bl...@gmx.org>.
and found a new problem... org.codehaus.groovy.syntax.Numbers has a
parseInteger method which takes an AST class from antlr as first
parameter. The method is used from the parser and I think it is for the
parser mainly. Of course the new parser uses this method as well. The
purpose is to parse things like "109G" and "0x1234".
The reason it has a AST parameter is for error reporting. You can use it
with a null value, then there will be no error reporting. The reporting
is done using ASTRuntimeException, which is one of our classes in the
antlr package. For the parser only the error reporting version is
interesting...but the new parser does not do that.
First of all...do we see that as public API? Because if not, I can
change it freely... and I would like to change ASTRuntimeException.
instead of using the AST I would like to use only line and column as
parameters
bye Jochen
Re: making parser modules for antlr2 and antlr4
Posted by Jochen Theodorou <bl...@gmx.org>.
On 25.10.2016 23:36, John Wagenleitner wrote:
[...]
> Could the existing "org.codehaus.groovy.syntax.TokenMismatchException"
> cover the cases for "NoViableAltException" and
> "MismatchedTokenException" (or maybe all cases since the focus is on
> finding unexpected EOF)?
yes, just found that presumably unused class as well ;)
bye Jochen
Re: making parser modules for antlr2 and antlr4
Posted by John Wagenleitner <jo...@gmail.com>.
On Tue, Oct 25, 2016 at 1:53 PM, Jochen Theodorou <bl...@gmx.org> wrote:
> Hi,
>
> I am looking a bit into the usage of our old parser and keeping in mind
> the new parser we want to eventually make the default.
>
> But one problem I talked about in the past already is the direct usage of
> antlr2 types in something else than the parser itself. Moving the current
> parser into a subproject has the nice effect of highlighting those
> problems, since anltr2 classes are then not automatically part of the main
> module anymore.
>
> And then you find things like this:
>
> if (cause instanceof NoViableAltException) {
>> return isEofToken(((NoViableAltException)
>> cause).token);
>> } else if (cause instanceof NoViableAltForCharException) {
>> char badChar = ((NoViableAltForCharException)
>> cause).foundChar;
>> return badChar == CharScanner.EOF_CHAR;
>> } else if (cause instanceof MismatchedCharException) {
>> char badChar = (char) ((MismatchedCharException)
>> cause).foundChar;
>> return badChar == CharScanner.EOF_CHAR;
>> } else if (cause instanceof MismatchedTokenException) {
>> return isEofToken(((MismatchedTokenException)
>> cause).token);
>> }
>>
>
> The cause is from a SyntaxErrorMessage (actually cause is the cause of the
> cause). Basically this tells me we are lacking the capability to handle
> this case with only groovy-core classes. For the sake of compatibility I
> don´t think we should get rid of the cause, but extend SyntaxError(Message)
> or SyntaxException to support the transport of the missing information.
>
> I could now try to make a single class to cover all of the cases above,
> which means a token (maybe expecting) and foundChar (maybe expectedChar)
> and the problem, that if I have a token I do not have a char and vice
> versa. I could work with Char...
>
> if I want to project the given case and use the classic subclassing
> approach I would produce two new subclasses of SyntaxException. If I wanted
> to support the expected token or char, it would be 4 classes.
>
> Any opinions on that?
>
>
Could the existing "org.codehaus.groovy.syntax.TokenMismatchException"
cover the cases for "NoViableAltException" and "MismatchedTokenException"
(or maybe all cases since the focus is on finding unexpected EOF)?