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

Refactor type & bounds parsing thoroughly #67148

Merged
merged 28 commits into from
Dec 22, 2019
Merged

Conversation

Centril
Copy link
Contributor

@Centril Centril commented Dec 8, 2019

PR is based on #67131 with first one from this PR being extract parse_ty_tuple_or_parens.

Also fixes #67146.

r? @estebank

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 8, 2019
self.error_opt_out_lifetime(question);
let bound = GenericBound::Outlives(self.expect_lifetime());
if has_parens {
// FIXME(Centril): Consider not erroring here and accepting `('lt)` instead,
Copy link
Contributor

@petrochenkov petrochenkov Dec 8, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

('lt) is entirely useless and it's some PITA to parse it in the first trait object position like type A = ('lt) + OtherBounds, so it isn't accepted anywhere.

@petrochenkov
Copy link
Contributor

I still feel like half of these changes (extracting small functions used once) make code readability worse, but you are the primary person working with this code now, so feel free to adjust it to your personal tastes basically.

@bors

This comment has been minimized.

@Centril
Copy link
Contributor Author

Centril commented Dec 20, 2019

#67131 has merged & I've rebased this PR so it should be ready for review now.

Copy link
Contributor

@estebank estebank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

r=me after addressing nitpicks (including ignoring them)

src/librustc_parse/parser/ty.rs Outdated Show resolved Hide resolved
src/librustc_parse/parser/ty.rs Outdated Show resolved Hide resolved
@Centril
Copy link
Contributor Author

Centril commented Dec 21, 2019

Addressed the comments in the two commits above, @bors r=estebank

@bors
Copy link
Contributor

bors commented Dec 21, 2019

📌 Commit 690b0b3 has been approved by estebank

@bors
Copy link
Contributor

bors commented Dec 21, 2019

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

@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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 21, 2019
Centril added a commit to Centril/rust that referenced this pull request Dec 21, 2019
 Refactor type & bounds parsing thoroughly

PR is based on rust-lang#67131 with first one from this PR being ` extract parse_ty_tuple_or_parens`.

Also fixes rust-lang#67146.

r? @estebank
@bors

This comment has been minimized.

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 21, 2019
@Centril
Copy link
Contributor Author

Centril commented Dec 21, 2019

@bors r=estebank

@bors
Copy link
Contributor

bors commented Dec 21, 2019

📌 Commit db4818f has been approved by estebank

@bors
Copy link
Contributor

bors commented Dec 21, 2019

🌲 The tree is currently closed for pull requests below priority 100, this pull request will be tested once the tree is reopened

@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 Dec 21, 2019
Centril added a commit to Centril/rust that referenced this pull request Dec 22, 2019
 Refactor type & bounds parsing thoroughly

PR is based on rust-lang#67131 with first one from this PR being ` extract parse_ty_tuple_or_parens`.

Also fixes rust-lang#67146.

r? @estebank
Centril added a commit to Centril/rust that referenced this pull request Dec 22, 2019
 Refactor type & bounds parsing thoroughly

PR is based on rust-lang#67131 with first one from this PR being ` extract parse_ty_tuple_or_parens`.

Also fixes rust-lang#67146.

r? @estebank
bors added a commit that referenced this pull request Dec 22, 2019
Rollup of 6 pull requests

Successful merges:

 - #67148 ( Refactor type & bounds parsing thoroughly)
 - #67410 (Reenable static linking of libstdc++ on windows-gnu)
 - #67439 (Cleanup `lower_pattern_unadjusted` & Improve slice pat typeck)
 - #67480 (Require issue = "none" over issue = "0" in unstable attributes)
 - #67500 (Tweak non_shorthand_field_patterns' suggestion)
 - #67504 (Warn against relying on ?Sized being last)

Failed merges:

r? @ghost
@bors bors merged commit db4818f into rust-lang:master Dec 22, 2019
@Centril Centril deleted the ty-polish branch December 22, 2019 11:38
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Feb 10, 2020
change an instance of span_bug() to struct_span_err() to avoid ICE

After rust-lang#67148, the `span_bug()` in `parse_ty_tuple_or_parens()` is reachable because `parse_paren_comma_seq()` can return an `Ok()` even in cases where it encounters an error.
This pull request prevents an ICE in such cases by replacing the `span_bug()` with `struct_span_error()`.

Fixes rust-lang#68890.
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.

Parser accepts negative lifetime bounds and interprets as positive bound
5 participants