Skip to content

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
Collaborator

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
@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