Skip to content

Commit

Permalink
Auto merge of #13367 - y21:issue13364, r=Manishearth
Browse files Browse the repository at this point in the history
Visit struct fields recursively in uninit fallback check

This makes the fallback a bit more consistent with the other checks and rustc.

Fixes #13364. When using a generic type as the `Vec` element type like the issue title says, rustc's uninit check fails and our fallback is used, which didn't look at struct fields when it could.

changelog: none
  • Loading branch information
bors committed Sep 9, 2024
2 parents 1e797e9 + ae5326b commit 938f8ba
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 5 deletions.
11 changes: 7 additions & 4 deletions clippy_utils/src/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -606,10 +606,13 @@ fn is_uninit_value_valid_for_ty_fallback<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'t
ty::Tuple(types) => types.iter().all(|ty| is_uninit_value_valid_for_ty(cx, ty)),
// Unions are always fine right now.
// This includes MaybeUninit, the main way people use uninitialized memory.
// For ADTs, we could look at all fields just like for tuples, but that's potentially
// exponential, so let's avoid doing that for now. Code doing that is sketchy enough to
// just use an `#[allow()]`.
ty::Adt(adt, _) => adt.is_union(),
ty::Adt(adt, _) if adt.is_union() => true,
// Types (e.g. `UnsafeCell<MaybeUninit<T>>`) that recursively contain only types that can be uninit
// can themselves be uninit too.
// This purposefully ignores enums as they may have a discriminant that can't be uninit.
ty::Adt(adt, args) if adt.is_struct() => adt
.all_fields()
.all(|field| is_uninit_value_valid_for_ty(cx, field.ty(cx.tcx, args))),
// For the rest, conservatively assume that they cannot be uninit.
_ => false,
}
Expand Down
32 changes: 32 additions & 0 deletions tests/ui/uninit_vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,4 +150,36 @@ fn main() {
vec.set_len(10);
}
}

fn nested_union<T>() {
let mut vec: Vec<UnsafeCell<MaybeUninit<T>>> = Vec::with_capacity(1);
unsafe {
vec.set_len(1);
}
}

struct Recursive<T>(*const Recursive<T>, MaybeUninit<T>);
fn recursive_union<T>() {
// Make sure we don't stack overflow on recursive types.
// The pointer acts as the base case because it can't be uninit regardless of its pointee.

let mut vec: Vec<Recursive<T>> = Vec::with_capacity(1);
//~^ uninit_vec
unsafe {
vec.set_len(1);
}
}

#[repr(u8)]
enum Enum<T> {
Variant(T),
}
fn union_in_enum<T>() {
// Enums can have a discriminant that can't be uninit, so this should still warn
let mut vec: Vec<Enum<T>> = Vec::with_capacity(1);
//~^ uninit_vec
unsafe {
vec.set_len(1);
}
}
}
24 changes: 23 additions & 1 deletion tests/ui/uninit_vec.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -125,5 +125,27 @@ LL | vec.set_len(10);
|
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: aborting due to 12 previous errors
error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> tests/ui/uninit_vec.rs:166:9
|
LL | let mut vec: Vec<Recursive<T>> = Vec::with_capacity(1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | vec.set_len(1);
| ^^^^^^^^^^^^^^
|
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
--> tests/ui/uninit_vec.rs:179:9
|
LL | let mut vec: Vec<Enum<T>> = Vec::with_capacity(1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
LL | vec.set_len(1);
| ^^^^^^^^^^^^^^
|
= help: initialize the buffer or wrap the content in `MaybeUninit`

error: aborting due to 14 previous errors

0 comments on commit 938f8ba

Please sign in to comment.