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

syntax: cleanup param, method, and misc parsing #64910

Merged
merged 21 commits into from
Oct 2, 2019

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Sep 30, 2019

Do some misc cleanup of the parser:

Next up in a different PR:

r? @petrochenkov

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 30, 2019
@Centril Centril changed the title syntax: cleanup parameter and method parsing [WIP] syntax: cleanup parameter and method parsing Sep 30, 2019
@Centril Centril changed the title [WIP] syntax: cleanup parameter and method parsing [WIP] syntax: cleanup param, method, and misc parsing Sep 30, 2019
@Centril Centril changed the title [WIP] syntax: cleanup param, method, and misc parsing syntax: cleanup param, method, and misc parsing Sep 30, 2019
@petrochenkov
Copy link
Contributor

@Centril
Please, merge parse_self_param back into a single function.
Fragmenting implementation like this is harmful for readability.
I may be biased because I wrote it, but from my point of view it's written exactly like it should be (except for moving ifs on arms perhaps).

@petrochenkov
Copy link
Contributor

I think the end goal here is unified syntax and AST representation for free, trait, impl and foreign items (#48137) with inappropriate qualifiers or selfs, missing bodies, etc reported as semantic errors.

@petrochenkov
Copy link
Contributor

merge ... back into a single function

Same applies to parse_visibility, except perhaps for the diagnostics/recovery-only part.

src/libsyntax/parse/parser.rs Outdated Show resolved Hide resolved
src/libsyntax/parse/parser/item.rs Outdated Show resolved Hide resolved
@petrochenkov
Copy link
Contributor

Otherwise, LGTM.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 30, 2019
@bors
Copy link
Contributor

bors commented Oct 1, 2019

📌 Commit 5c5dd80 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 1, 2019
Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that rust-lang#64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing rust-lang#64252 -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that rust-lang#64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing rust-lang#64252 -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that rust-lang#64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing rust-lang#64252 -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that rust-lang#64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing rust-lang#64252 -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
bors added a commit that referenced this pull request Oct 1, 2019
Rollup of 5 pull requests

Successful merges:

 - #63416 (apfloat: improve doc comments)
 - #64404 (Add long error explanation for E0495)
 - #64910 (syntax: cleanup param, method, and misc parsing)
 - #64912 (Remove unneeded `fn main` blocks from docs)
 - #64952 (Update cargo.)

Failed merges:

r? @ghost
tmandry added a commit to tmandry/rust that referenced this pull request Oct 1, 2019
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that rust-lang#64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing rust-lang#64252 -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
tmandry added a commit to tmandry/rust that referenced this pull request Oct 1, 2019
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that rust-lang#64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing rust-lang#64252 -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that rust-lang#64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing rust-lang#64252 -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
Centril added a commit to Centril/rust that referenced this pull request Oct 1, 2019
syntax: cleanup param, method, and misc parsing

Do some misc cleanup of the parser:
- Method and parameter parsing is refactored.
- A parser for `const | mut` is introduced that rust-lang#64588 can reuse.
- Some other misc parsing.

Next up in a different PR:
- ~Implementing rust-lang#64252 -- maybe some other time...
- Heavily restructuring up `item.rs` which is a mess (hopefully, no promises ^^).

r? @petrochenkov
bors added a commit that referenced this pull request Oct 1, 2019
Rollup of 7 pull requests

Successful merges:

 - #63416 (apfloat: improve doc comments)
 - #64820 (BTreeSet intersection, is_subset & difference optimizations)
 - #64910 (syntax: cleanup param, method, and misc parsing)
 - #64912 (Remove unneeded `fn main` blocks from docs)
 - #64933 (Fixes #64919. Suggest fix based on operator precendence.)
 - #64943 (Add lower bound doctests for `saturating_{add,sub}` signed ints)
 - #64950 (Simplify interners)

Failed merges:

r? @ghost
@bors bors merged commit 5c5dd80 into rust-lang:master Oct 2, 2019
@Centril Centril deleted the params-cleanup branch October 2, 2019 16:24
Centril added a commit to Centril/rust that referenced this pull request Oct 8, 2019
syntax: more cleanups in item and function signature parsing

Follow up to rust-lang#64910.

Best read commit-by-commit.

r? @estebank
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants