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

Corrected EBNF grammar in the manual #14277

Merged
merged 1 commit into from May 20, 2014

Conversation

Projects
None yet
4 participants
@pczarn
Contributor

pczarn commented May 18, 2014

The grammar for use declarations was outdated. Corrected some other mistakes.

use_decl : "pub" ? "use" ident [ '=' path
| "::" path_glob ] ;
use_decl : "pub" ? "use" [ ident '=' path
| path_glob ] ;

This comment has been minimized.

@chris-morgan

chris-morgan May 19, 2014

Member

Um… what? These changes would make use; valid, which it is certainly not. A use statement must be followed by an identifier.

The grammar already existing appears to me to be correct.

@chris-morgan

chris-morgan May 19, 2014

Member

Um… what? These changes would make use; valid, which it is certainly not. A use statement must be followed by an identifier.

The grammar already existing appears to me to be correct.

This comment has been minimized.

@chris-morgan

chris-morgan May 19, 2014

Member

I take back what I said about the grammar existing being correct—I’ve been reading other grammar rules too much recently (the IETF RFC ones in particular) and was thinking […] meant optional. There is a problem with the current grammar, but the new one is wrong also. I think that just tacking a question mark after the ] on the existing grammar would be correct? OK, OK, that responsibility got shifted to the path_glob rule… not doing well today, am I?

@chris-morgan

chris-morgan May 19, 2014

Member

I take back what I said about the grammar existing being correct—I’ve been reading other grammar rules too much recently (the IETF RFC ones in particular) and was thinking […] meant optional. There is a problem with the current grammar, but the new one is wrong also. I think that just tacking a question mark after the ] on the existing grammar would be correct? OK, OK, that responsibility got shifted to the path_glob rule… not doing well today, am I?

This comment has been minimized.

@pczarn

pczarn May 19, 2014

Contributor

My line of thought is that a single identifier is also a path. path_glob can match multiple paths.

// Valid declarations
use ident = simple::path;
use simple::path;
use {many, idents}; // doesn't start with an ident!
// Invalid
use simple::path = another::path;
use *;
use;
@pczarn

pczarn May 19, 2014

Contributor

My line of thought is that a single identifier is also a path. path_glob can match multiple paths.

// Valid declarations
use ident = simple::path;
use simple::path;
use {many, idents}; // doesn't start with an ident!
// Invalid
use simple::path = another::path;
use *;
use;
Show outdated Hide outdated src/doc/rust.md
| '(' meta_seq ')' ] ? ;
meta_seq : meta_item [ ',' meta_seq ]* ;
meta_seq : meta_item [ ',' meta_seq ] ? ;

This comment has been minimized.

@chris-morgan

chris-morgan May 19, 2014

Member

This change is incorrect. Counterexample attribute: #[foo(bar, baz, quux)].

@chris-morgan

chris-morgan May 19, 2014

Member

This change is incorrect. Counterexample attribute: #[foo(bar, baz, quux)].

This comment has been minimized.

@chris-morgan

chris-morgan May 19, 2014

Member

Oh, hang on… recursive rule. Sorry.

@chris-morgan

chris-morgan May 19, 2014

Member

Oh, hang on… recursive rule. Sorry.

This comment has been minimized.

@pczarn

pczarn May 19, 2014

Contributor

Should I modify it further to a non-recursive one?

@pczarn

pczarn May 19, 2014

Contributor

Should I modify it further to a non-recursive one?

Show outdated Hide outdated src/doc/rust.md
@@ -1741,9 +1741,9 @@ import public items from their destination, not private items.
~~~~ {.notrust .ebnf .gram}
attribute : '#' '!' ? '[' meta_item ']' ;
meta_item : ident [ '=' literal
meta_item : ident [ '=' string_lit

This comment has been minimized.

@chris-morgan

chris-morgan May 19, 2014

Member

As far as the grammar is concerned, I think that literal may actually be correct, though the compiler will proceed to reject a literal which is not a string literal.

@chris-morgan

chris-morgan May 19, 2014

Member

As far as the grammar is concerned, I think that literal may actually be correct, though the compiler will proceed to reject a literal which is not a string literal.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@alexcrichton

alexcrichton May 20, 2014

Member

I think @chris-morgan is right about the meta_item grammar. Let's leave it as a literal for now instead of a string_lit, otherwise these look great to me, thanks!

Member

alexcrichton commented May 20, 2014

I think @chris-morgan is right about the meta_item grammar. Let's leave it as a literal for now instead of a string_lit, otherwise these look great to me, thanks!

Correct EBNF grammar in the manual
The grammar for use declarations was outdated.
@pczarn

This comment has been minimized.

Show comment
Hide comment
@pczarn

pczarn May 20, 2014

Contributor

Alright, it's done.

Contributor

pczarn commented May 20, 2014

Alright, it's done.

@alexcrichton

This comment has been minimized.

Show comment
Hide comment
@bors

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 20, 2014

Contributor

saw approval from alexcrichton
at pczarn@6674913

Contributor

bors commented on 6674913 May 20, 2014

saw approval from alexcrichton
at pczarn@6674913

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 20, 2014

Contributor

merging pczarn/rust/manual-grammar = 6674913 into auto

Contributor

bors replied May 20, 2014

merging pczarn/rust/manual-grammar = 6674913 into auto

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 20, 2014

Contributor

pczarn/rust/manual-grammar = 6674913 merged ok, testing candidate = 84320a4

Contributor

bors replied May 20, 2014

pczarn/rust/manual-grammar = 6674913 merged ok, testing candidate = 84320a4

This comment has been minimized.

Show comment
Hide comment
@bors

bors May 20, 2014

Contributor

fast-forwarding master to auto = 84320a4

Contributor

bors replied May 20, 2014

fast-forwarding master to auto = 84320a4

bors added a commit that referenced this pull request May 20, 2014

auto merge of #14277 : pczarn/rust/manual-grammar, r=alexcrichton
The grammar for use declarations was outdated. Corrected some other mistakes.

@bors bors closed this May 20, 2014

@bors bors merged commit 6674913 into rust-lang:master May 20, 2014

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default all tests passed

@pczarn pczarn deleted the pczarn:manual-grammar branch Jul 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment