Skip to content

Conversation

Centri3
Copy link
Member

@Centri3 Centri3 commented Jun 15, 2023

Although this doesn't close #9983, I think this is good enough and is more general than such a change to large_enum_variant :)

changelog: New lint [null_pointer_optimization]

@rustbot
Copy link
Collaborator

rustbot commented Jun 15, 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 Jun 15, 2023
@Centri3 Centri3 force-pushed the null_pointer_optimization branch from 778fcee to 55c3023 Compare June 16, 2023 00:27
@bors
Copy link
Contributor

bors commented Jun 19, 2023

☔ The latest upstream changes (presumably #10951) made this pull request unmergeable. Please resolve the merge conflicts.

@Centri3 Centri3 force-pushed the null_pointer_optimization branch from 55c3023 to f4abdab Compare June 19, 2023 03:42
@Alexendoo
Copy link
Member

Sometimes you do need a &/&mut Option<T> which you can't get from the Option<Box<T>>, it would be worth mentioning that somewhere, in a note or the description at the very least

@Centri3
Copy link
Member Author

Centri3 commented Jun 19, 2023

IIUC, into_inner would let you but it's unstable, so, but yes I will add it to the description

@Centri3 Centri3 force-pushed the null_pointer_optimization branch from f4abdab to cf628b1 Compare June 19, 2023 15:33
@Alexendoo
Copy link
Member

Box::into_inner(b) is just *b

So technically yeah you can move the T out of the box and place it into an Option and take a reference to that, but this consumes the box and can't be done if you have a &/&mut Option<Box<T>>

@Alexendoo Alexendoo added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Jul 18, 2023
@Centri3
Copy link
Member Author

Centri3 commented Jul 18, 2023

We could probably add an additional check for #[repr(transparent)] types around Box or NonNull, they're likely uncommon but it's still guaranteed to be optimized

Description should probably be updated (again!) to mention *b over into_inner

@Centri3 Centri3 force-pushed the null_pointer_optimization branch from cf628b1 to 9dbcf93 Compare August 1, 2023 08:25
@bors
Copy link
Contributor

bors commented Aug 11, 2023

☔ The latest upstream changes (presumably #11316) made this pull request unmergeable. Please resolve the merge conflicts.

@Alexendoo Alexendoo added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties I-nominated Issue: Nominated to be discussed at the next Clippy meeting and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Aug 31, 2023
@Alexendoo Alexendoo removed the I-nominated Issue: Nominated to be discussed at the next Clippy meeting label Sep 18, 2023
@xFrednet
Copy link
Contributor

Hey this is triage, I'm closing this due to inactivity. Currently, @Centri3 sadly doesn't have the time to continue this implementation. If anyone is interested in continuing this PR, you're more than welcome to create a new PR and push it over the finish line. :D

Thank you to @Centri3 and the reviewers for the time, that you already put into this!

@rustbot label +S-inactive-closed -S-waiting-on-author -S-waiting-on-review

@xFrednet xFrednet closed this Mar 31, 2024
@rustbot rustbot added S-inactive-closed Status: Closed due to inactivity and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 31, 2024
@Centri3 Centri3 deleted the null_pointer_optimization branch April 1, 2024 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-inactive-closed Status: Closed due to inactivity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clippy::large_enum_variant should suggest Option<Box<T>> over Box<Option<T>>
5 participants