From 5eafae22ceb0e4fae835a5eb7518a55e75061e20 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Fri, 7 Dec 2018 16:18:38 +0200 Subject: [PATCH 1/2] Remove confusing comment about ideally using `!` for `c_void` Using `!` for `c_void` would have the problem that pointers and potentially references to an uninhabited type would be created, and at least for references this is UB. Also document in addition that newtype wrappers around `c_void` are not recommended for representing opaque types (as a workaround for `extern type` not being stable) but instead refer to the Nomicon. --- src/libcore/ffi.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/libcore/ffi.rs b/src/libcore/ffi.rs index d7a112eb90df8..02e92af853452 100644 --- a/src/libcore/ffi.rs +++ b/src/libcore/ffi.rs @@ -13,11 +13,12 @@ use ::fmt; /// and `*mut c_void` is equivalent to C's `void*`. That said, this is /// *not* the same as C's `void` return type, which is Rust's `()` type. /// -/// Ideally, this type would be equivalent to [`!`], but currently it may -/// be more ideal to use `c_void` for FFI purposes. +/// To model pointers to opaque types in FFI, until `extern type` is +/// stabilized, it is recommended to use a newtype wrapper around an empty +/// byte array. See the [Nomicon] for details. /// -/// [`!`]: ../../std/primitive.never.html /// [pointer]: ../../std/primitive.pointer.html +/// [Nomicon]: https://doc.rust-lang.org/nomicon/ffi.html#representing-opaque-structs // N.B., for LLVM to recognize the void pointer type and by extension // functions like malloc(), we need to have it represented as i8* in // LLVM bitcode. The enum used here ensures this and prevents misuse From 8de8880b7b74cb0294c6d8c75c24656130278509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Dr=C3=B6ge?= Date: Mon, 10 Dec 2018 10:10:42 +0200 Subject: [PATCH 2/2] Update code comments of `c_void` to explain the reasoning for its current implementation We need at least two variants of the enum as otherwise the compiler complains about the #[repr(u8)] attribute and we also need at least one variant as otherwise the enum would be uninhabitated and dereferencing pointers to it would be UB. As such, mark the variants not unstable because they should not actually exist but because they are temporary implementation details until `extern type` is stable and can be used instead. --- src/libcore/ffi.rs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/libcore/ffi.rs b/src/libcore/ffi.rs index 02e92af853452..22e219d51f41f 100644 --- a/src/libcore/ffi.rs +++ b/src/libcore/ffi.rs @@ -22,16 +22,18 @@ use ::fmt; // N.B., for LLVM to recognize the void pointer type and by extension // functions like malloc(), we need to have it represented as i8* in // LLVM bitcode. The enum used here ensures this and prevents misuse -// of the "raw" type by only having private variants.. We need two +// of the "raw" type by only having private variants. We need two // variants, because the compiler complains about the repr attribute -// otherwise. +// otherwise and we need at least one variant as otherwise the enum +// would be uninhabited and at least dereferencing such pointers would +// be UB. #[repr(u8)] #[stable(feature = "raw_os", since = "1.1.0")] pub enum c_void { - #[unstable(feature = "c_void_variant", reason = "should not have to exist", + #[unstable(feature = "c_void_variant", reason = "temporary implementation detail", issue = "0")] #[doc(hidden)] __variant1, - #[unstable(feature = "c_void_variant", reason = "should not have to exist", + #[unstable(feature = "c_void_variant", reason = "temporary implementation detail", issue = "0")] #[doc(hidden)] __variant2, }