-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Kill array_assume_init #103134
base: master
Are you sure you want to change the base?
Kill array_assume_init #103134
Conversation
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
This comment has been minimized.
This comment has been minimized.
eb99568
to
8820c89
Compare
This comment has been minimized.
This comment has been minimized.
I don't see |
@@ -161,22 +161,6 @@ fn assume_init_good() { | |||
assert!(TRUE); | |||
} | |||
|
|||
#[test] | |||
fn uninit_array_assume_init() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there an equivalent test for transpose
already?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe? There are doc tests, but again transpose is NOT assume_init so I'm not sure what we'd be testing (that transmute works?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd be testing that this same behavior still works with the new methods, which sounds useful to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, added back.
#[rustc_const_unstable(feature = "const_maybe_uninit_array_assume_init", issue = "96097")] | ||
#[inline(always)] | ||
#[track_caller] | ||
pub const unsafe fn array_assume_init<const N: usize>(array: [Self; N]) -> [T; N] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like I've mentioned in some other PRs, I'm not willing to take an API removal (even an unstable one) without libs-api team oversight.
Up to you if you want to bring this up in an ACP to see if T-libs-api agrees, or if you want to make this just a T-libs change that updates the implementations of things to use transpose
without actually removing things.
@rustbot author
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nah, I want to get rid of these. Created rust-lang/libs-team#122
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, this is probably what you were trying to tell me but I think it'll be easier to split stdlib usage removal from api removal, so I'll pull out the usage removals.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, it's always easiest to split things up by decision makers.
Such as, where possible:
- changes to
compiler
separate fromlibrary
- additions separate from removals
- internal details separate from anything impacting callers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haha yeah, thanks. I feel like I've learned this lesson at least a dozen times by now, but the initial ease of piling a bunch of crap together is too good a fool. :)
Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
8820c89
to
8e68496
Compare
…=scottmcm Remove all uses of array_assume_init See rust-lang#103134 (comment) r? `@scottmcm`
Waiting on ACP in rust-lang/libs-team#122 -- libs-api indicated that |
Remove all uses of array_assume_init See rust-lang/rust#103134 (comment) r? `@scottmcm`
☔ The latest upstream changes (presumably #107634) made this pull request unmergeable. Please resolve the merge conflicts. |
No longer needed with transpose.
r? @scottmcm