E0229: Suggest Moving Type Constraints to Type Parameter Declaration#126054
Merged
bors merged 2 commits intorust-lang:masterfrom Jun 14, 2024
Merged
Conversation
Collaborator
|
r? @fee1-dead rustbot has assigned @fee1-dead. Use |
rustbot
added
S-waiting-on-review
Collaborator
|
HIR ty lowering was modified cc @fmease |
veera-sivarajan
commented
Jun 6, 2024
| let generics = tcx.generics_of(def_id); | ||
| let matching_param = | ||
| generics.own_params.iter().find(|p| p.name.as_str() == constraint.ident.as_str()); | ||
| let matching_param = generics.own_params.iter().find(|p| p.name == constraint.ident.name); |
Contributor
Author
There was a problem hiding this comment.
Also, changed this because comparing interned strings is more efficient
fmease
reviewed
Jun 6, 2024
veera-sivarajan
force-pushed
the
bugfix-113073-bound-on-generics-2
branch
from
acf90a8 to
047afa3
Compare
fee1-dead
requested changes
Jun 12, 2024
Member
fee1-dead
left a comment
fee1-dead
left a comment
There was a problem hiding this comment.
This looks really good, thanks for implementing it! Had one small nit.
rustbot
added
S-waiting-on-author
veera-sivarajan
force-pushed
the
bugfix-113073-bound-on-generics-2
branch
from
047afa3 to
5da1b41
Compare
Contributor
Author
|
@rustbot ready |
rustbot
added
S-waiting-on-review
fee1-dead
approved these changes
Jun 14, 2024
Comment on lines
+1
to
+29
| error[E0261]: use of undeclared lifetime name `'a` | ||
| --> $DIR/impl-block-params-declared-in-wrong-spot-issue-113073.rs:7:13 | ||
| | | ||
| LL | impl Foo<T: 'a + Default> for u8 {} | ||
| | - ^^ undeclared lifetime | ||
| | | | ||
| | help: consider introducing lifetime `'a` here: `<'a>` | ||
|
|
||
| error[E0229]: associated item constraints are not allowed here | ||
| --> $DIR/impl-block-params-declared-in-wrong-spot-issue-113073.rs:3:10 | ||
| | | ||
| LL | impl Foo<T: Default> for String {} | ||
| | ^^^^^^^^^^ associated item constraint not allowed here | ||
| | | ||
| help: declare the type parameter right after the `impl` keyword | ||
| | | ||
| LL | impl<T: Default> Foo<T> for String {} | ||
| | ++++++++++++ ~ | ||
|
|
||
| error[E0229]: associated item constraints are not allowed here | ||
| --> $DIR/impl-block-params-declared-in-wrong-spot-issue-113073.rs:7:10 | ||
| | | ||
| LL | impl Foo<T: 'a + Default> for u8 {} | ||
| | ^^^^^^^^^^^^^^^ associated item constraint not allowed here | ||
| | | ||
| help: declare the type parameter right after the `impl` keyword | ||
| | | ||
| LL | impl<'a, T: 'a + Default> Foo<T> for u8 {} | ||
| | +++++++++++++++++++++ ~ |
Member
There was a problem hiding this comment.
We might want to silence E0261 as well, since the E0229 would be the best suggestion in this case. Something to do in the future I suppose.
Member
|
@bors r+ |
Collaborator
bors
added
S-waiting-on-bors
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 14, 2024
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#123769 (Improve escaping of byte, byte str, and c str proc-macro literals) - rust-lang#126054 (`E0229`: Suggest Moving Type Constraints to Type Parameter Declaration) - rust-lang#126135 (add HermitOS support for vectored read/write operations) - rust-lang#126266 (Unify guarantees about the default allocator) - rust-lang#126285 (`UniqueRc`: support allocators and `T: ?Sized`.) - rust-lang#126399 (extend the check for LLVM build) - rust-lang#126426 (const validation: fix ICE on dangling ZST reference) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Jun 14, 2024
Rollup merge of rust-lang#126054 - veera-sivarajan:bugfix-113073-bound-on-generics-2, r=fee1-dead `E0229`: Suggest Moving Type Constraints to Type Parameter Declaration Fixes rust-lang#113073 This PR suggests `impl<T: Bound> Trait<T> for Foo` when finding `impl Trait<T: Bound> for Foo`. Tangentially, it also improves a handful of other error messages. It accomplishes this in two steps: 1. Check if constrained arguments and parameter names appear in the same order and delay emitting "incorrect number of generic arguments" error because it can be confusing for the programmer to see `0 generic arguments provided` when there are `n` constrained generic arguments. 2. Inside `E0229`, suggest declaring the type parameter right after the `impl` keyword by finding the relevant impl block's span for type parameter declaration. This also handles lifetime declarations correctly. Also, the multi part suggestion doesn't use the fluent error mechanism because translating all the errors to fluent style feels outside the scope of this PR. I will handle it in a separate PR if this gets approved.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
Aug 6, 2024
…dtwco Fix ICE Caused by Incorrectly Delaying E0107 Fixes rust-lang#128249 For the following code: ```rust trait Foo<T> {} impl Foo<T: Default> for u8 {} ``` rust-lang#126054 added some logic to delay emitting E0107 as the names of associated type `T` in the impl header and generic parameter `T` in `trait Foo` match. But it failed to ensure whether such unexpected associated type bounds are coming from a impl block header. This caused an ICE as the compiler was delaying E0107 for code like: ```rust trait Trait<Type> { type Type; fn method(&self) -> impl Trait<Type: '_>; } ``` because it assumed the associated type bound `Type: '_` is for the generic parameter `Type` in `trait Trait` since the names are same. This PR adds a check to ensure that E0107 is delayed only in the context of impl block header.
rust-timer
added a commit
to rust-lang-ci/rust
that referenced
this pull request
Aug 7, 2024
Rollup merge of rust-lang#128377 - veera-sivarajan:fix-128249, r=davidtwco Fix ICE Caused by Incorrectly Delaying E0107 Fixes rust-lang#128249 For the following code: ```rust trait Foo<T> {} impl Foo<T: Default> for u8 {} ``` rust-lang#126054 added some logic to delay emitting E0107 as the names of associated type `T` in the impl header and generic parameter `T` in `trait Foo` match. But it failed to ensure whether such unexpected associated type bounds are coming from a impl block header. This caused an ICE as the compiler was delaying E0107 for code like: ```rust trait Trait<Type> { type Type; fn method(&self) -> impl Trait<Type: '_>; } ``` because it assumed the associated type bound `Type: '_` is for the generic parameter `Type` in `trait Trait` since the names are same. This PR adds a check to ensure that E0107 is delayed only in the context of impl block header.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Fixes #113073
This PR suggests
impl<T: Bound> Trait<T> for Foowhen findingimpl Trait<T: Bound> for Foo. Tangentially, it also improves a handful of other error messages.It accomplishes this in two steps:
Check if constrained arguments and parameter names appear in the same order and delay emitting "incorrect number of generic arguments" error because it can be confusing for the programmer to see
0 generic arguments providedwhen there arenconstrained generic arguments.Inside
E0229, suggest declaring the type parameter right after theimplkeyword by finding the relevant impl block's span for type parameter declaration. This also handles lifetime declarations correctly.Also, the multi part suggestion doesn't use the fluent error mechanism because translating all the errors to fluent style feels outside the scope of this PR. I will handle it in a separate PR if this gets approved.