Improve parse errors for stray lifetimes in type position #139854
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Namely, use a more sensical primary span. Don't pretty-print AST nodes for the diagnostic message. Why: * It's lossy (e.g., it doesn't replicate trailing `+`s in trait objects. * It's prone to leak error nodes (printed as `(/*ERROR*/)`) since the LHS can easily represent recovered code (e.g., `fn(i32?) + T`).
|
r? @davidtwco rustbot has assigned @davidtwco. Use |
rustbot
added
S-waiting-on-review
fmease
changed the title
fmease
commented
Apr 15, 2025
| // 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.
I'm open to dropping the edition check making us emit the new message and output TyKind::Error in all editions (undoing parts of PR #69760), thereby removing NeedPlusAfterTraitObjectLifetime entirely.
There was a problem hiding this comment.
I'm open to also suggesting dyn 'a + /* Trait */ when facing 'a in modern editions, too.
This comment has been minimized.
This comment has been minimized.
fmease
force-pushed
the
modern-diag-for-lt-in-ty
branch
from
e9e8414 to
f8c5436
Compare
fmease
commented
Apr 15, 2025
Comment on lines
+414
to
+416
| // 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. |
There was a problem hiding this comment.
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 in tests/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)).
fmease
force-pushed
the
modern-diag-for-lt-in-ty
branch
from
f8c5436 to
6242335
Compare
|
@bors r+ |
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 17, 2025
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#135340 (Add `explicit_extern_abis` Feature and Enforce Explicit ABIs) - rust-lang#139440 (rustc_target: RISC-V: feature addition batch 2) - rust-lang#139667 (cfi: Remove #[no_sanitize(cfi)] for extern weak functions) - rust-lang#139828 (Don't require rigid alias's trait to hold) - rust-lang#139854 (Improve parse errors for stray lifetimes in type position) - rust-lang#139889 (Clean UI tests 3 of n) - rust-lang#139894 (Fix `opt-dist` CLI flag and make it work without LLD) - rust-lang#139900 (stepping into impls for normalization is unproductive) - rust-lang#139915 (replace some #[rustc_intrinsic] usage with use of the libcore declarations) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Apr 17, 2025
Rollup merge of rust-lang#139854 - fmease:modern-diag-for-lt-in-ty, r=davidtwco Improve parse errors for stray lifetimes in type position While technically & syntactically speaking lifetimes do begin[^1] types in type contexts (this essentially excludes generic argument lists) and require a following `+` to form a complete type (`'a +` denotes a bare trait object type), the likelihood that a user meant to write a lifetime-prefixed bare trait object type in *modern* editions (Rust ≥2021) when placing a lifetime into a type context is incredibly low (they would need to add at least three tokens to turn it into a *semantically* well-formed TOT: `'a` → `dyn 'a + Trait`). Therefore let's *lie* in modern editions (just like in PR rust-lang#131239, a precedent if you will) by stating "*expected type, found lifetime*" in such cases which is a lot more a approachable, digestible and friendly compared to "*lifetime in trait object type must be followed by `+`*" (as added in PR rust-lang#69760). I've also added recovery for "ampersand-less" reference types (e.g., `'a ()`, `'a mut Ty`) in modern editions because it was trivial to do and I think it's not unlikely to occur in practice. Fixes rust-lang#133413. [^1]: For example, in the context of decl macros, this implies that a lone `'a` always matches syntax fragment `ty` ("even if" there's a later macro matcher expecting syntax fragment `lifetime`). Rephrased, lifetimes (in type contexts) *commit* to the type parser.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
While technically & syntactically speaking lifetimes do begin1 types in type contexts (this essentially excludes generic argument lists) and require a following
+to form a complete type ('a +denotes a bare trait object type), the likelihood that a user meant to write a lifetime-prefixed bare trait object type in modern editions (Rust ≥2021) when placing a lifetime into a type context is incredibly low (they would need to add at least three tokens to turn it into a semantically well-formed TOT:'a→dyn 'a + Trait).Therefore let's lie in modern editions (just like in PR #131239, a precedent if you will) by stating "expected type, found lifetime" in such cases which is a lot more a approachable, digestible and friendly compared to "lifetime in trait object type must be followed by
+" (as added in PR #69760).I've also added recovery for "ampersand-less" reference types (e.g.,
'a (),'a mut Ty) in modern editions because it was trivial to do and I think it's not unlikely to occur in practice.Fixes #133413.
Footnotes
For example, in the context of decl macros, this implies that a lone
'aalways matches syntax fragmentty("even if" there's a later macro matcher expecting syntax fragmentlifetime). Rephrased, lifetimes (in type contexts) commit to the type parser. ↩