Skip to content

Conversation

lukas-code
Copy link
Member

fixes #146174

This code suggests to write Foo::new(...) when the user writes Foo(...) or Foo { ... } and the constructor is private, where new is some associated function that returns Self.

When checking that the return type of new is Self, we need to instantiate the parameters of new with infer vars, so we don't end up with a type like Box<$param(0)> in a context that doesn't have any parameters. But then we can't use normalize_erasing_late_bound_regions anymore because that goes though a query that can't deal with infer vars.

Since this is diagnostic-only code that is supposed to check for exactly -> Self, I think it's fine to just skip normalizing here, especially since The Correct WayTM would involve a probe and make this code even more complicated.

Also, the code here does almost the same thing, and these suggestions can probably be unified in the future:

// Only assoc fns that return `Self`, `Option<Self>` or `Result<Self, _>`.
let ret_ty = self
.tcx
.fn_sig(item.def_id)
.instantiate(self.tcx, self.fresh_args_for_item(DUMMY_SP, item.def_id))
.output();
let ret_ty = self.tcx.instantiate_bound_regions_with_erased(ret_ty);

r? @compiler-errors
cc @Qelxiros -- this should unblock #144420

@rustbot
Copy link
Collaborator

rustbot commented Sep 4, 2025

compiler-errors is not on the review rotation at the moment.
They may take a while to respond.

@rustbot 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 Sep 4, 2025
@compiler-errors
Copy link
Member

Thanks

r=me when green

@bors delegate+

@bors

This comment was marked as off-topic.

@compiler-errors

This comment was marked as off-topic.

@lukas-code
Copy link
Member Author

@bors r=compiler-errors rollup

@bors
Copy link
Collaborator

bors commented Sep 5, 2025

📌 Commit 4ca8078 has been approved by compiler-errors

It is now in the queue for this repository.

@bors 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 Sep 5, 2025
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 5, 2025
…iler-errors

fix ICE when suggesting `::new`

fixes rust-lang#146174

This code suggests to write `Foo::new(...)` when the user writes `Foo(...)` or `Foo { ... }` and the constructor is private, where `new` is some associated function that returns `Self`.

When checking that the return type of `new` is `Self`, we need to instantiate the parameters of `new` with infer vars, so we don't end up with a type like `Box<$param(0)>` in a context that doesn't have any parameters. But then we can't use `normalize_erasing_late_bound_regions` anymore because that goes though a query that can't deal with infer vars.

Since this is diagnostic-only code that is supposed to check for exactly `-> Self`, I think it's fine to just skip normalizing here, especially since The Correct Way<sup>TM</sup> would involve a probe and make this code even more complicated.

Also, the code here does almost the same thing, and these suggestions can probably be unified in the future: https://github.com/rust-lang/rust/blob/4ca8078d37c53ee4ff8fb32b4453b915116f25b8/compiler/rustc_hir_typeck/src/method/suggest.rs#L2123-L2129

r? `@compiler-errors`
cc `@Qelxiros` -- this should unblock rust-lang#144420
tgross35 added a commit to tgross35/rust that referenced this pull request Sep 5, 2025
…iler-errors

fix ICE when suggesting `::new`

fixes rust-lang#146174

This code suggests to write `Foo::new(...)` when the user writes `Foo(...)` or `Foo { ... }` and the constructor is private, where `new` is some associated function that returns `Self`.

When checking that the return type of `new` is `Self`, we need to instantiate the parameters of `new` with infer vars, so we don't end up with a type like `Box<$param(0)>` in a context that doesn't have any parameters. But then we can't use `normalize_erasing_late_bound_regions` anymore because that goes though a query that can't deal with infer vars.

Since this is diagnostic-only code that is supposed to check for exactly `-> Self`, I think it's fine to just skip normalizing here, especially since The Correct Way<sup>TM</sup> would involve a probe and make this code even more complicated.

Also, the code here does almost the same thing, and these suggestions can probably be unified in the future: https://github.com/rust-lang/rust/blob/4ca8078d37c53ee4ff8fb32b4453b915116f25b8/compiler/rustc_hir_typeck/src/method/suggest.rs#L2123-L2129

r? ```@compiler-errors```
cc ```@Qelxiros``` -- this should unblock rust-lang#144420
bors added a commit that referenced this pull request Sep 5, 2025
Rollup of 6 pull requests

Successful merges:

 - #144342 (add exact bitshifts)
 - #145709 (Fix LoongArch C function ABI when passing/returning structs containing floats)
 - #146152 (Unify and deduplicate algebraic float tests)
 - #146186 (Update cc-rs to 1.2.33, and switch rustc_codegen_ssa to use find-msvc-tools)
 - #146207 (std: Implement WASIp2-specific stdio routines)
 - #146217 (fix ICE when suggesting `::new`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Sep 5, 2025
Rollup of 5 pull requests

Successful merges:

 - #144342 (add exact bitshifts)
 - #145709 (Fix LoongArch C function ABI when passing/returning structs containing floats)
 - #146152 (Unify and deduplicate algebraic float tests)
 - #146207 (std: Implement WASIp2-specific stdio routines)
 - #146217 (fix ICE when suggesting `::new`)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 8ff60a6 into rust-lang:master Sep 5, 2025
14 of 16 checks passed
rust-timer added a commit that referenced this pull request Sep 5, 2025
Rollup merge of #146217 - lukas-code:suggest-new-ice, r=compiler-errors

fix ICE when suggesting `::new`

fixes #146174

This code suggests to write `Foo::new(...)` when the user writes `Foo(...)` or `Foo { ... }` and the constructor is private, where `new` is some associated function that returns `Self`.

When checking that the return type of `new` is `Self`, we need to instantiate the parameters of `new` with infer vars, so we don't end up with a type like `Box<$param(0)>` in a context that doesn't have any parameters. But then we can't use `normalize_erasing_late_bound_regions` anymore because that goes though a query that can't deal with infer vars.

Since this is diagnostic-only code that is supposed to check for exactly `-> Self`, I think it's fine to just skip normalizing here, especially since The Correct Way<sup>TM</sup> would involve a probe and make this code even more complicated.

Also, the code here does almost the same thing, and these suggestions can probably be unified in the future: https://github.com/rust-lang/rust/blob/4ca8078d37c53ee4ff8fb32b4453b915116f25b8/compiler/rustc_hir_typeck/src/method/suggest.rs#L2123-L2129

r? ````@compiler-errors````
cc ````@Qelxiros```` -- this should unblock #144420
@rustbot rustbot added this to the 1.91.0 milestone Sep 5, 2025
@lukas-code lukas-code deleted the suggest-new-ice branch September 5, 2025 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Complex function signature in Box method causes ICE
4 participants