Merge pull request #198 from madsmtm/verify-under-debug-assertions

Rework verification feature selection

.github/workflows/ci.yml

@@ -155,7 +155,7 @@ jobs:
  # Use --no-fail-fast, except with dinghy
  TESTARGS: ${{ matrix.dinghy && ' ' || '--no-fail-fast' }} ${{ matrix.test-args }}
  SOME_FEATURES: ${{ matrix.features || 'malloc,block,exception,foundation' }}
- FEATURES: ${{ matrix.features || 'malloc,block,exception,foundation,catch-all,verify_message,uuid' }}
+ FEATURES: ${{ matrix.features || 'malloc,block,exception,foundation,catch-all,verify,uuid' }}
  UNSTABLE_FEATURES: ${{ matrix.unstable-features || 'unstable-autoreleasesafe,unstable-c-unwind' }}
  CMD: cargo
 

crates/objc2-encode/src/encoding_box.rs

@@ -105,6 +105,27 @@ impl EncodingBox {
  Encoding::ULongLong => Self::ULongLong,
  _ => unreachable!(),
  };
+
+ /// Parse and comsume an encoding from the start of a string.
+ ///
+ /// This is can be used to parse concatenated encodings, such as those
+ /// returned by `method_getTypeEncoding`.
+ ///
+ /// [`from_str`][Self::from_str] is simpler, use that instead if you can.
+ pub fn from_start_of_str(s: &mut &str) -> Result<Self, ParseError> {
+ let mut parser = Parser::new(s);
+ parser.strip_leading_qualifiers();
+
+ match parser.parse_encoding() {
+ Err(err) => Err(ParseError::new(parser, err)),
+ Ok(encoding) => {
+ let remaining = parser.remaining();
+ *s = remaining;
+
+ Ok(encoding)
+ }
+ }
+ }
 }
 
 /// Same formatting as [`Encoding`]'s `Display` implementation.
@@ -213,4 +234,19 @@ mod tests {
  assert_eq!(expected, actual);
  assert_eq!(expected.to_string(), "AA{a}");
  }
+
+ #[test]
+ fn parse_part_of_string() {
+ let mut s = "{a}c";
+
+ let expected = EncodingBox::Struct("a".into(), None);
+ let actual = EncodingBox::from_start_of_str(&mut s).unwrap();
+ assert_eq!(expected, actual);
+
+ let expected = EncodingBox::Char;
+ let actual = EncodingBox::from_start_of_str(&mut s).unwrap();
+ assert_eq!(expected, actual);
+
+ assert_eq!(s, "");
+ }
 }

crates/objc2-encode/src/parse.rs

@@ -117,6 +117,10 @@ impl<'a> Parser<'a> {
  }
  }
 
+ pub(crate) fn remaining(&self) -> &'a str {
+ &self.data[self.split_point..]
+ }
+
  fn peek(&self) -> Result<u8> {
  self.try_peek().ok_or(ErrorKind::UnexpectedEnd)
  }

crates/objc2/CHANGELOG.md

@@ -31,26 +31,35 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
  in methods declared with `extern_methods!`, and let that be the `NSError**`
  parameter.
 * Added `#[method_id(...)]` attribute to `extern_methods!`.
+* Added `"verify"` feature as a replacement for the `"verify_message"`
+ feature.
 
 ### Changed
 * Allow other types than `&Class` as the receiver in `msg_send_id!` methods
  of the `new` family.
 * **BREAKING**: Changed the `Allocated` struct to be used as `Allocated<T>`
  instead of `Id<Allocated<T>, O>`.
 * Verify the message signature of overriden methods when declaring classes if
- the `verify_message` feature is enabled.
+ the `verify` feature is enabled.
 * Verify in `declare_class!` that protocols are implemented correctly.
 * **BREAKING**: Changed the name of the attribute macro in `extern_methods`
  from `#[sel(...)]` to `#[method(...)]`.
 * **BREAKING**: Changed `extern_methods!` and `declare_class!` such that
  associated functions whoose first parameter is called `this`, is treated as
  instance methods instead of class methods.
+* **BREAKING**: Message verification is now enabled by default. Your message
+ sends might panic with `debug_assertions` enabled if they are detected to
+ be invalid. Test your code to see if that is the case!
 
 ### Fixed
 * Fixed duplicate selector extraction in `extern_methods!`.
 * Fixed using `deprecated` attributes in `declare_class!`.
 * Fixed `cfg` attributes on methods and implementations in `declare_class!`.
 
