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
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2327,7 +2327,7 @@ impl<'a> Parser<'a> {
let before = self.prev_token.clone();
let binder = if self.check_keyword(kw::For) {
let lo = self.token.span;
let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
let (lifetime_defs, _) = self.parse_late_bound_lifetime_defs()?;
let span = lo.to(self.prev_token.span);

self.psess.gated_spans.gate(sym::closure_lifetime_binder, span);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/generics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ impl<'a> Parser<'a> {
// * `for<'a> Trait1<'a>: Trait2<'a /* ok */>`
// * `(for<'a> Trait1<'a>): Trait2<'a /* not ok */>`
// * `for<'a> for<'b> Trait1<'a, 'b>: Trait2<'a /* ok */, 'b /* not ok */>`
let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
let (lifetime_defs, _) = self.parse_late_bound_lifetime_defs()?;

// Parse type with mandatory colon and (possibly empty) bounds,
// or with mandatory equality sign and the second type.
Expand Down
98 changes: 60 additions & 38 deletions compiler/rustc_parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use rustc_ast::{
};
use rustc_errors::{Applicability, PResult};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::{Span, Symbol};
use rustc_span::{ErrorGuaranteed, Span, Symbol};
use thin_vec::{thin_vec, ThinVec};

#[derive(Copy, Clone, PartialEq)]
Expand Down Expand Up @@ -280,7 +280,7 @@ impl<'a> Parser<'a> {
// Function pointer type or bound list (trait object type) starting with a poly-trait.
// `for<'lt> [unsafe] [extern "ABI"] fn (&'lt S) -> T`
// `for<'lt> Trait1<'lt> + Trait2 + 'a`
let lifetime_defs = self.parse_late_bound_lifetime_defs()?;
let (lifetime_defs, _) = self.parse_late_bound_lifetime_defs()?;
if self.check_fn_front_matter(false, Case::Sensitive) {
self.parse_ty_bare_fn(
lo,
Expand Down Expand Up @@ -833,12 +833,9 @@ impl<'a> Parser<'a> {
let lo = self.token.span;
let leading_token = self.prev_token.clone();
let has_parens = self.eat(&token::OpenDelim(Delimiter::Parenthesis));
let inner_lo = self.token.span;

let modifiers = self.parse_trait_bound_modifiers()?;
let bound = if self.token.is_lifetime() {
self.error_lt_bound_with_modifiers(modifiers);
self.parse_generic_lt_bound(lo, inner_lo, has_parens)?
self.parse_generic_lt_bound(lo, has_parens)?
} else if self.eat_keyword(kw::Use) {
// parse precise captures, if any. This is `use<'lt, 'lt, P, P>`; a list of
// lifetimes and ident params (including SelfUpper). These are validated later
Expand All @@ -848,7 +845,7 @@ impl<'a> Parser<'a> {
let (args, args_span) = self.parse_precise_capturing_args()?;
GenericBound::Use(args, use_span.to(args_span))
} else {
self.parse_generic_ty_bound(lo, has_parens, modifiers, &leading_token)?
self.parse_generic_ty_bound(lo, has_parens, &leading_token)?
};

Ok(bound)
Expand All @@ -858,50 +855,64 @@ impl<'a> Parser<'a> {
/// ```ebnf
/// LT_BOUND = LIFETIME
/// ```
fn parse_generic_lt_bound(
&mut self,
lo: Span,
inner_lo: Span,
has_parens: bool,
) -> PResult<'a, GenericBound> {
let bound = GenericBound::Outlives(self.expect_lifetime());
fn parse_generic_lt_bound(&mut self, lo: Span, has_parens: bool) -> PResult<'a, GenericBound> {
let lt = self.expect_lifetime();
let bound = GenericBound::Outlives(lt);
if has_parens {
// FIXME(Centril): Consider not erroring here and accepting `('lt)` instead,
// possibly introducing `GenericBound::Paren(P<GenericBound>)`?
self.recover_paren_lifetime(lo, inner_lo)?;
self.recover_paren_lifetime(lo, lt.ident.span)?;
}
Ok(bound)
}

/// Emits an error if any trait bound modifiers were present.
fn error_lt_bound_with_modifiers(&self, modifiers: TraitBoundModifiers) {
match modifiers.constness {
fn error_lt_bound_with_modifiers(
&self,
modifiers: TraitBoundModifiers,
binder_span: Option<Span>,
) -> ErrorGuaranteed {
let TraitBoundModifiers { constness, asyncness, polarity } = modifiers;

match constness {
BoundConstness::Never => {}
BoundConstness::Always(span) | BoundConstness::Maybe(span) => {
self.dcx().emit_err(errors::ModifierLifetime {
span,
modifier: modifiers.constness.as_str(),
});
return self
.dcx()
.emit_err(errors::ModifierLifetime { span, modifier: constness.as_str() });
}
}

match modifiers.polarity {
match polarity {
BoundPolarity::Positive => {}
BoundPolarity::Negative(span) | BoundPolarity::Maybe(span) => {
self.dcx().emit_err(errors::ModifierLifetime {
span,
modifier: modifiers.polarity.as_str(),
});
return self
.dcx()
.emit_err(errors::ModifierLifetime { span, modifier: polarity.as_str() });
}
}

match asyncness {
BoundAsyncness::Normal => {}
BoundAsyncness::Async(span) => {
return self
.dcx()
.emit_err(errors::ModifierLifetime { span, modifier: asyncness.as_str() });
}
}

if let Some(span) = binder_span {
return self.dcx().emit_err(errors::ModifierLifetime { span, modifier: "for<...>" });
}

unreachable!("lifetime bound intercepted in `parse_generic_ty_bound` but no modifiers?")
}

/// Recover on `('lifetime)` with `(` already eaten.
fn recover_paren_lifetime(&mut self, lo: Span, inner_lo: Span) -> PResult<'a, ()> {
let inner_span = inner_lo.to(self.prev_token.span);
fn recover_paren_lifetime(&mut self, lo: Span, lt_span: Span) -> PResult<'a, ()> {
self.expect(&token::CloseDelim(Delimiter::Parenthesis))?;
let span = lo.to(self.prev_token.span);
let (sugg, snippet) = if let Ok(snippet) = self.span_to_snippet(inner_span) {
let (sugg, snippet) = if let Ok(snippet) = self.span_to_snippet(lt_span) {
(Some(span), snippet)
} else {
(None, String::new())
Expand All @@ -916,7 +927,7 @@ impl<'a> Parser<'a> {
/// If no modifiers are present, this does not consume any tokens.
///
/// ```ebnf
/// TRAIT_BOUND_MODIFIERS = [["~"] "const"] ["?" | "!"]
/// TRAIT_BOUND_MODIFIERS = [["~"] "const"] ["async"] ["?" | "!"]
/// ```
fn parse_trait_bound_modifiers(&mut self) -> PResult<'a, TraitBoundModifiers> {
let constness = if self.eat(&token::Tilde) {
Expand Down Expand Up @@ -970,15 +981,23 @@ impl<'a> Parser<'a> {
/// TY_BOUND_NOPAREN = [TRAIT_BOUND_MODIFIERS] [for<LT_PARAM_DEFS>] SIMPLE_PATH
/// ```
///
/// For example, this grammar accepts `~const ?for<'a: 'b> m::Trait<'a>`.
/// For example, this grammar accepts `for<'a: 'b> ~const ?m::Trait<'a>`.
fn parse_generic_ty_bound(
&mut self,
lo: Span,
has_parens: bool,
modifiers: TraitBoundModifiers,
leading_token: &Token,
) -> PResult<'a, GenericBound> {
let mut lifetime_defs = self.parse_late_bound_lifetime_defs()?;
let modifiers = self.parse_trait_bound_modifiers()?;
let (mut lifetime_defs, binder_span) = self.parse_late_bound_lifetime_defs()?;
Comment on lines +991 to +992
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.


// Recover erroneous lifetime bound with modifiers or binder.
// e.g. `T: for<'a> 'a` or `T: ~const 'a`.
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);
}
Comment on lines +996 to +999
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


let mut path = if self.token.is_keyword(kw::Fn)
&& self.look_ahead(1, |tok| tok.kind == TokenKind::OpenDelim(Delimiter::Parenthesis))
&& let Some(path) = self.recover_path_from_fn()
Expand Down Expand Up @@ -1094,16 +1113,19 @@ impl<'a> Parser<'a> {
}

/// Optionally parses `for<$generic_params>`.
pub(super) fn parse_late_bound_lifetime_defs(&mut self) -> PResult<'a, ThinVec<GenericParam>> {
pub(super) fn parse_late_bound_lifetime_defs(
&mut self,
) -> PResult<'a, (ThinVec<GenericParam>, Option<Span>)> {
if self.eat_keyword(kw::For) {
let lo = self.token.span;
self.expect_lt()?;
let params = self.parse_generic_params()?;
self.expect_gt()?;
// We rely on AST validation to rule out invalid cases: There must not be type
// parameters, and the lifetime parameters must not have bounds.
Ok(params)
// We rely on AST validation to rule out invalid cases: There must not be
// type or const parameters, and parameters must not have bounds.
Ok((params, Some(lo.to(self.prev_token.span))))
} else {
Ok(ThinVec::new())
Ok((ThinVec::new(), None))
}
}

Expand Down
5 changes: 5 additions & 0 deletions tests/ui/higher-ranked/erroneous-lifetime-bound.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fn foo<T>() where T: for<'a> 'a {}
//~^ ERROR `for<...>` may only modify trait bounds, not lifetime bounds
//~| ERROR use of undeclared lifetime name `'a` [E0261]

fn main() {}
25 changes: 25 additions & 0 deletions tests/ui/higher-ranked/erroneous-lifetime-bound.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
error: `for<...>` may only modify trait bounds, not lifetime bounds
--> $DIR/erroneous-lifetime-bound.rs:1:25
|
LL | fn foo<T>() where T: for<'a> 'a {}
| ^^^^

error[E0261]: use of undeclared lifetime name `'a`
--> $DIR/erroneous-lifetime-bound.rs:1:30
|
LL | fn foo<T>() where T: for<'a> 'a {}
| ^^ undeclared lifetime
|
= 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)

| +++++++
help: consider introducing lifetime `'a` here
|
LL | fn foo<'a, T>() where T: for<'a> 'a {}
| +++

error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0261`.
25 changes: 25 additions & 0 deletions tests/ui/impl-trait/precise-capturing/bound-modifiers.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//@ edition: 2021

#![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

//~| ERROR cannot find trait `r#use` in this scope
//~| WARN relaxing a default bound only does something for `?Sized`
//~| WARN relaxing a default bound only does something for `?Sized`

fn asyncness() -> impl Sized + async use<> {}
//~^ ERROR expected identifier, found keyword `use`
//~| ERROR cannot find trait `r#use` in this scope
//~| ERROR async closures are unstable

fn constness() -> impl Sized + const use<> {}
//~^ ERROR expected identifier, found keyword `use`
//~| ERROR cannot find trait `r#use` in this scope
//~| ERROR const trait impls are experimental

fn binder() -> impl Sized + for<'a> use<> {}
//~^ ERROR expected identifier, found keyword `use`
//~| ERROR cannot find trait `r#use` in this scope

fn main() {}
87 changes: 87 additions & 0 deletions tests/ui/impl-trait/precise-capturing/bound-modifiers.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
error: expected identifier, found keyword `use`
--> $DIR/bound-modifiers.rs:5:32
|
LL | fn polarity() -> impl Sized + ?use<> {}
| ^^^ expected identifier, found keyword

error: expected identifier, found keyword `use`
--> $DIR/bound-modifiers.rs:11:38
|
LL | fn asyncness() -> impl Sized + async use<> {}
| ^^^ expected identifier, found keyword

error: expected identifier, found keyword `use`
--> $DIR/bound-modifiers.rs:16:38
|
LL | fn constness() -> impl Sized + const use<> {}
| ^^^ expected identifier, found keyword

error: expected identifier, found keyword `use`
--> $DIR/bound-modifiers.rs:21:37
|
LL | fn binder() -> impl Sized + for<'a> use<> {}
| ^^^ expected identifier, found keyword

error[E0405]: cannot find trait `r#use` in this scope
--> $DIR/bound-modifiers.rs:5:32
|
LL | fn polarity() -> impl Sized + ?use<> {}
| ^^^ not found in this scope

error[E0405]: cannot find trait `r#use` in this scope
--> $DIR/bound-modifiers.rs:11:38
|
LL | fn asyncness() -> impl Sized + async use<> {}
| ^^^ not found in this scope

error[E0405]: cannot find trait `r#use` in this scope
--> $DIR/bound-modifiers.rs:16:38
|
LL | fn constness() -> impl Sized + const use<> {}
| ^^^ not found in this scope

error[E0405]: cannot find trait `r#use` in this scope
--> $DIR/bound-modifiers.rs:21:37
|
LL | fn binder() -> impl Sized + for<'a> use<> {}
| ^^^ not found in this scope

error[E0658]: async closures are unstable
--> $DIR/bound-modifiers.rs:11:32
|
LL | fn asyncness() -> impl Sized + async use<> {}
| ^^^^^
|
= note: see issue #62290 <https://github.com/rust-lang/rust/issues/62290> for more information
= help: add `#![feature(async_closure)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date
= help: to use an async block, remove the `||`: `async {`

error[E0658]: const trait impls are experimental
--> $DIR/bound-modifiers.rs:16:32
|
LL | fn constness() -> impl Sized + const use<> {}
| ^^^^^
|
= note: see issue #67792 <https://github.com/rust-lang/rust/issues/67792> for more information
= help: add `#![feature(const_trait_impl)]` to the crate attributes to enable
= note: this compiler was built on YYYY-MM-DD; consider upgrading it if it is out of date

warning: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
--> $DIR/bound-modifiers.rs:5:31
|
LL | fn polarity() -> impl Sized + ?use<> {}
| ^^^^^^

warning: relaxing a default bound only does something for `?Sized`; all other traits are not bound by default
--> $DIR/bound-modifiers.rs:5:31
|
LL | fn polarity() -> impl Sized + ?use<> {}
| ^^^^^^
|
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no`

error: aborting due to 10 previous errors; 2 warnings emitted

Some errors have detailed explanations: E0405, E0658.
For more information about an error, try `rustc --explain E0405`.
Loading