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

Add check if ty has_escaping_bound_vars in zero_sized_map_values lint #7470

Merged
merged 1 commit into from Jul 19, 2021

Conversation

DevinR528
Copy link
Contributor

Fixes: #7447

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 Jul 15, 2021
@DevinR528
Copy link
Contributor Author

DevinR528 commented Jul 15, 2021

Not exactly sure if this is the best way, it seems that checking for Cow here would also work but the way it is now, seems more robust since that is the method call causing the ICE?

This also fails so Cow wouldn't work struct Foo<'a>(Cow<'a, whatever>)

@flip1995
Copy link
Member

I think this is a more general problem in Clippy. The same error with layout_of happened to me yesterday, when I did the rustup. The fix was the same. Clippy has about 20 calls to layout_of. So maybe we should implement a utils function for this, which calls has_escaping_bound_vars before calling layout_of?

@flip1995
Copy link
Member

Let's ask about it: rust-lang/rust#86867 (comment)

@DevinR528
Copy link
Contributor Author

Cool in a day or so I can just preemptively add this to all the layout_of calls, but I will follow the conversation in the rust repo.

@flip1995
Copy link
Member

can just preemptively add this to all the layout_of calls,

Yeah. Or better: implement a clippy_utils::layout_of, that looks like this:

fn layout_of(cx: LateContext, ty: Ty) -> Option<_> {
    if ty.has_escaping_bound_vars() {
        return None;
    }
    cx.tcx.layout_of(ty)
}

If you don't have time for it, let me know and I'll do this tomorrow. We have to sync back those fixes before the release in 1 and a half weeks.

@flip1995
Copy link
Member

Since we didn't get any more reports on this, I'm holding off from replacing all of the layout_of calls for now. I have a patch ready here: https://github.com/flip1995/rust-clippy/tree/layout_of-util

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 19, 2021

📌 Commit 7312611 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Jul 19, 2021

⌛ Testing commit 7312611 with merge f467750...

@bors
Copy link
Collaborator

bors commented Jul 19, 2021

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

@bors bors merged commit f467750 into rust-lang:master Jul 19, 2021
@flip1995 flip1995 mentioned this pull request Jul 19, 2021
bors added a commit that referenced this pull request Jul 19, 2021
Rustup

r? `@ghost`

Out of cycle sync for 2 ICE fixes #7470 #7471 #7473

changelog: none
@DevinR528
Copy link
Contributor Author

Cool thanks! Sorry I didn't get around to it I was doing a bit more traveling than I thought I'll keep an eye on any ICE that could be related.

bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 19, 2021
Update Clippy

This is an out-of-cycle Clippy update, to fix 3 ICEs before the release (This should be merged before beta is branched):

rust-lang/rust-clippy#7470
rust-lang/rust-clippy#7471
rust-lang/rust-clippy#7473

cc `@jackh726` `@JohnTitor` rust-lang/rust-clippy#7470 was caused by rust-lang#86867. I saw the same ICE in the last rustup for Clippy though, so this might be a more general problem. Is there something we should check before calling `layout_of`? Should we always check for `ty.has_escaping_bound_vars()` before calling `layout_of`? Or is this overkill?

r? `@Manishearth`
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 checking symbolic-minidump with 2021-07-07 clippy
4 participants