Fix dyn -> param suggestion in struct ICEs#138238

compiler-errors · 2025-03-08T20:47:21Z

Makes the logic from #138042 a bit less ICEy and more clean. Also fixes an incorrect suggestion when the struct already has generics. I'll point out the major changes and observations in the code.

Fixes #138229
Fixes #138211

r? nnethercote since you reviewed the original pr, or re-roll if you don't want to review this

rustbot · 2025-03-08T20:47:27Z

HIR ty lowering was modified

cc @fmease

compiler-errors · 2025-03-08T20:48:11Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

-        let mut sugg = vec![];

-        let parent_id = tcx.hir_get_parent_item(self_ty.hir_id).def_id;
-        let parent_item = tcx.hir_node_by_def_id(parent_id).expect_item();


expect_item here is not valid, since there are other kinds of HIR parents that can own this dyn Trait, like extern items (or trait/impl items, closures, etc).

compiler-errors · 2025-03-08T20:48:59Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

+        let parent_hir_id = tcx.parent_hir_id(self_ty.hir_id);
+        let parent_item = tcx.hir_get_parent_item(self_ty.hir_id).def_id;
+
+        let generics = match tcx.hir_node_by_def_id(parent_item) {


I added a check that we're suggesting this on a field directly. dyn may show up in other positions like in a where clause or in an expr on a default field value.

compiler-errors · 2025-03-08T20:51:03Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

-        if let hir::ItemKind::Enum(..) = parent_item.kind {
-            return false;
-        }
+        // Look at the direct HIR parent, since we care about the relationship between


This was also expect_iteming unnecessarily. I took the logic and made it focus on the relationship between the ty and its parent node, so we don't suggest turning:

struct Foo { f: Box<Any>, /* more fields */ }

Into this:

struct Foo<T: Any> { f: Box<T>, /* more fields */ }

My general logic here is that if the ty's hir node parent is another ty, then it's probably just a misspelled dyn Trait. There is no perfect heuristic here, but I think it's less invasive to suggest the most naive thing here, which is just to insert dyn, unless we're very confident it will be wrong.

compiler-errors · 2025-03-08T20:51:28Z

tests/ui/dyn-keyword/suggest-struct-or-union-add-generic-impl-trait.rs

@@ -13,7 +13,6 @@ struct Foo2 {
    //~^ ERROR expected a type, found a trait


I moved this test out of suggestions/ since that's a kitchen sink

compiler-errors · 2025-03-08T20:51:51Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs


-        // If the parent item is a struct, check if self_ty is the last field.
-        if let hir::ItemKind::Struct(variant_data, _) = parent_item.kind {
-            if variant_data.fields().last().unwrap().ty.span != self_ty.span {


We don't need to use span comparisons here, hir_id comparisons are more faithful.

compiler-errors · 2025-03-08T20:52:23Z

compiler/rustc_hir_analysis/src/hir_ty_lowering/lint.rs

+        let param = "TUV"
+            .chars()
+            .map(|c| c.to_string())
+            .chain((0..).map(|i| format!("P{i}")))
+            .find(|s| !generics.params.iter().any(|param| param.name.ident().as_str() == s))
+            .expect("we definitely can find at least one param name to generate");


This logic didn't account for preexisting generic args, so this tries its best to suggest T, U, V, P1, P2, ...

nnethercote · 2025-03-09T21:15:44Z

@bors r+

Thank for fixing the problems.

bors · 2025-03-09T21:15:46Z

📌 Commit ceb0401 has been approved by nnethercote

It is now in the queue for this repository.

nnethercote · 2025-03-09T21:16:27Z

cc @xizheyin

compiler-errors · 2025-03-09T21:16:56Z

error path only so

@bors rollup

…struct, r=nnethercote Fix dyn -> param suggestion in struct ICEs Makes the logic from rust-lang#138042 a bit less ICEy and more clean. Also fixes an incorrect suggestion when the struct already has generics. I'll point out the major changes and observations in the code. Fixes rust-lang#138229 Fixes rust-lang#138211 r? nnethercote since you reviewed the original pr, or re-roll if you don't want to review this

…iaskrgr Rollup of 6 pull requests Successful merges: - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable) - rust-lang#137793 (Stablize anonymous pipe) - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs) - rust-lang#138270 (chore: Fix some comments) - rust-lang#138280 (fix ICE in pretty-printing `global_asm!`) - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…) r? `@ghost` `@rustbot` modify labels: rollup

…struct, r=nnethercote Fix dyn -> param suggestion in struct ICEs Makes the logic from rust-lang#138042 a bit less ICEy and more clean. Also fixes an incorrect suggestion when the struct already has generics. I'll point out the major changes and observations in the code. Fixes rust-lang#138229 Fixes rust-lang#138211 r? nnethercote since you reviewed the original pr, or re-roll if you don't want to review this

…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#136395 (Update to rand 0.9.0) - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable) - rust-lang#137585 (Update documentation to consistently use 'm' in atomic synchronization example) - rust-lang#137926 (Add a test for `-znostart-stop-gc` usage with LLD) - rust-lang#138074 (Support `File::seek` for Hermit) - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs) - rust-lang#138270 (chore: Fix some comments) - rust-lang#138280 (fix ICE in pretty-printing `global_asm!`) - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…) r? `@ghost` `@rustbot` modify labels: rollup

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#136395 (Update to rand 0.9.0) - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable) - rust-lang#137585 (Update documentation to consistently use 'm' in atomic synchronization example) - rust-lang#137926 (Add a test for `-znostart-stop-gc` usage with LLD) - rust-lang#138074 (Support `File::seek` for Hermit) - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs) - rust-lang#138270 (chore: Fix some comments) - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…) r? `@ghost` `@rustbot` modify labels: rollup

…struct, r=nnethercote Fix dyn -> param suggestion in struct ICEs Makes the logic from rust-lang#138042 a bit less ICEy and more clean. Also fixes an incorrect suggestion when the struct already has generics. I'll point out the major changes and observations in the code. Fixes rust-lang#138229 Fixes rust-lang#138211 r? nnethercote since you reviewed the original pr, or re-roll if you don't want to review this

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#136395 (Update to rand 0.9.0) - rust-lang#137279 (Make some invalid codegen attr errors structured/translatable) - rust-lang#137585 (Update documentation to consistently use 'm' in atomic synchronization example) - rust-lang#137926 (Add a test for `-znostart-stop-gc` usage with LLD) - rust-lang#138074 (Support `File::seek` for Hermit) - rust-lang#138238 (Fix dyn -> param suggestion in struct ICEs) - rust-lang#138270 (chore: Fix some comments) - rust-lang#138286 (triagebot.toml: Don't label `test/rustdoc-json` as A-rustdoc-search (…) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#138238 - compiler-errors:dyn-suggestion-in-struct, r=nnethercote Fix dyn -> param suggestion in struct ICEs Makes the logic from rust-lang#138042 a bit less ICEy and more clean. Also fixes an incorrect suggestion when the struct already has generics. I'll point out the major changes and observations in the code. Fixes rust-lang#138229 Fixes rust-lang#138211 r? nnethercote since you reviewed the original pr, or re-roll if you don't want to review this

compiler-errors added 2 commits March 8, 2025 20:40

Rework maybe_suggest_add_generic_impl_trait

bca0ab8

Fix suggestion when there are generics, inline some things

ceb0401

rustbot assigned nnethercote Mar 8, 2025

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 Mar 8, 2025

compiler-errors commented Mar 8, 2025

View reviewed changes

compiler-errors mentioned this pull request Mar 9, 2025

Bare Trait Object Return in Trait Method Causes Panic #138277

Closed

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 Mar 9, 2025

matthiaskrgr mentioned this pull request Mar 9, 2025

Rollup of 6 pull requests #138289

Closed

matthiaskrgr mentioned this pull request Mar 10, 2025

Rollup of 9 pull requests #138298

Closed

matthiaskrgr mentioned this pull request Mar 10, 2025

Rollup of 8 pull requests #138302

Merged

jieyouxu mentioned this pull request Mar 10, 2025

Rollup of 25 pull requests #138324

Closed

bors merged commit 1ae083d into rust-lang:master Mar 11, 2025
6 checks passed

rustbot added this to the 1.87.0 milestone Mar 11, 2025

compiler-errors mentioned this pull request Mar 11, 2025

nightly rustc panicked when no dyn before trait in type T = #138369

Closed

jieyouxu mentioned this pull request Mar 16, 2025

ICE in type-checking ... assert_receiver_is_total_eq #138543

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix dyn -> param suggestion in struct ICEs#138238