Conversation
| //! Verifies that Option<T> has the same size as T for nullable pointer types, | ||
| //! and that custom enums get the same optimization. |
There was a problem hiding this comment.
Slight correction
| //! Verifies that Option<T> has the same size as T for nullable pointer types, | |
| //! and that custom enums get the same optimization. | |
| //! Verifies that Option<T> has the same size as T for non-nullable pointer types, | |
| //! and for custom enums that have a niche. |
| @@ -1,27 +1,34 @@ | |||
| //@ run-pass | |||
| //! Nullable pointer optimization with iota-reduction for enums. | |||
There was a problem hiding this comment.
The term should be "null pointer optimization", not "nullable"; because types like &T and Box<T> are not nullable.
Also for the file name
| @@ -1,31 +1,41 @@ | |||
| //! Nullable pointer optimization preserves type sizes. | |||
There was a problem hiding this comment.
Nullable -> Null (also the file name)
tgross35
left a comment
There was a problem hiding this comment.
Few small comments, please also squash!
|
@rustbot ready |
| @@ -1,31 +1,41 @@ | |||
| //! null pointer optimization preserves type sizes. | |||
| //! | |||
| //! Verifies that Option<T> has the same size as T for null pointer types, | |||
There was a problem hiding this comment.
"null" -> "non-nullable" (A strictly-null pointer type would be a ZST :D )
There was a problem hiding this comment.
names also should be non-null in both tests?
There was a problem hiding this comment.
nvm, just hopes that not, pushed as it replaced null with non-null
There was a problem hiding this comment.
So for some context here: NPO (null pointer optimization) is an optimization on non-nullable pointer types (&T, Box<T>, etc) using the null representation. (I know, a bit confusing). Basically it knows that &T has an identical layout to *T; but &T can never be 0x0, because that it's a non-nullable type. So that is what's known as a "niche", and it can use that to store something; e.g. for Option<&T>, 0x0 means None (since it can't be &T) and any other value is Some(&T).
All that to say that comment should be "...same size as T for non-nullable pointer types", since the type has to be non-nullable for NPO to mean anything (so what you have is 👍). And then the test could be named null-pointer-optimization-sizes.rs or npo-sizes.rs (keeping null-pointer-optimization together rather than putting size in the middle)
There was a problem hiding this comment.
so last changes should be fine, thanks for detailed explanation!
There was a problem hiding this comment.
oh, wait need some adjustments
There was a problem hiding this comment.
now i believe should be fine
There was a problem hiding this comment.
Sorry, I was asking for one last thing 😄
null-pointer-size-optimization.rs->null-pointer-optimization-sizes.rssince "null pointer optimization" is a phrase
Looks like it's still at tests/ui/layout/null-pointer-size-optimization.rs?
There was a problem hiding this comment.
oh wait again then! for some reasons this message didnt rendered on my side 0_0
There was a problem hiding this comment.
ready, also recheck comments in those tests
50f65e8 to
1778762
Compare
|
Thank you! |
Rollup of 11 pull requests Successful merges: - #142440 (`tests/ui`: A New Order [14/N]) - #143040 (Add `const Rem`) - #143086 (Update poison.rs to fix the typo (sys->sync)) - #143202 (`tests/ui`: A New Order [18/N]) - #143296 (`tests/ui`: A New Order [21/N]) - #143297 (`tests/ui`: A New Order [22/N]) - #143299 (`tests/ui`: A New Order [24/N]) - #143300 (`tests/ui`: A New Order [25/N]) - #143397 (test passing a `VaList` from rust to C) - #143410 (Block SIMD in transmute_immediate; delete `OperandValueKind`) - #143452 (Fix CLI completion check in `tidy`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #143296 - Kivooeo:tf21, r=tgross35 `tests/ui`: A New Order [21/N] > [!NOTE] > > Intermediate commits are intended to help review, but will be squashed prior to merge. Some `tests/ui/` housekeeping, to trim down number of tests directly under `tests/ui/`. Part of #133895. r? `@tgross35`
Note
Intermediate commits are intended to help review, but will be squashed prior to merge.
Some
tests/ui/housekeeping, to trim down number of tests directly undertests/ui/. Part of #133895.r? @tgross35