-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check eof in tree #898
Check eof in tree #898
Conversation
…nterval. updated documentation for getSourceInterval(), added unit tests. fixed logic for special cases.
@@ -217,6 +220,9 @@ public void reset() { | |||
public Token match(int ttype) throws RecognitionException { | |||
Token t = getCurrentToken(); | |||
if ( t.getType()==ttype ) { | |||
if ( t.getType()==Token.EOF ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 t.getType()
could be replaced by ttype
for efficiency.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might not be using CommonToken, right?
❗ In addition to the items in the diff, |
i could swear i put that in. oh only in my plugin ;)
|
* token. | ||
* token and has interval i..i for token index i. | ||
* | ||
* <p>An interval of i..j for j < i indicates an empty interval.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 The getSourceInterval()
method was updated in 81e2a65 to ensure that an empty interval is always represented as i..i-1. I recommend changing this paragraph to the following:
<p>An interval of i..i-1 indicates an empty interval at position i in the input stream,
where 0 <= i <= the size of the input token stream.</p>
Two suggestions from reading the updated docs, but 👍 |
Fixes #896 and Fixes #897.