+### Removed
+* **BREAKING**: Removed `"verify_message"` feature. It has been mostly
+ replaced by `debug_assertions` and the `"verify"` feature.
+
 
 ## 0.3.0-beta.3 - 2022-09-01
 

crates/objc2/Cargo.toml

@@ -34,9 +34,8 @@ exception = ["objc-sys/unstable-exception"]
 # Wrap every `objc2::msg_send` call in a `@try/@catch` block
 catch-all = ["exception"]
 
-# Verify type encodings on every message send
-# Only intended to be used while debugging!
-verify_message = ["malloc"] # TODO: Remove malloc feature here
+# Enable all verification steps when debug assertions are enabled.
+verify = ["malloc"]
 
 # Expose features that require linking to `libc::free`.
 #

crates/objc2/src/__macro_helpers.rs

@@ -1,16 +1,16 @@
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 use alloc::vec::Vec;
 use core::ptr;
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 use std::collections::HashSet;
 
 use crate::declare::ClassBuilder;
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 use crate::declare::MethodImplementation;
 use crate::encode::Encode;
 use crate::message::__TupleExtender;
 use crate::rc::{Allocated, Id, Ownership, Shared};
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 use crate::runtime::MethodDescription;
 use crate::runtime::{Class, Object, Protocol, Sel};
 use crate::{Message, MessageArguments, MessageReceiver};
@@ -478,7 +478,7 @@ impl ModuleInfo {
 
 impl ClassBuilder {
  #[doc(hidden)]
- #[cfg(feature = "verify_message")]
+ #[cfg(all(debug_assertions, feature = "verify"))]
  pub fn __add_protocol_methods<'a, 'b>(
  &'a mut self,
  protocol: &'b Protocol,
@@ -497,7 +497,7 @@ impl ClassBuilder {
  }
 
  #[doc(hidden)]
- #[cfg(not(feature = "verify_message"))]
+ #[cfg(not(all(debug_assertions, feature = "verify")))]
  #[inline]
  pub fn __add_protocol_methods(&mut self, protocol: &Protocol) -> &mut Self {
  self.add_protocol(protocol);
@@ -509,7 +509,7 @@ impl ClassBuilder {
 /// - Only methods on the protocol are overriden.
 /// - TODO: The methods have the correct signature.
 /// - All required methods are overridden.
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 pub struct ClassProtocolMethodsBuilder<'a, 'b> {
  builder: &'a mut ClassBuilder,
  protocol: &'b Protocol,
@@ -521,7 +521,7 @@ pub struct ClassProtocolMethodsBuilder<'a, 'b> {
  registered_class_methods: HashSet<Sel>,
 }
 
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 impl ClassProtocolMethodsBuilder<'_, '_> {
  #[inline]
  pub unsafe fn add_method<T, F>(&mut self, sel: Sel, func: F)
@@ -579,7 +579,7 @@ impl ClassProtocolMethodsBuilder<'_, '_> {
  }
 }
 
-#[cfg(feature = "verify_message")]
+#[cfg(all(debug_assertions, feature = "verify"))]
 impl Drop for ClassProtocolMethodsBuilder<'_, '_> {
  fn drop(&mut self) {
  for desc in &self.required_instance_methods {
@@ -778,7 +778,7 @@ mod tests {
 
  #[test]
  #[should_panic = "unexpected NULL newMethodOnInstance; receiver was NULL"]
- #[cfg(not(feature = "verify_message"))] // Does NULL receiver checks
+ #[cfg(not(debug_assertions))] // Does NULL receiver checks
  fn test_new_any_with_null_receiver() {
  let obj: *const NSObject = ptr::null();
  let _obj: Id<Object, Shared> = unsafe { msg_send_id![obj, newMethodOnInstance] };
@@ -801,7 +801,7 @@ mod tests {
 
  #[test]
  #[should_panic = "failed allocating object"]
- #[cfg(not(feature = "verify_message"))] // Does NULL receiver checks
+ #[cfg(not(debug_assertions))] // Does NULL receiver checks
  fn test_init_with_null_receiver() {
  let obj: Option<Allocated<RcTestObject>> = None;
  let _obj: Id<RcTestObject, Owned> = unsafe { msg_send_id![obj, init] };
@@ -823,7 +823,7 @@ mod tests {
 
  #[test]
  #[should_panic = "unexpected NULL description; receiver was NULL"]
- #[cfg(not(feature = "verify_message"))] // Does NULL receiver checks
+ #[cfg(not(debug_assertions))] // Does NULL receiver checks
  fn test_normal_with_null_receiver() {
  let obj: *const NSObject = ptr::null();
  let _obj: Id<NSString, Shared> = unsafe { msg_send_id![obj, description] };