Skip to content
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

Fix token declaration and verification in the model lexer #25137

Merged
merged 3 commits into from
May 7, 2015
Merged

Fix token declaration and verification in the model lexer #25137

merged 3 commits into from
May 7, 2015

Conversation

carols10cents
Copy link
Member

I know this is the most trivial thing since it's just the model lexer, not the real lexer, but it is simpler to read and it'd be great if it was up to date but it's been rotting and this is a tiny bit of that.

Thanks!!!!!!

This appears to not have too much of a detrimental effect, but it
doesn't seem to be what is intended either.

antlr doesn't mind that `PLUS` isn't declared in `tokens` and happily
uses the `PLUS` that appears later in the file, but the generated
RustLexer.tokens had PLUS at the end rather than where it was intended:

NOT=10
TILDE=11
PLUT=12
MINUS=13
...
PLUS=56
There were some tokens used in the grammar but not declared. Antlr
doesn't really seem to care and happily uses them, but they appear in
RustLexer.tokens in a potentially-unexpected order.
To prevent the reference grammar from getting out of sync with the real
grammar, panic if RustLexer.tokens contains an unknown token in a
similar way that verify.rs panics if it encounters an unknown binary
operation token.
@rust-highfive
Copy link
Collaborator

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

@bors: r+ 9c7d5ae rollup

Thanks!

steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 7, 2015
…, r=alexcrichton

I know this is the most trivial thing since it's *just* the model lexer, not the real lexer, but [it is simpler to read](rust-lang#15883 (comment)) and it'd be great if it was up to date but [it's been rotting](rust-lang#22379) and this is a tiny bit of that.

Thanks!!!!!!
steveklabnik added a commit to steveklabnik/rust that referenced this pull request May 7, 2015
…, r=alexcrichton

I know this is the most trivial thing since it's *just* the model lexer, not the real lexer, but [it is simpler to read](rust-lang#15883 (comment)) and it'd be great if it was up to date but [it's been rotting](rust-lang#22379) and this is a tiny bit of that.

Thanks!!!!!!
bors added a commit that referenced this pull request May 7, 2015
@bors bors merged commit 9c7d5ae into rust-lang:master May 7, 2015
@carols10cents carols10cents deleted the fix-token-declaration branch May 8, 2015 01:23
Manishearth added a commit to Manishearth/rust that referenced this pull request May 18, 2015
…richton

Hiiii soooo I'm trying to get the reference grammar and associated tests running again, and I swear I tested before but I must have had multiple things going on when I did, because the change I made in rust-lang#25137 to verify.rs is totally wrong. The RustLexer.tokens file that antlr generates has two sections:

```
EQ=1
LT=2
LE=3
EQEQ=4
NE=5
...
COMMENT=56
SHEBANG=57
UTF8_BOM=58
'='=1
'<'=2
'<='=3
'=='=4
...
```

and verify.rs is only interested in the first half-- the `continue` is to ignore the second half. In 9c7d5ae, I made it panic instead. I was trying to make sure verify.rs handled everything that might happen in the first half and complain if it didn't. That would mean the reference grammar was out of sync with at least verify.rs, if not the real grammar. But it's totally ok for verify.rs to not handle the entire second half of the file.

I'm sorry for breaking this :( Good thing these tests aren't being run regularly yet...? 😳
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants