[jira] [Commented] (CASSANDRA-12598) BailErrorStragery alike for ANTLR grammar parsing
[ https://issues.apache.org/jira/browse/CASSANDRA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15564951#comment-15564951 ] Benjamin Lerer commented on CASSANDRA-12598: Committed into 3.X at 830954304ebcf450b213c9ebc53cb4eefb87f02d and merged into trunk > BailErrorStragery alike for ANTLR grammar parsing > - > > Key: CASSANDRA-12598 > URL: https://issues.apache.org/jira/browse/CASSANDRA-12598 > Project: Cassandra > Issue Type: Bug > Components: CQL >Reporter: Berenguer Blasi >Assignee: Berenguer Blasi > Fix For: 3.x > > > CQL parsing is missing a mechanism similar to > http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html > This solves: > - Stopping parsing instead of continuing when we've got already an error > which is wasteful. > - Any skipped java code tied to 'recovered' missing tokens might later cause > java exceptions (think non-init variables, non incremented integers (div by > zero), etc.) which will bubble up directly and will hide properly formatted > error messages to the user with no indication on what went wrong at all. Just > a cryptic NPE i.e -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-12598) BailErrorStragery alike for ANTLR grammar parsing
[
https://issues.apache.org/jira/browse/CASSANDRA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15554512#comment-15554512
]
Berenguer Blasi commented on CASSANDRA-12598:
-
{{recoverFromMismatchedSet}} is marked indeed as not used anymore indeed
[link|http://www.antlr3.org/api/Java/org/antlr/runtime/BaseRecognizer.html#recoverFromMismatchedSet(org.antlr.runtime.IntStream,%20org.antlr.runtime.RecognitionException,%20org.antlr.runtime.BitSet)]
but doesn't mention any deprecation. Makes me wonder if it'll ever be used
again...
This approach works for me too +1. Thanks [~blerer] for looking into this.
> BailErrorStragery alike for ANTLR grammar parsing
> -
>
> Key: CASSANDRA-12598
> URL: https://issues.apache.org/jira/browse/CASSANDRA-12598
> Project: Cassandra
> Issue Type: Bug
> Components: CQL
>Reporter: Berenguer Blasi
>Assignee: Berenguer Blasi
> Fix For: 3.x
>
>
> CQL parsing is missing a mechanism similar to
> http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html
> This solves:
> - Stopping parsing instead of continuing when we've got already an error
> which is wasteful.
> - Any skipped java code tied to 'recovered' missing tokens might later cause
> java exceptions (think non-init variables, non incremented integers (div by
> zero), etc.) which will bubble up directly and will hide properly formatted
> error messages to the user with no indication on what went wrong at all. Just
> a cryptic NPE i.e
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-12598) BailErrorStragery alike for ANTLR grammar parsing
[
https://issues.apache.org/jira/browse/CASSANDRA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15552096#comment-15552096
]
Benjamin Lerer commented on CASSANDRA-12598:
ANTLR 3.5.2 perform recovery at 2 places: in
[BaseRecognizer::recoverFromMismatchedToken|https://github.com/antlr/antlr3/blob/master/runtime/Java/src/main/java/org/antlr/runtime/BaseRecognizer.java#L588]
and
in
[BaseRecognizer::recover|https://github.com/antlr/antlr3/blob/master/runtime/Java/src/main/java/org/antlr/runtime/BaseRecognizer.java#L353].
[BaseRecognizer::recoverFromMismatchedSet|https://github.com/antlr/antlr3/blob/master/runtime/Java/src/main/java/org/antlr/runtime/BaseRecognizer.java#L621]
is not used anymore.
In case of mismatch, {{BaseRecognizer::recoverFromMismatchedToken}} will attend
to recover by either deleting the unexpected token or inserting the missing
one, reporting an error in both cases.
If those 2 technics do not work it will throw a {{MismatchedTokenException}}.
That Exception will be catch by the default code that ANTLR generates at the
end of each rule’s method:
{code}
catch (RecognitionException re) {
reportError(re);
recover(input,re);
}
{code}
By consequence, the right way to disable recovery is to override
{{BaseRecognizer::recoverFromMismatchedToken}} to make it always throw a
{{MismatchedTokenException}} and
to either override the catch code generated by ANTLR using the {{rulecatch}}
action or simply override {{BaseRecognizer::recover}} to make it does nothing.
As you seems to be in favor of the bulletproof approach, overriding
{{BaseRecognizer::recover}} sound like the best option.
The final version of my experimentations is:
|[trunk|https://github.com/blerer/cassandra/tree/12598-trunk]|[utests|http://cassci.datastax.com/view/Dev/view/blerer/job/blerer-12598-trunk-testall/]|[dtests|http://cassci.datastax.com/view/Dev/view/blerer/job/blerer-12598-trunk-dtest/lastCompletedBuild/]|
While trying the different approaches, I realized that several rules have not
been written correctly and were preventing the generated code to compile
depending on how the {{rulecatch}} action was used. So, I fixed those rules in
the provided patch.
[~Bereng] Could you have a look at the patch and tell me if it looks fine to
you?
> BailErrorStragery alike for ANTLR grammar parsing
> -
>
> Key: CASSANDRA-12598
> URL: https://issues.apache.org/jira/browse/CASSANDRA-12598
> Project: Cassandra
> Issue Type: Bug
> Components: CQL
>Reporter: Berenguer Blasi
>Assignee: Berenguer Blasi
> Fix For: 3.x
>
>
> CQL parsing is missing a mechanism similar to
> http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html
> This solves:
> - Stopping parsing instead of continuing when we've got already an error
> which is wasteful.
> - Any skipped java code tied to 'recovered' missing tokens might later cause
> java exceptions (think non-init variables, non incremented integers (div by
> zero), etc.) which will bubble up directly and will hide properly formatted
> error messages to the user with no indication on what went wrong at all. Just
> a cryptic NPE i.e
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-12598) BailErrorStragery alike for ANTLR grammar parsing
[
https://issues.apache.org/jira/browse/CASSANDRA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15552095#comment-15552095
]
Benjamin Lerer commented on CASSANDRA-12598:
ANTLR 3.5.2 perform recovery at 2 places: in
[BaseRecognizer::recoverFromMismatchedToken|https://github.com/antlr/antlr3/blob/master/runtime/Java/src/main/java/org/antlr/runtime/BaseRecognizer.java#L588]
and
in
[BaseRecognizer::recover|https://github.com/antlr/antlr3/blob/master/runtime/Java/src/main/java/org/antlr/runtime/BaseRecognizer.java#L353].
[BaseRecognizer::recoverFromMismatchedSet|https://github.com/antlr/antlr3/blob/master/runtime/Java/src/main/java/org/antlr/runtime/BaseRecognizer.java#L621]
is not used anymore.
In case of mismatch, {{BaseRecognizer::recoverFromMismatchedToken}} will attend
to recover by either deleting the unexpected token or inserting the missing
one, reporting an error in both cases.
If those 2 technics do not work it will throw a {{MismatchedTokenException}}.
That Exception will be catch by the default code that ANTLR generates at the
end of each rule’s method:
{code}
catch (RecognitionException re) {
reportError(re);
recover(input,re);
}
{code}
By consequence, the right way to disable recovery is to override
{{BaseRecognizer::recoverFromMismatchedToken}} to make it always throw a
{{MismatchedTokenException}} and
to either override the catch code generated by ANTLR using the {{rulecatch}}
action or simply override {{BaseRecognizer::recover}} to make it does nothing.
As you seems to be in favor of the bulletproof approach, overriding
{{BaseRecognizer::recover}} sound like the best option.
The final version of my experimentations is:
|[trunk|https://github.com/blerer/cassandra/tree/12598-trunk]|[utests|http://cassci.datastax.com/view/Dev/view/blerer/job/blerer-12598-trunk-testall/]|[dtests|http://cassci.datastax.com/view/Dev/view/blerer/job/blerer-12598-trunk-dtest/lastCompletedBuild/]|
While trying the different approaches, I realized that several rules have not
been written correctly and were preventing the generated code to compile
depending on how the {{rulecatch}} action was used. So, I fixed those rules in
the provided patch.
[~Bereng] Could you have a look at the patch and tell me if it looks fine to
you?
> BailErrorStragery alike for ANTLR grammar parsing
> -
>
> Key: CASSANDRA-12598
> URL: https://issues.apache.org/jira/browse/CASSANDRA-12598
> Project: Cassandra
> Issue Type: Bug
> Components: CQL
>Reporter: Berenguer Blasi
>Assignee: Berenguer Blasi
> Fix For: 3.x
>
>
> CQL parsing is missing a mechanism similar to
> http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html
> This solves:
> - Stopping parsing instead of continuing when we've got already an error
> which is wasteful.
> - Any skipped java code tied to 'recovered' missing tokens might later cause
> java exceptions (think non-init variables, non incremented integers (div by
> zero), etc.) which will bubble up directly and will hide properly formatted
> error messages to the user with no indication on what went wrong at all. Just
> a cryptic NPE i.e
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-12598) BailErrorStragery alike for ANTLR grammar parsing
[
https://issues.apache.org/jira/browse/CASSANDRA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15542078#comment-15542078
]
Berenguer Blasi commented on CASSANDRA-12598:
-
Sorry for the late reply, I am back online.
I sleep better knowing I've overloaded those methods as this effectively gives
you the {{BailErrorStrategy}} behaviour we're looking for 100% guaranteed. If
somebody calls any of these methods directly, some auto-generated code changes
to use them otherwise different than now, etc. It is more future proof overall
imo.
> BailErrorStragery alike for ANTLR grammar parsing
> -
>
> Key: CASSANDRA-12598
> URL: https://issues.apache.org/jira/browse/CASSANDRA-12598
> Project: Cassandra
> Issue Type: Bug
> Components: CQL
>Reporter: Berenguer Blasi
>Assignee: Berenguer Blasi
> Fix For: 3.x
>
>
> CQL parsing is missing a mechanism similar to
> http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html
> This solves:
> - Stopping parsing instead of continuing when we've got already an error
> which is wasteful.
> - Any skipped java code tied to 'recovered' missing tokens might later cause
> java exceptions (think non-init variables, non incremented integers (div by
> zero), etc.) which will bubble up directly and will hide properly formatted
> error messages to the user with no indication on what went wrong at all. Just
> a cryptic NPE i.e
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-12598) BailErrorStragery alike for ANTLR grammar parsing
[ https://issues.apache.org/jira/browse/CASSANDRA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15529730#comment-15529730 ] Aleksey Yeschenko commented on CASSANDRA-12598: --- [~Bereng] ping > BailErrorStragery alike for ANTLR grammar parsing > - > > Key: CASSANDRA-12598 > URL: https://issues.apache.org/jira/browse/CASSANDRA-12598 > Project: Cassandra > Issue Type: Bug > Components: CQL >Reporter: Berenguer Blasi >Assignee: Berenguer Blasi > Fix For: 3.x > > > CQL parsing is missing a mechanism similar to > http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html > This solves: > - Stopping parsing instead of continuing when we've got already an error > which is wasteful. > - Any skipped java code tied to 'recovered' missing tokens might later cause > java exceptions (think non-init variables, non incremented integers (div by > zero), etc.) which will bubble up directly and will hide properly formatted > error messages to the user with no indication on what went wrong at all. Just > a cryptic NPE i.e -- This message was sent by Atlassian JIRA (v6.3.4#6332)
[jira] [Commented] (CASSANDRA-12598) BailErrorStragery alike for ANTLR grammar parsing
[
https://issues.apache.org/jira/browse/CASSANDRA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15506425#comment-15506425
]
Benjamin Lerer commented on CASSANDRA-12598:
[~Bereng] Are you sure that we need to override {{recover}} and
{{recoverFromMismatchedSet}}? They seems to be called only after
{{addRecognitionError}}.
> BailErrorStragery alike for ANTLR grammar parsing
> -
>
> Key: CASSANDRA-12598
> URL: https://issues.apache.org/jira/browse/CASSANDRA-12598
> Project: Cassandra
> Issue Type: Bug
> Components: CQL
>Reporter: Berenguer Blasi
> Fix For: 3.x
>
>
> CQL parsing is missing a mechanism similar to
> http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html
> This solves:
> - Stopping parsing instead of continuing when we've got already an error
> which is wasteful.
> - Any skipped java code tied to 'recovered' missing tokens might later cause
> java exceptions (think non-init variables, non incremented integers (div by
> zero), etc.) which will bubble up directly and will hide properly formatted
> error messages to the user with no indication on what went wrong at all. Just
> a cryptic NPE i.e
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (CASSANDRA-12598) BailErrorStragery alike for ANTLR grammar parsing
[ https://issues.apache.org/jira/browse/CASSANDRA-12598?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15490424#comment-15490424 ] Berenguer Blasi commented on CASSANDRA-12598: - Branch available here: [link|https://github.com/bereng/cassandra/tree/CASSANDRA-12598-trunk_2] Test all results available here [link|https://cassci.datastax.com/view/Dev/view/bereng/job/bereng-CASSANDRA-12598-trunk_2-testall/2/] Dtest resultst available here [link|https://cassci.datastax.com/view/Dev/view/bereng/job/bereng-CASSANDRA-12598-trunk_2-dtest/6/] (Failure looks unrelated, previous commit fails on a different test also) > BailErrorStragery alike for ANTLR grammar parsing > - > > Key: CASSANDRA-12598 > URL: https://issues.apache.org/jira/browse/CASSANDRA-12598 > Project: Cassandra > Issue Type: Bug > Components: CQL >Reporter: Berenguer Blasi > Fix For: 3.x > > > CQL parsing is missing a mechanism similar to > http://www.antlr.org/api/Java/org/antlr/v4/runtime/BailErrorStrategy.html > This solves: > - Stopping parsing instead of continuing when we've got already an error > which is wasteful. > - Any skipped java code tied to 'recovered' missing tokens might later cause > java exceptions (think non-init variables, non incremented integers (div by > zero), etc.) which will bubble up directly and will hide properly formatted > error messages to the user with no indication on what went wrong at all. Just > a cryptic NPE i.e -- This message was sent by Atlassian JIRA (v6.3.4#6332)
