Proper bool handling

block2/Cargo.toml

@@ -25,13 +25,13 @@ std = ["alloc", "objc2-encode/std", "block-sys/std"]
 alloc = ["objc2-encode/alloc", "block-sys/alloc"]
 
 # Runtime selection. Default is `apple`. See `block-sys` for details.
-apple = ["block-sys/apple"]
-compiler-rt = ["block-sys/compiler-rt"]
-gnustep-1-7 = ["block-sys/gnustep-1-7"]
-gnustep-1-8 = ["gnustep-1-7", "block-sys/gnustep-1-8"]
-gnustep-1-9 = ["gnustep-1-8", "block-sys/gnustep-1-9"]
-gnustep-2-0 = ["gnustep-1-9", "block-sys/gnustep-2-0"]
-gnustep-2-1 = ["gnustep-2-0", "block-sys/gnustep-2-1"]
+apple = ["block-sys/apple", "objc2-encode/apple"]
+compiler-rt = ["block-sys/compiler-rt", "objc2-encode/apple"] # Use Apple's encoding
+gnustep-1-7 = ["block-sys/gnustep-1-7", "objc2-encode/gnustep-1-7"]
+gnustep-1-8 = ["gnustep-1-7", "block-sys/gnustep-1-8", "objc2-encode/gnustep-1-8"]
+gnustep-1-9 = ["gnustep-1-8", "block-sys/gnustep-1-9", "objc2-encode/gnustep-1-9"]
+gnustep-2-0 = ["gnustep-1-9", "block-sys/gnustep-2-0", "objc2-encode/gnustep-2-0"]
+gnustep-2-1 = ["gnustep-2-0", "block-sys/gnustep-2-1", "objc2-encode/gnustep-2-1"]
 
 [dependencies]
 objc2-encode = { path = "../objc2-encode", version = "=2.0.0-pre.1", default-features = false }

objc-sys/src/types.rs

