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 ICE 6840 - make is_normalizable more strict #6866

Merged
merged 4 commits into from
Mar 9, 2021
Merged

Conversation

anall
Copy link
Contributor

@anall anall commented Mar 8, 2021

fixes #6840

make is_normalizable more strict, which should catch this ICE and related cases

changelog: Fix ICE in [zero_sized_map_values]

@rust-highfive
Copy link

r? @flip1995

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 8, 2021
clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
clippy_utils/src/lib.rs Outdated Show resolved Hide resolved
is_normalizable_helper(cx, param_env, ty, &mut HashMap::new())
}

fn is_normalizable_helper<'tcx>(
Copy link
Member

Choose a reason for hiding this comment

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

One last thing: This function should not have to exist, if the normalize function in rustc would work as intended. Can you add a FIXME linking to the normalize documentation, explaining that this helper can be removed in the future, once normalize doesn't panic anymore?

Copy link
Member

@flip1995 flip1995 left a comment

Choose a reason for hiding this comment

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

LGTM. Just the fixme comment would be great.

@flip1995
Copy link
Member

flip1995 commented Mar 9, 2021

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Mar 9, 2021

📌 Commit 9707599 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Mar 9, 2021

⌛ Testing commit 9707599 with merge ba624ee...

bors added a commit that referenced this pull request Mar 9, 2021
Fix ICE 6840 - make is_normalizable more strict

fixes #6840

make `is_normalizable` more strict, which should catch this ICE and related cases

changelog:
@bors
Copy link
Collaborator

bors commented Mar 9, 2021

💔 Test failed - checks-action_test

@flip1995
Copy link
Member

flip1995 commented Mar 9, 2021

@bors retry

@bors
Copy link
Collaborator

bors commented Mar 9, 2021

⌛ Testing commit 9707599 with merge 8a5f98a...

@bors
Copy link
Collaborator

bors commented Mar 9, 2021

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing 8a5f98a to master...

@bors bors merged commit 8a5f98a into rust-lang:master Mar 9, 2021
@stuhood
Copy link

stuhood commented Mar 25, 2021

Hey folks! Thanks for the fix. Did this miss the boat for clippy 0.1.51 (2fd73fa 2021-03-23) ? We're still seeing it there.

@anall
Copy link
Contributor Author

anall commented Mar 25, 2021

This unfortunately did miss 1.51. Looks like this fix is available in the current Rust beta release (1.52)

@anall anall deleted the ice6840 branch March 25, 2021 17:46
@flip1995
Copy link
Member

Yeah, it missed it. To catch such things earlier and backport them, we're heavily relying on Clippy nightly/beta users, reporting that they want to have those fixes backported, before the new beta is branched (~1 week before release).

@stuhood
Copy link

stuhood commented Mar 25, 2021

Ok, thanks: we'll try to consume beta to get this.

This unfortunately did miss 1.51. Looks like this fix is available in the current Rust beta release (1.52)

It doesn't look like it's in the latest beta toolchain either, afaict:

$ rustup toolchain install beta
info: syncing channel updates for 'beta-x86_64-apple-darwin'
info: latest update on 2021-03-19, rust version 1.51.0-beta.8 (73f48e5f6 2021-03-18)
...

@flip1995
Copy link
Member

The new beta wasn't released yet. It should be in beta tomorrow. (see the version number of beta, it still says 1.51)

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.

ICE in v1.50.0 that does not occur in v1.49.0
5 participants