Fix ICE: Restrict param constraint suggestion by estebank · Pull Request #117246 · rust-lang/rust

estebank · 2023-10-26T18:33:21Z

When encountering an associated item with a type param that could be constrained, do not look at the parent item if the type param comes from the associated item.

Fix#117209, fix#89868.

When encountering an associated item with a type param that could be constrained, do not look at the parent item if the type param comes from the associated item. Fixrust-lang#117209.

rustbot · 2023-10-26T18:33:29Z

r? @petrochenkov

(rustbot has picked a reviewer for you, use r? to override)

matthiaskrgr · 2023-10-26T18:39:43Z

Does that also fix #89868 ?

lqd · 2023-10-26T18:43:23Z

It seems unlikely that it would 😅

matthiaskrgr · 2023-10-26T19:02:56Z

my disappointment is immeasurable and my day is ruined

estebank · 2023-10-26T19:57:12Z

@matthiaskrgr it does now.

lqd · 2023-10-26T19:59:15Z

A masterful move, unruining Matthias' day :)

compiler-errors · 2023-10-26T21:06:32Z

@@ -0,0 +1,7 @@
+struct A<B>(B);
+impl<B>A<B>{fn d(){fn d(){Self(1)}}}


I feel like this is not the right way of fixing this bug?
Why do we allow Self constructors from outer items that reference parameters that aren't in scope anyways? Like, we're referencing a type parameter that doesn't exist here, right?
I feel like this also may cause strange behavior (or unsoundness? not exactly sure, depends on when we compare params by name or by index) when we actually do have a substitutable param here:
use std::marker::PhantomData; struct S0<T>(PhantomData<T>); impl<T> S0<T> { fn foo() { fn bar<T2>() -> S0<T2> { Self(PhantomData) // <~ this constructs S0<T> not S0<T2> :( } } }
cc @cjgillot who tried this and had to unland it in #111557

#111020 was reverted due to a regression, but it was probably a right thing to do in general.
Maybe we should do the usual bugfix process with a crater run, deprecation lint, and a tracking issue here.
In the meantime this PR could be merged to avoid the ICE.

petrochenkov · 2023-10-27T14:16:35Z

@@ -0,0 +1,7 @@
+struct A<B>(B);
+impl<B>A<B>{fn d(){fn d(){Self(1)}}}


#111020 was reverted due to a regression, but it was probably a right thing to do in general.
Maybe we should do the usual bugfix process with a crater run, deprecation lint, and a tracking issue here.
In the meantime this PR could be merged to avoid the ICE.

Fixrust-lang#89868.

petrochenkov · 2023-10-27T17:13:25Z

@bors r+

bors · 2023-10-27T17:13:27Z

📌 Commit 7449478 has been approved by petrochenkov

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#116834 (Remove `rustc_symbol_mangling/messages.ftl`.) - rust-lang#117212 (Properly restore snapshot when failing to recover parsing ternary) - rust-lang#117246 (Fix ICE: Restrict param constraint suggestion) - rust-lang#117247 (NVPTX: Allow PassMode::Direct for ptx kernels for now) - rust-lang#117270 (Hide internal methods from documentation) - rust-lang#117281 (std::thread : add SAFETY comment) - rust-lang#117287 (fix miri target information for Test step) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#117246 - estebank:issue-117209, r=petrochenkov Fix ICE: Restrict param constraint suggestion When encountering an associated item with a type param that could be constrained, do not look at the parent item if the type param comes from the associated item. Fixrust-lang#117209, fixrust-lang#89868.

Warn (or error) when `Self` ctor from outer item is referenced in inner nested item This implements a warning `SELF_CONSTRUCTOR_FROM_OUTER_ITEM` when a self constructor from an outer impl is referenced in an inner nested item. This is a proper fix mentioned rust-lang#117246 (comment). This warning is additionally bumped to a hard error when the self type references generic parameters, since it's almost always going to ICE, and is basically *never* correct to do. This also reverts part of rust-lang#117246, since I believe this is the proper fix and we shouldn't need the helper functions (`opt_param_at`/`opt_type_param`) any longer, since they shouldn't really ever be used in cases where we don't have this problem.

…enkov Warn (or error) when `Self` ctor from outer item is referenced in inner nested item This implements a warning `SELF_CONSTRUCTOR_FROM_OUTER_ITEM` when a self constructor from an outer impl is referenced in an inner nested item. This is a proper fix mentioned rust-lang#117246 (comment). This warning is additionally bumped to a hard error when the self type references generic parameters, since it's almost always going to ICE, and is basically *never* correct to do. This also reverts part of rust-lang#117246, since I believe this is the proper fix and we shouldn't need the helper functions (`opt_param_at`/`opt_type_param`) any longer, since they shouldn't really ever be used in cases where we don't have this problem.

Warn (or error) when `Self` ctor from outer item is referenced in inner nested item This implements a warning `SELF_CONSTRUCTOR_FROM_OUTER_ITEM` when a self constructor from an outer impl is referenced in an inner nested item. This is a proper fix mentioned rust-lang/rust#117246 (comment). This warning is additionally bumped to a hard error when the self type references generic parameters, since it's almost always going to ICE, and is basically *never* correct to do. This also reverts part of rust-lang/rust#117246, since I believe this is the proper fix and we shouldn't need the helper functions (`opt_param_at`/`opt_type_param`) any longer, since they shouldn't really ever be used in cases where we don't have this problem.

Restrict param constraint suggestion
3bbc70a
When encountering an associated item with a type param that could be constrained, do not look at the parent item if the type param comes from the associated item. Fixrust-lang#117209.

rustbot assigned petrochenkov Oct 26, 2023

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 Oct 26, 2023

estebank changed the title ~~Restrict param constraint suggestion~~Fix ICE: Restrict param constraint suggestionOct 26, 2023

This comment was marked as outdated.
Sign in to view

lqd reviewed Oct 26, 2023
View reviewed changes

Comment threadtests/ui/malformed/do-not-ice-on-note_and_explain.rs Outdated

estebank force-pushed the issue-117209 branch from 6681abb to 3ac5511Compare October 26, 2023 20:14

compiler-errors reviewed Oct 26, 2023
View reviewed changes

petrochenkov reviewed Oct 27, 2023
View reviewed changes

petrochenkov 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 Oct 27, 2023

Account for type param from other item in note_and_explain
7449478
Fixrust-lang#89868.

estebank force-pushed the issue-117209 branch from 3ac5511 to 7449478Compare October 27, 2023 16:24

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 27, 2023

matthiaskrgr mentioned this pull request Oct 27, 2023
Rollup of 7 pull requests #117294
Merged

bors merged commit a77f743 into rust-lang:masterOct 27, 2023

rustbot added this to the 1.75.0 milestone Oct 27, 2023

github-actionsBot mentioned this pull request Oct 31, 2023
Automated pull from upstream masterferrocene/ferrocene#74
Merged

lqd mentioned this pull request Nov 6, 2023
-Zthreads changes compiled binary #117635
Closed

estebank deleted the issue-117209 branch November 9, 2023 05:12

matthiaskrgr mentioned this pull request Mar 4, 2024
ICE: param types with same index differ in name#121983
Closed

compiler-errors mentioned this pull request Apr 20, 2024
Warn (or error) when Self ctor from outer item is referenced in inner nested item #124187
Merged

Navigation Menu

Uh oh!

Uh oh!

Fix ICE: Restrict param constraint suggestion - #117246

Fix ICE: Restrict param constraint suggestion#117246
bors merged 2 commits into
rust-lang:masterfrom
estebank:issue-117209

estebank commented Oct 26, 2023•
edited
Loading

Uh oh!

rustbot commented Oct 26, 2023

Uh oh!

matthiaskrgr commented Oct 26, 2023

Uh oh!

lqd commented Oct 26, 2023

Uh oh!

matthiaskrgr commented Oct 26, 2023

Uh oh!

estebank commented Oct 26, 2023

Uh oh!

lqd commented Oct 26, 2023

Uh oh!

This comment was marked as outdated.

Uh oh!

compiler-errorsOct 26, 2023•
edited
Loading

Uh oh!

petrochenkovOct 27, 2023

Uh oh!

Uh oh!

petrochenkovOct 27, 2023

Uh oh!

petrochenkov commented Oct 27, 2023

Uh oh!

bors commented Oct 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

		@@ -0,0 +1,7 @@
		struct A<B>(B);
		impl<B>A<B>{fn d(){fn d(){Self(1)}}}

Uh oh!

Uh oh!

Conversation

estebank commented Oct 26, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Oct 26, 2023

Uh oh!

matthiaskrgr commented Oct 26, 2023

Uh oh!

lqd commented Oct 26, 2023

Uh oh!

matthiaskrgr commented Oct 26, 2023

Uh oh!

estebank commented Oct 26, 2023

Uh oh!

lqd commented Oct 26, 2023

Uh oh!

This comment was marked as outdated.

Uh oh!

compiler-errorsOct 26, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petrochenkovOct 27, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

petrochenkovOct 27, 2023

Choose a reason for hiding this comment

Uh oh!

petrochenkov commented Oct 27, 2023

Uh oh!

bors commented Oct 27, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

estebank commented Oct 26, 2023•
edited
Loading

compiler-errorsOct 26, 2023•
edited
Loading