Skip to content

Conversation

Noratrieb
Copy link
Member

@Noratrieb Noratrieb commented Mar 18, 2023

rustc has proper heuristics for actually checking whether a type allows being left uninitialized (by asking CTFE). We can now use this for our helper instead of rolling our own bad version with false positives.

I added this in rustc in rust-lang/rust#108669

Fix #10407

changelog: [uninit_vec]: fix false positives
changelog: [uninit_assumed_init]: fix false positives

@rustbot
Copy link
Collaborator

rustbot commented Mar 18, 2023

r? @Alexendoo

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 18, 2023
@Noratrieb Noratrieb force-pushed the uninit branch 2 times, most recently from 833a265 to 4edb4d1 Compare March 18, 2023 22:30
// edge case: For now we lint on empty arrays
let _: [u8; 0] = unsafe { MaybeUninit::uninit().assume_init() };

// edge case: For now we accept unit tuples
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also changed this comment because this isn't really a "for now", ZSTs are simply valid here.

rustc has proper heuristics for actually checking whether a type allows
being left uninitialized (by asking CTFE). We can now use this for our
helper instead of rolling our own bad version with false positives.
@Alexendoo
Copy link
Member

Great, thanks!

@bors r+

@bors
Copy link
Contributor

bors commented Mar 21, 2023

📌 Commit 84b6049 has been approved by Alexendoo

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Mar 21, 2023

⌛ Testing commit 84b6049 with merge 7b3c4aa...

@bors
Copy link
Contributor

bors commented Mar 21, 2023

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: Alexendoo
Pushing 7b3c4aa to master...

@bors bors merged commit 7b3c4aa into rust-lang:master Mar 21, 2023
@bors bors mentioned this pull request Mar 21, 2023
@Noratrieb Noratrieb deleted the uninit branch March 21, 2023 19:40
@zesterer
Copy link

I believe we've found a false-positive for this here.

@Noratrieb
Copy link
Member Author

Uh, that's because this is a polymorphic type and we are conservative around polymorphic types and always say that they are wrong.
That's not very good, open an issue. Maybe we should have/keep the bespoke logic as a fallback for polymorphic types since the rustc checks cannot handle them.

bors added a commit that referenced this pull request Mar 29, 2023
In uninit checking, add fallback for polymorphic types

After #10520, we always assumed that polymorphic types do not allow to be left uninitialized. But we can do better, by peeking into polymorphic types and adding a few special cases for going through tuples, arrays (because the length may be polymorphic) and blanket allowing all unions (like MaybeUninit).

fixes #10551

changelog: [uninit_vec]: fix false positive for polymorphic types
changelog: [uninit_assumed_init]: fix false positive for polymorphic types
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.

clippy doesn't properly model whether a type is allowed to be uninit
5 participants