From f887f5a9c6393d1f3eef98cdf13b4491e27b22e4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 14 Aug 2023 10:38:46 +0200 Subject: [PATCH 1/2] std: add some missing repr(transparent) --- library/core/src/ffi/c_str.rs | 3 +++ library/std/src/ffi/os_str.rs | 3 +++ library/std/src/path.rs | 6 ++++++ library/std/src/sys/wasi/fd.rs | 10 ++++++++-- library/std/src/sys_common/wtf8.rs | 2 ++ 5 files changed, 22 insertions(+), 2 deletions(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index b59ec12790d24..aa34569023eb8 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -88,6 +88,9 @@ use crate::str; // When attribute privacy is implemented, `CStr` should be annotated as `#[repr(transparent)]`. // Anyway, `CStr` representation and layout are considered implementation detail, are // not documented and must not be relied upon. +// For now we just hide this from rustdoc, technically making our doc test builds rely on +// unspecified layout assumptions. We are std, so we can get away with that. +#[cfg_attr(not(doc), repr(transparent))] pub struct CStr { // FIXME: this should not be represented with a DST slice but rather with // just a raw `c_char` along with some form of marker to make diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index 67e58fd1b8613..a9907ae10342d 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -116,6 +116,9 @@ impl crate::sealed::Sealed for OsString {} // When attribute privacy is implemented, `OsStr` should be annotated as `#[repr(transparent)]`. // Anyway, `OsStr` representation and layout are considered implementation details, are // not documented and must not be relied upon. +// For now we just hide this from rustdoc, technically making our doc test builds rely on +// unspecified layout assumptions. We are std, so we can get away with that. +#[cfg_attr(not(doc), repr(transparent))] pub struct OsStr { inner: Slice, } diff --git a/library/std/src/path.rs b/library/std/src/path.rs index 99f7a60f8ab01..ef5fc669037ab 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1164,6 +1164,9 @@ impl FusedIterator for Ancestors<'_> {} // When attribute privacy is implemented, `PathBuf` should be annotated as `#[repr(transparent)]`. // Anyway, `PathBuf` representation and layout are considered implementation detail, are // not documented and must not be relied upon. +// For now we just hide this from rustdoc, technically making our doc test builds rely on +// unspecified layout assumptions. We are std, so we can get away with that. +#[cfg_attr(not(doc), repr(transparent))] pub struct PathBuf { inner: OsString, } @@ -1989,6 +1992,9 @@ impl AsRef for PathBuf { // When attribute privacy is implemented, `Path` should be annotated as `#[repr(transparent)]`. // Anyway, `Path` representation and layout are considered implementation detail, are // not documented and must not be relied upon. +// For now we just hide this from rustdoc, technically making our doc test builds rely on +// unspecified layout assumptions. We are std, so we can get away with that. +#[cfg_attr(not(doc), repr(transparent))] pub struct Path { inner: OsStr, } diff --git a/library/std/src/sys/wasi/fd.rs b/library/std/src/sys/wasi/fd.rs index 1b50c2ea6dd57..d7295a799daab 100644 --- a/library/std/src/sys/wasi/fd.rs +++ b/library/std/src/sys/wasi/fd.rs @@ -16,14 +16,20 @@ pub struct WasiFd { fn iovec<'a>(a: &'a mut [IoSliceMut<'_>]) -> &'a [wasi::Iovec] { assert_eq!(mem::size_of::>(), mem::size_of::()); assert_eq!(mem::align_of::>(), mem::align_of::()); - // SAFETY: `IoSliceMut` and `IoVec` have exactly the same memory layout + // SAFETY: `IoSliceMut` and `IoVec` have exactly the same memory layout. + // We decorate our `IoSliceMut` with `repr(transparent)` (see `io.rs`), and + // `crate::io::IoSliceMut` is a `repr(transparent)` wrapper around our type, so this is + // guaranteed. unsafe { mem::transmute(a) } } fn ciovec<'a>(a: &'a [IoSlice<'_>]) -> &'a [wasi::Ciovec] { assert_eq!(mem::size_of::>(), mem::size_of::()); assert_eq!(mem::align_of::>(), mem::align_of::()); - // SAFETY: `IoSlice` and `CIoVec` have exactly the same memory layout + // SAFETY: `IoSlice` and `CIoVec` have exactly the same memory layout. + // We decorate our `IoSlice` with `repr(transparent)` (see `io.rs`), and + // `crate::io::IoSlice` is a `repr(transparent)` wrapper around our type, so this is + // guaranteed. unsafe { mem::transmute(a) } } diff --git a/library/std/src/sys_common/wtf8.rs b/library/std/src/sys_common/wtf8.rs index 195d175cc9bde..67db5ebd89cfc 100644 --- a/library/std/src/sys_common/wtf8.rs +++ b/library/std/src/sys_common/wtf8.rs @@ -459,6 +459,7 @@ impl Wtf8Buf { /// Converts this `Wtf8Buf` into a boxed `Wtf8`. #[inline] pub fn into_box(self) -> Box { + // SAFETY: relies on `Wtf8` being `repr(transparent)`. unsafe { mem::transmute(self.bytes.into_boxed_slice()) } } @@ -511,6 +512,7 @@ impl Extend for Wtf8Buf { /// Similar to `&str`, but can additionally contain surrogate code points /// if they’re not in a surrogate pair. #[derive(Eq, Ord, PartialEq, PartialOrd)] +#[repr(transparent)] pub struct Wtf8 { bytes: [u8], } From fe1a034f16adbed4302e4d27be96a7ec6fb27177 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 14 Aug 2023 22:55:29 +0200 Subject: [PATCH 2/2] actually this doesn't even affect doctests. nice. --- library/core/src/ffi/c_str.rs | 9 +++------ library/std/src/ffi/os_str.rs | 9 +++------ library/std/src/path.rs | 18 ++++++------------ 3 files changed, 12 insertions(+), 24 deletions(-) diff --git a/library/core/src/ffi/c_str.rs b/library/core/src/ffi/c_str.rs index aa34569023eb8..4082b208c1263 100644 --- a/library/core/src/ffi/c_str.rs +++ b/library/core/src/ffi/c_str.rs @@ -82,14 +82,11 @@ use crate::str; #[stable(feature = "core_c_str", since = "1.64.0")] #[rustc_has_incoherent_inherent_impls] #[lang = "CStr"] -// FIXME: // `fn from` in `impl From<&CStr> for Box` current implementation relies // on `CStr` being layout-compatible with `[u8]`. -// When attribute privacy is implemented, `CStr` should be annotated as `#[repr(transparent)]`. -// Anyway, `CStr` representation and layout are considered implementation detail, are -// not documented and must not be relied upon. -// For now we just hide this from rustdoc, technically making our doc test builds rely on -// unspecified layout assumptions. We are std, so we can get away with that. +// However, `CStr` layout is considered an implementation detail and must not be relied upon. We +// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under +// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy. #[cfg_attr(not(doc), repr(transparent))] pub struct CStr { // FIXME: this should not be represented with a DST slice but rather with diff --git a/library/std/src/ffi/os_str.rs b/library/std/src/ffi/os_str.rs index a9907ae10342d..43cecb19b148e 100644 --- a/library/std/src/ffi/os_str.rs +++ b/library/std/src/ffi/os_str.rs @@ -110,14 +110,11 @@ impl crate::sealed::Sealed for OsString {} /// [conversions]: super#conversions #[cfg_attr(not(test), rustc_diagnostic_item = "OsStr")] #[stable(feature = "rust1", since = "1.0.0")] -// FIXME: // `OsStr::from_inner` current implementation relies // on `OsStr` being layout-compatible with `Slice`. -// When attribute privacy is implemented, `OsStr` should be annotated as `#[repr(transparent)]`. -// Anyway, `OsStr` representation and layout are considered implementation details, are -// not documented and must not be relied upon. -// For now we just hide this from rustdoc, technically making our doc test builds rely on -// unspecified layout assumptions. We are std, so we can get away with that. +// However, `OsStr` layout is considered an implementation detail and must not be relied upon. We +// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under +// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy. #[cfg_attr(not(doc), repr(transparent))] pub struct OsStr { inner: Slice, diff --git a/library/std/src/path.rs b/library/std/src/path.rs index ef5fc669037ab..5842c096f1ab7 100644 --- a/library/std/src/path.rs +++ b/library/std/src/path.rs @@ -1158,14 +1158,11 @@ impl FusedIterator for Ancestors<'_> {} /// Which method works best depends on what kind of situation you're in. #[cfg_attr(not(test), rustc_diagnostic_item = "PathBuf")] #[stable(feature = "rust1", since = "1.0.0")] -// FIXME: // `PathBuf::as_mut_vec` current implementation relies // on `PathBuf` being layout-compatible with `Vec`. -// When attribute privacy is implemented, `PathBuf` should be annotated as `#[repr(transparent)]`. -// Anyway, `PathBuf` representation and layout are considered implementation detail, are -// not documented and must not be relied upon. -// For now we just hide this from rustdoc, technically making our doc test builds rely on -// unspecified layout assumptions. We are std, so we can get away with that. +// However, `PathBuf` layout is considered an implementation detail and must not be relied upon. We +// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under +// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy. #[cfg_attr(not(doc), repr(transparent))] pub struct PathBuf { inner: OsString, @@ -1986,14 +1983,11 @@ impl AsRef for PathBuf { /// ``` #[cfg_attr(not(test), rustc_diagnostic_item = "Path")] #[stable(feature = "rust1", since = "1.0.0")] -// FIXME: // `Path::new` current implementation relies // on `Path` being layout-compatible with `OsStr`. -// When attribute privacy is implemented, `Path` should be annotated as `#[repr(transparent)]`. -// Anyway, `Path` representation and layout are considered implementation detail, are -// not documented and must not be relied upon. -// For now we just hide this from rustdoc, technically making our doc test builds rely on -// unspecified layout assumptions. We are std, so we can get away with that. +// However, `Path` layout is considered an implementation detail and must not be relied upon. We +// want `repr(transparent)` but we don't want it to show up in rustdoc, so we hide it under +// `cfg(doc)`. This is an ad-hoc implementation of attribute privacy. #[cfg_attr(not(doc), repr(transparent))] pub struct Path { inner: OsStr,