Skip to content
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

Move Layout and LayoutErr to core::mem #59

Closed
TimDiekmann opened this issue Apr 26, 2020 · 6 comments
Closed

Move Layout and LayoutErr to core::mem #59

TimDiekmann opened this issue Apr 26, 2020 · 6 comments
Labels

Comments

@TimDiekmann
Copy link
Member

TimDiekmann commented Apr 26, 2020

Layout is basically a wrapper around mem::size_of and mem::align_of (batteries included) and should live in the same module. For compatibility, Layout (and its Error) have to be reexported to core::alloc.

NB: As the discussion just came up (#57), it would also be possible to rename LayoutErr to LayoutError in core::mem and reexport it in core::alloc as LayoutErr.

@SimonSapin
Copy link
Contributor

@rust-lang/libs Any thoughts on this, since it’s about stable APIs?

@LukasKalbertodt
Copy link
Member

LukasKalbertodt commented May 3, 2020

Adding those two types to core::mem makes sense to me!

Regarding renaming to LayoutError: I'd be uncomfortable with having the same type being available with two different names. I see two viable options:

  • Also call it LayoutErr in std::mem (although that would be unfortunate, yes), or
  • Move it to core::mem::LayoutError, reexport it as alloc::alloc::LayoutError and reexport it as alloc::alloc::LayoutErr, but soft deprecate the latter. We could add a doc comment to the reexport with a soft-deprecate warning or even hide alloc::LayoutErr in the docs completely. Or even actually hard deprecating it (if #[deprecated] on reexports is even supported).

@TimDiekmann
Copy link
Member Author

TimDiekmann commented May 3, 2020

Regarding renaming to AllocError: I'd be uncomfortable with having the same type being available with two different names.

I think you meant LayoutError?

When deprecating core::alloc::LayoutErr is an option, why even reexport as core::alloc::LayoutError? Should the reexport of Layout also be marked as deprecated?

This works fine:

#[stable(feature = "alloc_layout", since = "1.28.0")]
#[rustc_deprecated(since = "1.45.0", reason = "Use `core::mem::Layout` instead")]
pub use crate::mem::Layout;

#[stable(feature = "alloc_layout", since = "1.28.0")]
#[rustc_deprecated(since = "1.45.0", reason = "Use `core::mem::LayoutError` instead")]
pub use crate::mem::LayoutError as LayoutErr;

At least it compiles. I tried to build the docs but x.py decided to recompile LLVM which will take a while...

@LukasKalbertodt
Copy link
Member

I think you meant LayoutError?

Oh, yes, sorry. Fixed.

@TimDiekmann
Copy link
Member Author

TimDiekmann commented May 3, 2020

Which #[stable] attribute core::mem::Layout should receive? since = "1.45.0"? While it's confusing, that Layout is introduced in "1.45" it would not compile on older versions.

@TimDiekmann
Copy link
Member Author

Neither #[rustc_deprecated] nor documenting a reexport is working. #[rustc_deprecated] does not emit any warning and neither generate any hints in the documentation. The best bet is to use #[doc(no_inline)] to express that this is a reexport. Maybe we could (soft-)deprecate it later when rustc/rustdoc supports this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants