Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix 10021 #10027

Merged
merged 2 commits into from
Dec 4, 2022
Merged

Fix 10021 #10027

merged 2 commits into from
Dec 4, 2022

Conversation

smoelius
Copy link
Contributor

@smoelius smoelius commented Dec 3, 2022

This PR proposes a fix for #10021.

The problem is similar to the one that @mikerite described in #9505. The compiler is generating an empty substitution for a call, even though the type of Self seems to be needed for a predicate. In @mikerite's case, the call was to IntoFuture::into_future. In this case, the call is to Try::branch.

The proposed fix is to verify that the parameter whose type is changing has an index within the substitution. The strikes me as a reasonable approach, since if the check were to fail, the following code would be a no-op:

let new_subst = cx.tcx.mk_substs(
call_substs.iter()
.enumerate()
.map(|(i, t)|
if i == (*param_index as usize) {
GenericArg::from(ty)
} else {
t
}));

Like @mikerite's original solution, this solution turns ICEs into false negatives.

changelog: fix unnecessary_to_owned false positive involving Try::branch

@rustbot
Copy link
Collaborator

rustbot commented Dec 3, 2022

r? @dswij

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

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Dec 3, 2022
Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!

@dswij
Copy link
Member

dswij commented Dec 4, 2022

@bors r+

@bors
Copy link
Collaborator

bors commented Dec 4, 2022

📌 Commit 2701a40 has been approved by dswij

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Dec 4, 2022

⌛ Testing commit 2701a40 with merge cb8df45...

@bors
Copy link
Collaborator

bors commented Dec 4, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing cb8df45 to master...

@bors bors merged commit cb8df45 into rust-lang:master Dec 4, 2022
@smoelius smoelius deleted the fix-10021 branch December 4, 2022 10:28
@ghost
Copy link

ghost commented Dec 6, 2022

@rustbot label +beta-nominated

@rustbot
Copy link
Collaborator

rustbot commented Dec 6, 2022

Error: Label beta-nominated can only be set by Rust team members

Please file an issue on GitHub at triagebot if there's a problem with this bot, or reach out on #t-infra on Zulip.

@flip1995 flip1995 added the beta-nominated Nominated for backporting to the compiler in the beta channel. label Dec 6, 2022
@flip1995 flip1995 added beta-accepted Accepted for backporting to the compiler in the beta channel. and removed beta-nominated Nominated for backporting to the compiler in the beta channel. labels Dec 8, 2022
@flip1995
Copy link
Member

flip1995 commented Dec 8, 2022

rust-lang/rust#105470

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Dec 9, 2022
…earth

Clippy: backport ICE fix before beta branch

r? `@Manishearth`

Before beta is branched tomorrow we should backport the fix from rust-lang/rust-clippy#10027 for an ICE. That way we'll get this into stable one release sooner.

This only cherry-picks the fix, not the tests for it. The proper sync of this will be done next week Thursday.
RalfJung pushed a commit to RalfJung/miri that referenced this pull request Dec 11, 2022
Clippy: backport ICE fix before beta branch

r? `@Manishearth`

Before beta is branched tomorrow we should backport the fix from rust-lang/rust-clippy#10027 for an ICE. That way we'll get this into stable one release sooner.

This only cherry-picks the fix, not the tests for it. The proper sync of this will be done next week Thursday.
@xFrednet xFrednet removed the beta-accepted Accepted for backporting to the compiler in the beta channel. label Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants