-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Improve parse errors for stray lifetimes in type position #139854
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ use rustc_ast::{ | |
Pinnedness, PolyTraitRef, PreciseCapturingArg, TraitBoundModifiers, TraitObjectSyntax, Ty, | ||
TyKind, UnsafeBinderTy, | ||
}; | ||
use rustc_errors::{Applicability, PResult}; | ||
use rustc_errors::{Applicability, Diag, PResult}; | ||
use rustc_span::{ErrorGuaranteed, Ident, Span, kw, sym}; | ||
use thin_vec::{ThinVec, thin_vec}; | ||
|
||
|
@@ -411,6 +411,9 @@ impl<'a> Parser<'a> { | |
TyKind::Path(None, path) if maybe_bounds => { | ||
self.parse_remaining_bounds_path(ThinVec::new(), path, lo, true) | ||
} | ||
// For `('a) + …`, we know that `'a` in type position already lead to an error being | ||
// emitted. To reduce output, let's indirectly suppress E0178 (bad `+` in type) and | ||
// other irrelevant consequential errors. | ||
TyKind::TraitObject(bounds, TraitObjectSyntax::None) | ||
if maybe_bounds && bounds.len() == 1 && !trailing_plus => | ||
{ | ||
|
@@ -425,12 +428,60 @@ impl<'a> Parser<'a> { | |
} | ||
|
||
fn parse_bare_trait_object(&mut self, lo: Span, allow_plus: AllowPlus) -> PResult<'a, TyKind> { | ||
let lt_no_plus = self.check_lifetime() && !self.look_ahead(1, |t| t.is_like_plus()); | ||
let bounds = self.parse_generic_bounds_common(allow_plus)?; | ||
if lt_no_plus { | ||
self.dcx().emit_err(NeedPlusAfterTraitObjectLifetime { span: lo }); | ||
// A lifetime only begins a bare trait object type if it is followed by `+`! | ||
if self.token.is_lifetime() && !self.look_ahead(1, |t| t.is_like_plus()) { | ||
// In Rust 2021 and beyond, we assume that the user didn't intend to write a bare trait | ||
// object type with a leading lifetime bound since that seems very unlikely given the | ||
// fact that `dyn`-less trait objects are *semantically* invalid. | ||
if self.psess.edition.at_least_rust_2021() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm open to dropping the edition check making us emit the new message and output There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm open to also suggesting |
||
let lt = self.expect_lifetime(); | ||
let mut err = self.dcx().struct_span_err(lo, "expected type, found lifetime"); | ||
err.span_label(lo, "expected type"); | ||
return Ok(match self.maybe_recover_ref_ty_no_leading_ampersand(lt, lo, err) { | ||
Ok(ref_ty) => ref_ty, | ||
Err(err) => TyKind::Err(err.emit()), | ||
}); | ||
} | ||
|
||
self.dcx().emit_err(NeedPlusAfterTraitObjectLifetime { | ||
span: lo, | ||
suggestion: lo.shrink_to_hi(), | ||
}); | ||
} | ||
Ok(TyKind::TraitObject( | ||
self.parse_generic_bounds_common(allow_plus)?, | ||
TraitObjectSyntax::None, | ||
)) | ||
} | ||
|
||
fn maybe_recover_ref_ty_no_leading_ampersand<'cx>( | ||
&mut self, | ||
lt: Lifetime, | ||
lo: Span, | ||
mut err: Diag<'cx>, | ||
) -> Result<TyKind, Diag<'cx>> { | ||
if !self.may_recover() { | ||
return Err(err); | ||
} | ||
let snapshot = self.create_snapshot_for_diagnostic(); | ||
let mutbl = self.parse_mutability(); | ||
match self.parse_ty_no_plus() { | ||
Ok(ty) => { | ||
err.span_suggestion_verbose( | ||
lo.shrink_to_lo(), | ||
"you might have meant to write a reference type here", | ||
"&", | ||
Applicability::MaybeIncorrect, | ||
); | ||
err.emit(); | ||
Ok(TyKind::Ref(Some(lt), MutTy { ty, mutbl })) | ||
} | ||
Err(diag) => { | ||
diag.cancel(); | ||
self.restore_snapshot(snapshot); | ||
Err(err) | ||
} | ||
} | ||
Ok(TyKind::TraitObject(bounds, TraitObjectSyntax::None)) | ||
} | ||
|
||
fn parse_remaining_bounds_path( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
error: lifetimes must be followed by `+` to form a trait object type | ||
--> $DIR/trait-object-macro-matcher.rs:17:8 | ||
| | ||
LL | m!('static); | ||
| ^^^^^^^ | ||
| | ||
help: consider adding a trait bound after the potential lifetime bound | ||
| | ||
LL | m!('static + /* Trait */); | ||
| +++++++++++++ | ||
|
||
error: lifetimes must be followed by `+` to form a trait object type | ||
--> $DIR/trait-object-macro-matcher.rs:17:8 | ||
| | ||
LL | m!('static); | ||
| ^^^^^^^ | ||
| | ||
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` | ||
help: consider adding a trait bound after the potential lifetime bound | ||
| | ||
LL | m!('static + /* Trait */); | ||
| +++++++++++++ | ||
|
||
error[E0224]: at least one trait is required for an object type | ||
--> $DIR/trait-object-macro-matcher.rs:17:8 | ||
| | ||
LL | m!('static); | ||
| ^^^^^^^ | ||
|
||
error: aborting due to 3 previous errors | ||
|
||
For more information about this error, try `rustc --explain E0224`. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
error: expected type, found lifetime | ||
--> $DIR/trait-object-macro-matcher.rs:17:8 | ||
| | ||
LL | m!('static); | ||
| ^^^^^^^ expected type | ||
|
||
error: expected type, found lifetime | ||
--> $DIR/trait-object-macro-matcher.rs:17:8 | ||
| | ||
LL | m!('static); | ||
| ^^^^^^^ expected type | ||
| | ||
= note: duplicate diagnostic emitted due to `-Z deduplicate-diagnostics=no` | ||
|
||
error: aborting due to 2 previous errors | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,21 @@ | ||
// A single lifetime is not parsed as a type. | ||
// `ty` matcher in particular doesn't accept a single lifetime | ||
|
||
//@ revisions: e2015 e2021 | ||
//@[e2015] edition: 2015 | ||
//@[e2021] edition: 2021 | ||
|
||
macro_rules! m { | ||
($t: ty) => { | ||
let _: $t; | ||
}; | ||
} | ||
|
||
fn main() { | ||
//[e2021]~vv ERROR expected type, found lifetime | ||
//[e2021]~v ERROR expected type, found lifetime | ||
m!('static); | ||
//~^ ERROR lifetime in trait object type must be followed by `+` | ||
//~| ERROR lifetime in trait object type must be followed by `+` | ||
//~| ERROR at least one trait is required for an object type | ||
//[e2015]~^ ERROR lifetimes must be followed by `+` to form a trait object type | ||
//[e2015]~| ERROR lifetimes must be followed by `+` to form a trait object type | ||
//[e2015]~| ERROR at least one trait is required for an object type | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just an explainer for what's already going on here.
In commit fmease@f8c5436 which I've since removed from this PR, I've experimented with removing this which would've allowed us to suggest removing parentheses in
maybe_recover_from_bad_type_plus
for type('a) + Bounds…
(that's a FIXME intests/ui/parser/trait-object-lifetime-parens.rs
) but that made things worse (more verbose output, nasty boolean flag, still false-positives (e.g.,(?'a) + Bound
where?
is invalid but we recover which we can't detect later)).