diff options
| author | bors <bors@rust-lang.org> | 2024-09-09 23:25:46 +0000 |
|---|---|---|
| committer | bors <bors@rust-lang.org> | 2024-09-09 23:25:46 +0000 |
| commit | 938f8ba20aac34a2ef696627c8bf871844da67f9 (patch) | |
| tree | 0998f0b14a96006468573326426e45bcc84f34f0 | |
| parent | 1e797e9a381906c9b249209dae053bbbd13b50ca (diff) | |
| parent | ae5326b967d5a3d34d3c562882feeacd9839950c (diff) | |
| download | rust-938f8ba20aac34a2ef696627c8bf871844da67f9.tar.gz rust-938f8ba20aac34a2ef696627c8bf871844da67f9.zip | |
Auto merge of #13367 - y21:issue13364, r=Manishearth
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
| -rw-r--r-- | clippy_utils/src/ty.rs | 11 | ||||
| -rw-r--r-- | tests/ui/uninit_vec.rs | 32 | ||||
| -rw-r--r-- | tests/ui/uninit_vec.stderr | 24 |
3 files changed, 62 insertions, 5 deletions
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs index f80981c11af..5756e56c262 100644 --- a/clippy_utils/src/ty.rs +++ b/clippy_utils/src/ty.rs @@ -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, } diff --git a/tests/ui/uninit_vec.rs b/tests/ui/uninit_vec.rs index 0cc77a8775d..464f88140bd 100644 --- a/tests/ui/uninit_vec.rs +++ b/tests/ui/uninit_vec.rs @@ -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); + } + } } diff --git a/tests/ui/uninit_vec.stderr b/tests/ui/uninit_vec.stderr index e8b77d653f0..e7c81cf792f 100644 --- a/tests/ui/uninit_vec.stderr +++ b/tests/ui/uninit_vec.stderr @@ -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 |
