`E0229`: Suggest Moving Type Constraints to Type Parameter Declaration #126054

veera-sivarajan · 2024-06-06T00:41:16Z

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:

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

rustbot · 2024-06-06T00:41:24Z

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-06-06T00:41:25Z

HIR ty lowering was modified

cc @fmease

veera-sivarajan · 2024-06-06T00:44:49Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

@@ -1286,20 +1285,74 @@ pub fn prohibit_assoc_item_constraint(
        // Check if the type has a generic param with the same name
        // as the assoc type name in the associated item binding.
        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);


Also, changed this because comparing interned strings is more efficient

tests/ui/associated-type-bounds/no-gat-position.stderr

fee1-dead

This looks really good, thanks for implementing it! Had one small nit.

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs

veera-sivarajan · 2024-06-13T00:32:01Z

@rustbot ready

fee1-dead

LGTM!

fee1-dead · 2024-06-14T06:09:19Z

tests/ui/generics/impl-block-params-declared-in-wrong-spot-issue-113073.stderr

+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 {}
+   |     +++++++++++++++++++++     ~


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.

fee1-dead · 2024-06-14T06:09:36Z

@bors r+

bors · 2024-06-14T06:09:39Z

📌 Commit 5da1b41 has been approved by fee1-dead

It is now in the queue for this repository.

…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

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.

…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.

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.

Update Tests

6d19ac3

rustbot assigned fee1-dead Jun 6, 2024

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 6, 2024

veera-sivarajan commented Jun 6, 2024

View reviewed changes

fmease reviewed Jun 6, 2024

View reviewed changes

tests/ui/associated-type-bounds/no-gat-position.stderr Outdated Show resolved Hide resolved

veera-sivarajan force-pushed the bugfix-113073-bound-on-generics-2 branch from acf90a8 to 047afa3 Compare June 6, 2024 16:23

fee1-dead requested changes Jun 12, 2024

View reviewed changes

compiler/rustc_hir_analysis/src/hir_ty_lowering/errors.rs Outdated Show resolved Hide resolved

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 12, 2024

E0229: Suggest Moving Type Constraints to Type Parameter Declaration

5da1b41

veera-sivarajan force-pushed the bugfix-113073-bound-on-generics-2 branch from 047afa3 to 5da1b41 Compare June 12, 2024 23:33

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 13, 2024

fee1-dead approved these changes Jun 14, 2024

View reviewed changes

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 14, 2024

matthiaskrgr mentioned this pull request Jun 14, 2024

Rollup of 7 pull requests #126473

Merged

bors merged commit bfe0323 into rust-lang:master Jun 14, 2024
6 checks passed

rustbot added this to the 1.81.0 milestone Jun 14, 2024

matthiaskrgr mentioned this pull request Jul 26, 2024

ICE: trait takes 1 generic argument but 0 generic arguments were supplied #128249

Closed

veera-sivarajan mentioned this pull request Jul 30, 2024

Fix ICE Caused by Incorrectly Delaying E0107 #128377

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`E0229`: Suggest Moving Type Constraints to Type Parameter Declaration #126054

`E0229`: Suggest Moving Type Constraints to Type Parameter Declaration #126054

veera-sivarajan commented Jun 6, 2024

rustbot commented Jun 6, 2024

rustbot commented Jun 6, 2024

veera-sivarajan Jun 6, 2024

fee1-dead left a comment

veera-sivarajan commented Jun 13, 2024

fee1-dead left a comment

fee1-dead Jun 14, 2024

fee1-dead commented Jun 14, 2024

bors commented Jun 14, 2024

E0229: Suggest Moving Type Constraints to Type Parameter Declaration #126054

E0229: Suggest Moving Type Constraints to Type Parameter Declaration #126054

Conversation

veera-sivarajan commented Jun 6, 2024

rustbot commented Jun 6, 2024

rustbot commented Jun 6, 2024

veera-sivarajan Jun 6, 2024

Choose a reason for hiding this comment

fee1-dead left a comment

Choose a reason for hiding this comment

veera-sivarajan commented Jun 13, 2024

fee1-dead left a comment

Choose a reason for hiding this comment

fee1-dead Jun 14, 2024

Choose a reason for hiding this comment

fee1-dead commented Jun 14, 2024

bors commented Jun 14, 2024

`E0229`: Suggest Moving Type Constraints to Type Parameter Declaration #126054

`E0229`: Suggest Moving Type Constraints to Type Parameter Declaration #126054