@@ -68,12 +68,11 @@ mod inner {
 /// The Objective-C `BOOL` type.
 ///
 /// The type of this varies across platforms, so to convert an it into a Rust
-/// [`bool`], always compare it with [`YES`][`crate::YES`] or [`NO`][`crate::NO`].
+/// [`bool`], compare it with [`NO`][crate::NO].
 ///
-/// Note that this type implements `objc2_encode::Encode` and
-/// `objc2_encode::RefEncode`, but the `RefEncode` implementation is wrong
-/// on some platforms! You should only use this on FFI boundaries, otherwise
-/// prefer `objc2::runtime::Bool`.
+/// Note that this does _not_ implement `objc2::Encode` on all platforms! You
+/// should only use this on FFI boundaries, otherwise prefer
+/// `objc2::runtime::Bool`.
 ///
 /// See also the [corresponding documentation entry][docs].
 ///

objc2-encode/CHANGELOG.md

@@ -6,13 +6,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased - YYYY-MM-DD
 
+### Added
+* Added `EncodeConvert` trait to help with correctly handling `BOOL`/`bool`.
+
 ### Changed
 * **BREAKING**: Remove the lifetime specifier from `Encoding`, since the non
   -`'static` version was essentially useless.
 
 ### Fixed
 * Fixed the encoding output and comparison of structs behind pointers.
 
+### Removed
+* **BREAKING**: `bool` (and `AtomicBool`) no longer implements `Encode`, since
+  that was difficult to use correctly. See the `EncodeConvert` trait, or use
+  `objc2::runtime::Bool` instead.
+
 
 ## 2.0.0-pre.1 - 2022-07-19
 

objc2-encode/Cargo.toml

@@ -19,16 +19,28 @@ documentation = "https://docs.rs/objc2-encode/"
 license = "MIT"
 
 [features]
-default = ["std"]
+default = ["std", "apple"]
 
-# Doesn't do anything (this crate works on no_std), put here for forwards
-# compatibility.
-std = ["alloc"]
-alloc = []
+# Currently not possible to turn off, put here for forwards compatibility.
+std = ["alloc", "objc-sys/std"]
+alloc = ["objc-sys/alloc"]
 
 # Enables support for the nightly c_unwind feature
 unstable-c-unwind = []
 
+# Runtime selection. See `objc-sys` for details.
+apple = ["objc-sys/apple"]
+gnustep-1-7 = ["objc-sys/gnustep-1-7"]
+gnustep-1-8 = ["gnustep-1-7", "objc-sys/gnustep-1-8"]
+gnustep-1-9 = ["gnustep-1-8", "objc-sys/gnustep-1-9"]
+gnustep-2-0 = ["gnustep-1-9", "objc-sys/gnustep-2-0"]
+gnustep-2-1 = ["gnustep-2-0", "objc-sys/gnustep-2-1"]
+
+[dependencies]
+# TODO: Remove this dependency when we can select `macabi` targets separately
+# from other targets without using a build script
+objc-sys = { path = "../objc-sys", version = "=0.2.0-beta.1", default-features = false }
+
 [package.metadata.docs.rs]
 default-target = "x86_64-apple-darwin"
 

objc2/src/bool.rs → objc2-encode/src/__bool.rs

@@ -1,29 +1,20 @@
-use crate::{ffi, Encode, Encoding, RefEncode};
+//! This belongs in `objc2`, but it is put here to make `EncodeConvert` work
+//! properly!
 use core::fmt;
 
+use crate::{Encode, EncodeConvert, Encoding, RefEncode};
+
 /// The Objective-C `BOOL` type.
 ///
 /// This is a thin wrapper-type over [`objc_sys::BOOL`]. It is intended that
 /// you convert this into a Rust [`bool`] with the [`Bool::as_bool`] method as
 /// soon as possible.
 ///
-/// This is FFI-safe and can be used in directly with
-/// [`msg_send!`][`crate::msg_send`].
+/// This is FFI-safe and can be used directly with `msg_send!` and `extern`
+/// functions.
 ///
 /// Note that this is able to contain more states than `bool` on some
 /// platforms, but these cases should not be relied on!
-///
-/// # Example
-///
-/// ```no_run
-/// use objc2::{class, msg_send_bool, msg_send_id};
-/// use objc2::rc::{Id, Shared};
-/// use objc2::runtime::{Object, Bool};
-/// let ns_value: Id<Object, Shared> = unsafe {
-///     msg_send_id![class!(NSNumber), numberWithBool: Bool::YES]
-/// };
-/// assert!(unsafe { msg_send_bool![&ns_value, boolValue] });
-/// ```
 #[repr(transparent)]
 // We don't implement comparison traits because they could be implemented with
 // two slightly different semantics:
@@ -32,62 +23,60 @@ use core::fmt;
 // And it is not immediately clear for users which one was chosen.
 #[derive(Copy, Clone, Default)]
 pub struct Bool {
-    value: ffi::BOOL,
+    value: objc_sys::BOOL,
 }
 
 impl Bool {
     /// The equivalent of [`true`] for Objective-C's `BOOL` type.
-    pub const YES: Self = Self::from_raw(ffi::YES);
+    pub const YES: Self = Self::from_raw(objc_sys::YES);
 
     /// The equivalent of [`false`] for Objective-C's `BOOL` type.
-    pub const NO: Self = Self::from_raw(ffi::NO);
+    pub const NO: Self = Self::from_raw(objc_sys::NO);
 
     /// Creates an Objective-C boolean from a Rust boolean.
     #[inline]
     pub const fn new(value: bool) -> Self {
-        // true as u8 => 1
-        // false as u8 => 0
-        let value = value as ffi::BOOL;
+        // true as BOOL => 1 (YES)
+        // false as BOOL => 0 (NO)
+        let value = value as objc_sys::BOOL;
         Self { value }
     }
 
     /// Creates this from a boolean value received from a raw Objective-C API.
     #[inline]
-    pub const fn from_raw(value: ffi::BOOL) -> Self {
+    pub const fn from_raw(value: objc_sys::BOOL) -> Self {
         Self { value }
     }
 
     /// Retrieves the inner [`objc_sys::BOOL`] boolean type, to be used in raw
     /// Objective-C APIs.
     #[inline]
-    pub const fn as_raw(self) -> ffi::BOOL {
+    pub const fn as_raw(self) -> objc_sys::BOOL {
         self.value
     }
 
-    /// Returns `true` if `self` is [`NO`][`Self::NO`].
+    /// Returns `true` if `self` is [`NO`][Self::NO].
     ///
-    /// You should prefer using [`as_bool`][`Self::as_bool`].
+    /// You should prefer using [`as_bool`][Self::as_bool].
     #[inline]
     pub const fn is_false(self) -> bool {
-        // Always compare with 0
-        // This is what happens with the `!` operator in C.
-        self.value as u8 == 0
+        !self.as_bool()
     }
 
-    /// Returns `true` if `self` is the opposite of [`NO`][`Self::NO`].
+    /// Returns `true` if `self` is not [`NO`][Self::NO].
     ///
-    /// You should prefer using [`as_bool`][`Self::as_bool`].
+    /// You should prefer using [`as_bool`][Self::as_bool].
     #[inline]
     pub const fn is_true(self) -> bool {
-        // Always compare with 0
-        // This is what happens when using `if` in C.
-        self.value as u8 != 0
+        self.as_bool()
     }
 
     /// Converts this into the [`bool`] equivalent.
     #[inline]
     pub const fn as_bool(self) -> bool {
-        self.is_true()
+        // Always compare with 0 (NO)
+        // This is what happens with the `!` operator / when using `if` in C.
+        self.value != objc_sys::NO
     }
 }
 
@@ -113,7 +102,11 @@ impl fmt::Debug for Bool {
 
 // SAFETY: `Bool` is `repr(transparent)`.
 unsafe impl Encode for Bool {
-    const ENCODING: Encoding = ffi::BOOL::ENCODING;
+    // i8::__ENCODING == Encoding::Char
+    // u8::__ENCODING == Encoding::UChar
+    // bool::__ENCODING == Encoding::Bool
+    // i32::__ENCODING == Encoding::Int
+    const ENCODING: Encoding = objc_sys::BOOL::__ENCODING;
 }
 
 // Note that we shouldn't delegate to `BOOL`'s  `ENCODING_REF` since `BOOL` is
@@ -185,14 +178,13 @@ mod tests {
         assert_eq!(format!("{:?}", Bool::from(false)), "NO");
     }
 
-    // Can't really do this test since it won't compile on platforms where
-    // type BOOL = bool.
-    //
-    // #[test]
-    // fn test_outside_normal() {
-    //     let b = Bool::from_raw(42);
-    //     assert!(b.is_true());
-    //     assert!(!b.is_false());
-    //     assert_eq!(b.as_raw(), 42);
-    // }
+    #[test]
+    // Test on platform where we know the type of BOOL
+    #[cfg(all(feature = "apple", target_os = "macos", target_arch = "x86_64"))]
+    fn test_outside_normal() {
+        let b = Bool::from_raw(42);
+        assert!(b.is_true());
+        assert!(!b.is_false());
+        assert_eq!(b.as_raw(), 42);
+    }
 }