Skip to content

Revert "Panic if the grammar verifier sees a token it doesn't recognize" #25537

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

Merged
merged 1 commit into from
May 18, 2015
Merged

Revert "Panic if the grammar verifier sees a token it doesn't recognize" #25537

merged 1 commit into from
May 18, 2015

Conversation

carols10cents
Copy link
Member

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 #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...? 😳

This reverts commit 9c7d5ae.

This was wrong... the `continue` was to ignore the latter half of the
tokens file. Another mechanism will have to be used to keep the model
grammar's tokens in sync with the actual grammar's tokens :-/
@rust-highfive
Copy link
Contributor

r? @brson

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

@alexcrichton
Copy link
Member

@bors: r+ e70c858

No worries, thanks for the fix!

@alexcrichton
Copy link
Member

@bors: rollup

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...? 😳
bors added a commit that referenced this pull request May 18, 2015
@bors bors merged commit e70c858 into rust-lang:master May 18, 2015
@carols10cents carols10cents deleted the grammar-oops branch September 5, 2015 02:28
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