-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Clippy ICE due to async gen
block support
#11965
Comments
What does this have to do with |
That PR changed either how |
@Jarcho: That PR should not have affected how @P1n3appl3: Are you certain that this is failing after rust-lang/rust@f967532, and not some other merge commit on that same day? If you are certain, could you please instruct me how to reproduce the ICE? I have no idea how to build fuchsia. |
Some PR around that time made a change, but I never verified which commit did. #11952 has a repro using async fn. The clippy bug takes quite a bit to trigger and small changes can avoid it. Minimal repro without async would be this: fn f<'a, T: AsRef<[u8]>>(x: T, y: &'a ()) -> impl 'a + FnOnce() {
move || {
let _y = y;
}
}
fn g(_: impl FnOnce()) {}
fn main() {
g(f([].to_vec(), &()));
} Cause was clippy taking the return type of let x = f([].to_vec(), &());
g(x); |
This is the change from rust-lang/rust#118420 that causes clippy to begin ICEing: https://github.com/rust-lang/rust/pull/118420/files#r1419874371 That line fixes a bug where we were not recording the node_args properly during writeback in HIR typeck, and that bug was silently causing some code to not be executed in clippy due to a defensively placed if-condition: rust-clippy/clippy_lints/src/methods/unnecessary_to_owned.rs Lines 451 to 452 in ac4c209
But now that the args were recorded correctly, that if-condition no longer evaluated to I want to make it very clear that that change I made to HIR typeck is not wrong, and that it only causes clippy to start exercising code that it was previously silently skipping. |
I had assumed it was lowering changes that caused it since Clippy can be pretty sensitive to the exact HIR generated (I only took a quick look at that PR since I had already found the bug by that point). Either way it definitely exposed a Clippy bug in more code than it originally showed up in. |
I'm mostly just confused why even though the root cause for #9504 was correctly identified in #9505 ("Compiler generated call into_future nodes return empty substs"), it wasn't fixed in rust-lang/rust and it was instead just silently mitigated by an Had it been fixed correctly (i.e. by adding this Anyways, I don't think anything here needs to be done -- everything seems to be resolved. Thanks @Jarcho for fixing the binder ICE, and I'm glad to see that rust-lang/rust#118420 didn't introduce a real bug. |
Remove mitigations for incorrect node args This change https://github.com/rust-lang/rust/pull/118420/files#r1419874371 adds a missing `write_args` to properly record node args for lang-item calls. Thus, in the `unnecessary_to_owned` lint, this ensures that the `call_generic_args` extracted by `get_callee_generic_args_and_args` are always correct, and we can remove the mitigation for #9504 and #10021 since the root cause has been fixed. I'm not sure if there is other now-unnecessary code that can be removed, but this is the one I found when investigating #11965 (comment). changelog: none
Fuchsia's CI has picked up an ICE in clippy (that doesn't happen in rustc when run on the same code). It appeared when rust-lang/rust#118420 landed and I've confirmed that it consistently reproduces after that commit and doesn't happen before it.
We're still working on pulling out a minimal reproducer (here's the code it fails on) and either I or @tmandry will add that as a followup once we've got it.
Meta
rustc --version --verbose
:clippy-driver --version
:Error output
Backtrace
The text was updated successfully, but these errors were encountered: