Skip to content

Commit

Permalink
Auto merge of #12591 - y21:issue12585, r=Jarcho
Browse files Browse the repository at this point in the history
type certainty: clear `DefId` when an expression's type changes to non-adt

Fixes #12585

The root cause of the ICE in the linked issue was in the expression `one.x`, in the array literal.

The type of `one` is the `One` struct: an adt with a DefId, so its certainty is `Certain(def_id_of_one)`. However, the field access `.x` can then change the type (to `i32` here) and that should update that `DefId` accordingly. It does do that correctly when `one.x` would be another adt with a DefId:

https://github.com/rust-lang/rust-clippy/blob/97ba291d5aa026353ad93e48cf00e06f08c73830/clippy_utils/src/ty/type_certainty/mod.rs#L90-L91

but when it *isn't* an adt and there is no def id (which is the case in the linked issue: `one.x` is an i32), it keeps the `DefId` of `One`, even though that's the wrong type (which would then lead to a contradiction later when joining `Certainty`s):
https://github.com/rust-lang/rust-clippy/blob/97ba291d5aa026353ad93e48cf00e06f08c73830/clippy_utils/src/ty/type_certainty/mod.rs#L92-L93

In particular, in the linked issue, `from_array([one.x, two.x])` would try to join the `Certainty` of the two array elements, which *should* have been `[Certain(None), Certain(None)]`, because `i32`s have no `DefId`, but instead it was `[Certain(One), Certain(Two)]`, because the DefId wasn't cleared from when it was visiting `one` and `two`. This is the "contradiction" that could be seen in the ICE message

... so this changes it to clear the `DefId` when it isn't an adt.

cc `@smoelius` you implemented this initially in #11135, does this change make sense to you?

changelog: none
  • Loading branch information
bors committed Apr 4, 2024
2 parents a73e751 + 9f5d31e commit 8253040
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
2 changes: 1 addition & 1 deletion clippy_utils/src/ty/type_certainty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ fn expr_type_certainty(cx: &LateContext<'_>, expr: &Expr<'_>) -> Certainty {
if let Some(def_id) = adt_def_id(expr_ty) {
certainty.with_def_id(def_id)
} else {
certainty
certainty.clear_def_id()
}
}

Expand Down
26 changes: 26 additions & 0 deletions tests/ui/crashes/ice-12585.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
#![allow(clippy::unit_arg)]

struct One {
x: i32,
}
struct Two {
x: i32,
}

struct Product {}

impl Product {
pub fn a_method(self, _: ()) {}
}

fn from_array(_: [i32; 2]) -> Product {
todo!()
}

pub fn main() {
let one = One { x: 1 };
let two = Two { x: 2 };

let product = from_array([one.x, two.x]);
product.a_method(<()>::default());
}

0 comments on commit 8253040

Please sign in to comment.