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

Move binder and polarity parsing into parse_generic_ty_bound #127103

Merged
merged 1 commit into from
Jun 29, 2024

Conversation

compiler-errors
Copy link
Member

Let's pull out the parts of #127054 which just:

  1. Make the parsing code less confusing
  2. Fix ?use<> (to correctly be denied)
  3. Improve T: for<'a> 'a diagnostics

This should have no user-facing effects on stable parsing.

r? fmease

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 28, 2024
#![feature(precise_capturing)]

fn polarity() -> impl Sized + ?use<> {}
//~^ ERROR expected identifier, found keyword `use`
Copy link
Member Author

Choose a reason for hiding this comment

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

I chose against generalizing error_lt_bound_with_modifiers for use<> precise capturing syntax. I guess I could? Do you think anyone would really ever write ?use<> or for<'a> use<'a>??

Copy link
Member

Choose a reason for hiding this comment

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

I doubt it

Comment on lines +991 to +992
let modifiers = self.parse_trait_bound_modifiers()?;
let (mut lifetime_defs, binder_span) = self.parse_late_bound_lifetime_defs()?;
Copy link
Member Author

Choose a reason for hiding this comment

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

This should make the diff of #127054 really simple -- just invert these two lines, fixup some comments, and bless tests.

= note: for more information on higher-ranked polymorphism, visit https://doc.rust-lang.org/nomicon/hrtb.html
help: consider making the bound lifetime-generic with a new `'a` lifetime
|
LL | fn foo<T>() where for<'a> T: for<'a> 'a {}
Copy link
Member Author

@compiler-errors compiler-errors Jun 28, 2024

Choose a reason for hiding this comment

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

lol this kinda sucks tho -- maybe i should bail the parser or something so we don't get to resolution here.

Copy link
Member

@fmease fmease Jun 29, 2024

Choose a reason for hiding this comment

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

Yeah, I guess we could do that. Don't think it's super important. Can be addressed at some other point(tm)

Copy link
Member

@fmease fmease left a comment

Choose a reason for hiding this comment

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

Much appreciated, thanks :)

r=me with concern addressed

#![feature(precise_capturing)]

fn polarity() -> impl Sized + ?use<> {}
//~^ ERROR expected identifier, found keyword `use`
Copy link
Member

Choose a reason for hiding this comment

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

I doubt it

Comment on lines +996 to +999
if self.token.is_lifetime() {
let _: ErrorGuaranteed = self.error_lt_bound_with_modifiers(modifiers, binder_span);
return self.parse_generic_lt_bound(lo, has_parens);
}
Copy link
Member

@fmease fmease Jun 29, 2024

Choose a reason for hiding this comment

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

Does this regress the following valid code (semantically valid 2015–2018, syntactically valid >2018):

fn f() where for<'a> 'a + std::fmt::Debug: {}

If so, I remember that there are some helper methods with which you can "look ahead for trailing a +"

Copy link
Member

@fmease fmease Jun 29, 2024

Choose a reason for hiding this comment

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

Ah, no. The for<> belongs to the predicate, not to the type. So it should be fine and likely doesn't regress it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, and fn f() where (for<'a> 'a + std::fmt::Debug): {} is not valid

@compiler-errors
Copy link
Member Author

I checked and that code you shared does not regress

@bors r=fmease

@bors
Copy link
Contributor

bors commented Jun 29, 2024

📌 Commit 3bc3247 has been approved by fmease

It is now in the queue for this repository.

@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 Jun 29, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 29, 2024
…iaskrgr

Rollup of 9 pull requests

Successful merges:

 - rust-lang#126822 (Bootstrap command refactoring: port more `Command` usages to `BootstrapCmd` (step 2))
 - rust-lang#126835 (Simplifications in match lowering)
 - rust-lang#126953 (std: separate TLS key creation from TLS access)
 - rust-lang#127045 (Rename `super_predicates_of` and similar queries to `explicit_*` to note that they're not elaborated)
 - rust-lang#127075 (rustc_data_structures: Explicitly check for 64-bit atomics support)
 - rust-lang#127101 (remove redundant match statement from dataflow const prop)
 - rust-lang#127102 (Rename fuchsia builder and bump Fuchsia)
 - rust-lang#127103 (Move binder and polarity parsing into `parse_generic_ty_bound`)
 - rust-lang#127108 (unify `dylib` and `bin_helpers` and create `shared_helpers::parse_value_from_args`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit a4e92bf into rust-lang:master Jun 29, 2024
6 checks passed
@rustbot rustbot added this to the 1.81.0 milestone Jun 29, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Jun 29, 2024
Rollup merge of rust-lang#127103 - compiler-errors:tighten-trait-bound-parsing, r=fmease

Move binder and polarity parsing into `parse_generic_ty_bound`

Let's pull out the parts of rust-lang#127054 which just:
1. Make the parsing code less confusing
2. Fix `?use<>` (to correctly be denied)
3. Improve `T: for<'a> 'a` diagnostics

This should have no user-facing effects on stable parsing.

r? fmease
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. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants