From bffd0dbc70892f7c5f6cd5c4bb61143605477c24 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 1 Sep 2023 21:26:18 +0200 Subject: [PATCH 1/6] Remove lower-version clang workarounds --- crates/header-translator/framework-includes.h | 6 ------ crates/header-translator/src/rust_type.rs | 12 ++++++++++++ 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/crates/header-translator/framework-includes.h b/crates/header-translator/framework-includes.h index f4bb47113..22b1d3410 100644 --- a/crates/header-translator/framework-includes.h +++ b/crates/header-translator/framework-includes.h @@ -1,9 +1,3 @@ -// Workaround for clang < 13, only used in NSBundle.h -#define NS_FORMAT_ARGUMENT(A) - -// Workaround for clang < 13 -#define _Nullable_result _Nullable - #include #if TARGET_OS_OSX diff --git a/crates/header-translator/src/rust_type.rs b/crates/header-translator/src/rust_type.rs index 7e1858eaa..f6ab4a238 100644 --- a/crates/header-translator/src/rust_type.rs +++ b/crates/header-translator/src/rust_type.rs @@ -141,6 +141,10 @@ impl AttributeParser<'_, '_> { None } } + + fn nullable_result(&mut self, position: ParsePosition) -> bool { + self.strip("_Nullable_result", position) + } } impl Drop for AttributeParser<'_, '_> { @@ -584,6 +588,10 @@ impl Inner { let is_const = get_is_const(parser.is_const(ParsePosition::Suffix)); lifetime.update(parser.lifetime(ParsePosition::Suffix)); + + // TODO: Use _Nullable_result + let _nullable_result = parser.nullable_result(ParsePosition::Suffix); + let nullability = if let Some(nullability) = unexposed_nullability { nullability } else { @@ -672,6 +680,10 @@ impl Inner { let is_const = get_is_const(parser.is_const(ParsePosition::Suffix)); lifetime.update(parser.lifetime(ParsePosition::Suffix)); + + // TODO: Use _Nullable_result + let _nullable_result = parser.nullable_result(ParsePosition::Suffix); + let mut nullability = if let Some(nullability) = unexposed_nullability { nullability } else { From da6484ee96639d1e598f7d43009a3d967c31a91b Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sun, 3 Sep 2023 04:37:29 +0200 Subject: [PATCH 2/6] Parse sendability/mainthread attributes --- crates/header-translator/src/cache.rs | 1 + crates/header-translator/src/method.rs | 47 +++++++- crates/header-translator/src/rust_type.rs | 63 +++++++++-- crates/header-translator/src/stmt.rs | 106 +++++++++++++++--- .../header-translator/src/unexposed_attr.rs | 24 +++- .../header-translator/translation-config.toml | 6 + crates/icrate/CHANGELOG.md | 2 + crates/icrate/src/generated | 2 +- 8 files changed, 214 insertions(+), 37 deletions(-) diff --git a/crates/header-translator/src/cache.rs b/crates/header-translator/src/cache.rs index a6c772a78..0aa9883b3 100644 --- a/crates/header-translator/src/cache.rs +++ b/crates/header-translator/src/cache.rs @@ -84,6 +84,7 @@ impl<'a> Cache<'a> { ty: enum_ty, kind: _, variants: _, + sendable: _, }) = iter.peek_mut() { if enum_ty.is_typedef_to(&id.name) { diff --git a/crates/header-translator/src/method.rs b/crates/header-translator/src/method.rs index 55427b43d..78b91ac7b 100644 --- a/crates/header-translator/src/method.rs +++ b/crates/header-translator/src/method.rs @@ -52,6 +52,9 @@ struct MethodModifiers { returns_retained: bool, returns_not_retained: bool, designated_initializer: bool, + non_isolated: bool, + sendable: Option, + mainthreadonly: bool, } impl MethodModifiers { @@ -68,6 +71,18 @@ impl MethodModifiers { UnexposedAttr::ReturnsNotRetained => { this.returns_not_retained = true; } + UnexposedAttr::NonIsolated => { + this.non_isolated = true; + } + UnexposedAttr::Sendable => { + this.sendable = Some(true); + } + UnexposedAttr::NonSendable => { + this.sendable = Some(false); + } + UnexposedAttr::UIActor => { + this.mainthreadonly = true; + } attr => error!(?attr, "unknown attribute"), } } @@ -214,6 +229,7 @@ impl MemoryManagement { consumes_self: false, returns_retained: false, returns_not_retained: false, + .. } = modifiers { Self::Normal @@ -238,6 +254,9 @@ pub struct Method { safe: bool, mutating: bool, is_protocol: bool, + // Thread-safe, even on main-thread only (@MainActor/@UIActor) classes + non_isolated: bool, + mainthreadonly: bool, } impl Method { @@ -367,6 +386,10 @@ impl<'tu> PartialMethod<'tu> { let modifiers = MethodModifiers::parse(&entity, context); + if modifiers.sendable.is_some() { + error!("sendable on method"); + } + let mut arguments: Vec<_> = entity .get_arguments() .expect("method arguments") @@ -377,6 +400,7 @@ impl<'tu> PartialMethod<'tu> { let qualifier = entity .get_objc_qualifiers() .map(MethodArgumentQualifier::parse); + let mut sendable = None; immediate_children(&entity, |entity, _span| match entity.get_kind() { EntityKind::ObjCClassRef @@ -390,7 +414,11 @@ impl<'tu> PartialMethod<'tu> { } EntityKind::UnexposedAttr => { if let Some(attr) = UnexposedAttr::parse(&entity, context) { - error!(?attr, "unknown attribute"); + match attr { + UnexposedAttr::Sendable => sendable = Some(true), + UnexposedAttr::NonSendable => sendable = Some(false), + attr => error!(?attr, "unknown attribute"), + } } } // For some reason we recurse into array types @@ -399,7 +427,7 @@ impl<'tu> PartialMethod<'tu> { }); let ty = entity.get_type().expect("argument type"); - let ty = Ty::parse_method_argument(ty, qualifier, context); + let ty = Ty::parse_method_argument(ty, qualifier, sendable, context); (name, ty) }) @@ -463,6 +491,8 @@ impl<'tu> PartialMethod<'tu> { // immutable subclass, or as a property. mutating: data.mutating.unwrap_or(parent_is_mutable), is_protocol, + non_isolated: modifiers.non_isolated, + mainthreadonly: modifiers.mainthreadonly, }, )) } @@ -519,6 +549,7 @@ impl PartialProperty<'_> { let ty = Ty::parse_property_return( entity.get_type().expect("property type"), is_copy, + modifiers.sendable, context, ); @@ -538,6 +569,8 @@ impl PartialProperty<'_> { // is, so let's default to immutable. mutating: getter_data.mutating.unwrap_or(false), is_protocol, + non_isolated: modifiers.non_isolated, + mainthreadonly: modifiers.mainthreadonly, }) } else { None @@ -546,8 +579,12 @@ impl PartialProperty<'_> { let setter = if let Some(setter_name) = setter_name { let setter_data = setter_data.expect("setter_data must be present if setter_name was"); if !setter_data.skipped { - let ty = - Ty::parse_property(entity.get_type().expect("property type"), is_copy, context); + let ty = Ty::parse_property( + entity.get_type().expect("property type"), + is_copy, + modifiers.sendable, + context, + ); let selector = setter_name.clone() + ":"; let memory_management = @@ -566,6 +603,8 @@ impl PartialProperty<'_> { // Setters are usually mutable if the class itself is. mutating: setter_data.mutating.unwrap_or(parent_is_mutable), is_protocol, + non_isolated: modifiers.non_isolated, + mainthreadonly: modifiers.mainthreadonly, }) } else { None diff --git a/crates/header-translator/src/rust_type.rs b/crates/header-translator/src/rust_type.rs index f6ab4a238..9fcf11170 100644 --- a/crates/header-translator/src/rust_type.rs +++ b/crates/header-translator/src/rust_type.rs @@ -532,10 +532,30 @@ impl Inner { let unexposed_nullability = if let TypeKind::Unexposed = ty.get_kind() { let nullability = ty.get_nullability(); - attributed_name = parse_unexposed_tokens(&attributed_name); + let (new_attributed_name, attributed_attr) = parse_unexposed_tokens(&attributed_name); // Also parse the expected name to ensure that the formatting that // TokenStream does is the same on both. - name = parse_unexposed_tokens(&name); + let (new_name, attr) = parse_unexposed_tokens(&name); + if attributed_attr != attr { + error!( + ?attributed_attr, + ?attr, + "attributed attr was not equal to attr", + ); + } + + match attr { + Some(UnexposedAttr::NonIsolated | UnexposedAttr::UIActor) => { + // Ignored for now; these are almost always also emitted on the method/property, + // which is where they will be useful in any case. + } + Some(attr) => error!(?attr, "unknown attribute"), + None => {} + } + + attributed_name = new_attributed_name; + name = new_name; + ty = ty .get_modified_type() .expect("attributed type to have modified type"); @@ -1193,6 +1213,7 @@ impl Ty { pub fn parse_method_argument( ty: Type<'_>, _qualifier: Option, + _sendable: Option, context: &Context<'_>, ) -> Self { let ty = Inner::parse(ty, Lifetime::Unspecified, context); @@ -1253,7 +1274,7 @@ impl Ty { } pub fn parse_function_argument(ty: Type<'_>, context: &Context<'_>) -> Self { - let mut this = Self::parse_method_argument(ty, None, context); + let mut this = Self::parse_method_argument(ty, None, None, context); this.kind = TyKind::FnArgument; this } @@ -1304,6 +1325,7 @@ impl Ty { ty: Type<'_>, // Ignored; see `parse_property_return` _is_copy: bool, + _sendable: Option, context: &Context<'_>, ) -> Self { let ty = Inner::parse(ty, Lifetime::Unspecified, context); @@ -1320,7 +1342,12 @@ impl Ty { } } - pub fn parse_property_return(ty: Type<'_>, is_copy: bool, context: &Context<'_>) -> Self { + pub fn parse_property_return( + ty: Type<'_>, + is_copy: bool, + _sendable: Option, + context: &Context<'_>, + ) -> Self { let mut ty = Inner::parse(ty, Lifetime::Unspecified, context); // `@property(copy)` is expected to always return a nonnull instance @@ -1736,10 +1763,10 @@ impl fmt::Display for Ty { /// - NS_SWIFT_UNAVAILABLE /// - NS_REFINED_FOR_SWIFT /// - ... -fn parse_unexposed_tokens(s: &str) -> String { +fn parse_unexposed_tokens(s: &str) -> (String, Option) { let tokens = TokenStream::from_str(s).expect("parse attributed name"); let mut iter = tokens.into_iter().peekable(); - if let Some(TokenTree::Ident(ident)) = iter.peek() { + let attr = if let Some(TokenTree::Ident(ident)) = iter.peek() { let ident = ident.to_string(); if let Ok(attr) = UnexposedAttr::from_name(&ident, || { iter.next(); @@ -1750,13 +1777,15 @@ fn parse_unexposed_tokens(s: &str) -> String { None } }) { - if let Some(attr) = attr { - error!(?attr, "unknown attribute"); - } iter.next(); + attr + } else { + None } - } - TokenStream::from_iter(iter).to_string() + } else { + None + }; + (TokenStream::from_iter(iter).to_string(), attr) } #[cfg(test)] @@ -1766,7 +1795,9 @@ mod tests { #[test] fn test_parse_unexposed_tokens() { fn check(inp: &str, expected: &str) { - assert_eq!(parse_unexposed_tokens(inp), expected); + let (actual, attr) = parse_unexposed_tokens(inp); + assert_eq!(actual, expected); + assert_eq!(attr, None); } check("NS_RETURNS_INNER_POINTER const char *", "const char *"); @@ -1791,5 +1822,13 @@ mod tests { "API_DEPRECATED_WITH_REPLACEMENT(\"@\\\"com.adobe.encapsulated-postscript\\\"\", macos(10.0,10.14)) NSPasteboardType __strong", "NSPasteboardType __strong", ); + + let (actual, attr) = parse_unexposed_tokens("NS_SWIFT_NONISOLATED NSTextAttachment *"); + assert_eq!(actual, "NSTextAttachment *"); + assert_eq!(attr, Some(UnexposedAttr::NonIsolated)); + + let (actual, attr) = parse_unexposed_tokens("NS_SWIFT_UI_ACTOR SEL"); + assert_eq!(actual, "SEL"); + assert_eq!(attr, Some(UnexposedAttr::UIActor)); } } diff --git a/crates/header-translator/src/stmt.rs b/crates/header-translator/src/stmt.rs index 4a7597123..f7a18792d 100644 --- a/crates/header-translator/src/stmt.rs +++ b/crates/header-translator/src/stmt.rs @@ -140,6 +140,29 @@ fn parse_class_generics(entity: &Entity<'_>, _context: &Context<'_>) -> Vec, context: &Context<'_>) -> (Option, bool) { + let mut sendable = None; + let mut mainthreadonly = false; + + immediate_children(entity, |entity, _span| { + if let EntityKind::UnexposedAttr = entity.get_kind() { + if let Some(attr) = UnexposedAttr::parse(&entity, context) { + match attr { + UnexposedAttr::Sendable => sendable = Some(true), + UnexposedAttr::NonSendable => sendable = Some(false), + UnexposedAttr::UIActor => { + sendable = Some(false); + mainthreadonly = true; + } + attr => error!(?attr, "unknown attribute"), + } + } + } + }); + + (sendable, mainthreadonly) +} + fn parse_methods( entity: &Entity<'_>, get_data: impl Fn(&str) -> MethodData, @@ -212,7 +235,7 @@ fn parse_methods( /// - `EntityKind::ObjCInterfaceDecl` /// - `EntityKind::ObjCProtocolDecl` /// - `EntityKind::ObjCCategoryDecl` -fn verify_objc_decl(entity: &Entity<'_>, context: &Context<'_>) { +fn verify_objc_decl(entity: &Entity<'_>, _context: &Context<'_>) { let parent_kind = entity.get_kind(); immediate_children(entity, |entity, _span| { @@ -268,9 +291,7 @@ fn verify_objc_decl(entity: &Entity<'_>, context: &Context<'_>) { // Maybe useful for knowing when to implement `Error` for the type } (EntityKind::UnexposedAttr, _) => { - if let Some(attr) = UnexposedAttr::parse(&entity, context) { - error!(?attr, "unknown attribute"); - } + // Parsed in parse_attributes } (_, parent_kind) => error!(?parent_kind, "unknown in parent"), } @@ -327,6 +348,8 @@ pub enum Stmt { derives: Derives, mutability: Mutability, skipped: bool, + sendable: bool, + mainthreadonly: bool, }, /// @interface class_name (name) /// -> @@ -349,6 +372,8 @@ pub enum Stmt { availability: Availability, protocols: BTreeSet, methods: Vec, + required_sendable: bool, + required_mainthreadonly: bool, }, /// @interface ty: _ /// @interface ty (_) @@ -377,6 +402,7 @@ pub enum Stmt { availability: Availability, boxable: bool, fields: Vec<(String, Ty)>, + sendable: Option, }, /// typedef NS_OPTIONS(type, name) { /// variants* @@ -399,6 +425,7 @@ pub enum Stmt { ty: Ty, kind: Option, variants: Vec<(String, Availability, Expr)>, + sendable: Option, }, /// static const ty name = expr; /// extern const ty name; @@ -431,14 +458,22 @@ pub enum Stmt { }, } -fn parse_struct(entity: &Entity<'_>, context: &Context<'_>) -> (bool, Vec<(String, Ty)>) { +fn parse_struct( + entity: &Entity<'_>, + context: &Context<'_>, +) -> (bool, Vec<(String, Ty)>, Option) { let mut boxable = false; let mut fields = Vec::new(); + let mut sendable = None; immediate_children(entity, |entity, span| match entity.get_kind() { EntityKind::UnexposedAttr => { if let Some(attr) = UnexposedAttr::parse(&entity, context) { - error!(?attr, "unknown attribute"); + match attr { + UnexposedAttr::Sendable => sendable = Some(true), + UnexposedAttr::NonSendable => sendable = Some(false), + attr => error!(?attr, "unknown attribute"), + } } } EntityKind::FieldDecl => { @@ -462,7 +497,7 @@ fn parse_struct(entity: &Entity<'_>, context: &Context<'_>) -> (bool, Vec<(Strin _ => error!("unknown"), }); - (boxable, fields) + (boxable, fields, sendable) } fn parse_fn_param_children(entity: &Entity<'_>, context: &Context<'_>) { @@ -530,6 +565,8 @@ impl Stmt { context, ); + let (sendable, mainthreadonly) = parse_attributes(entity, context); + let mut protocols = Default::default(); parse_protocols(entity, &mut protocols, context); @@ -555,6 +592,8 @@ impl Stmt { .filter_map(|(superclass_id, _, entity)| { let superclass_data = context.class_data.get(&superclass_id.name); + // let (sendable, mainthreadonly) = parse_attributes(entity, context); + // Explicitly keep going, even if the class itself is skipped // if superclass_data.skipped @@ -610,6 +649,8 @@ impl Stmt { derives: data.map(|data| data.derives.clone()).unwrap_or_default(), mutability: data.map(|data| data.mutability.clone()).unwrap_or_default(), skipped: data.map(|data| data.definition_skipped).unwrap_or_default(), + sendable: sendable.unwrap_or(false), + mainthreadonly, }) .chain(protocols.into_iter().map(|protocol| Self::ProtocolImpl { cls: id.clone(), @@ -652,6 +693,14 @@ impl Stmt { context, ); + let (sendable, mainthreadonly) = parse_attributes(entity, context); + if sendable.is_some() { + error!(?sendable, "sendable on category"); + } + if mainthreadonly { + error!("@UIActor on category"); + } + if !designated_initializers.is_empty() { warn!( ?designated_initializers, @@ -670,6 +719,8 @@ impl Stmt { let subclass_data = context.class_data.get(&subclass.name); assert!(!subclass_data.map(|data| data.skipped).unwrap_or_default()); + // let (sendable, mainthreadonly) = parse_attributes(entity, context); + let (mut methods, _) = parse_methods( entity, |name| { @@ -748,6 +799,8 @@ impl Stmt { context, ); + let (sendable, mainthreadonly) = parse_attributes(entity, context); + if !designated_initializers.is_empty() { warn!( ?designated_initializers, @@ -760,6 +813,8 @@ impl Stmt { availability, protocols, methods, + required_sendable: sendable.unwrap_or(false), + required_mainthreadonly: mainthreadonly, }] } EntityKind::TypedefDecl => { @@ -776,7 +831,11 @@ impl Stmt { if kind.is_some() { panic!("got multiple unexposed attributes {kind:?}, {attr:?}"); } - kind = Some(attr); + match attr { + // TODO + UnexposedAttr::Sendable => warn!("sendable on typedef"), + _ => kind = Some(attr), + } } } EntityKind::StructDecl => { @@ -812,7 +871,7 @@ impl Stmt { _ => error!("unknown"), }); - if let Some((boxable, fields)) = struct_ { + if let Some((boxable, fields, sendable)) = struct_ { assert_eq!(kind, None, "should not have parsed a kind"); return vec![Self::StructDecl { id, @@ -820,6 +879,7 @@ impl Stmt { availability, boxable, fields, + sendable, }]; } @@ -870,13 +930,14 @@ impl Stmt { } if !id.name.starts_with('_') { - let (boxable, fields) = parse_struct(entity, context); + let (boxable, fields, sendable) = parse_struct(entity, context); return vec![Self::StructDecl { id, encoding_name: None, availability, boxable, fields, + sendable, }]; } } @@ -907,6 +968,7 @@ impl Stmt { let ty = Ty::parse_enum(ty, context); let mut kind = None; let mut variants = Vec::new(); + let mut sendable = None; immediate_children(entity, |entity, _span| match entity.get_kind() { EntityKind::EnumConstantDecl => { @@ -941,10 +1003,19 @@ impl Stmt { } EntityKind::UnexposedAttr => { if let Some(attr) = UnexposedAttr::parse(&entity, context) { - if let Some(kind) = &kind { - assert_eq!(kind, &attr, "got differing enum kinds in {id:?}"); - } else { - kind = Some(attr); + match attr { + UnexposedAttr::Sendable => sendable = Some(true), + UnexposedAttr::NonSendable => sendable = Some(false), + attr => { + if let Some(kind) = &kind { + assert_eq!( + kind, &attr, + "got differing enum kinds in {id:?}" + ); + } else { + kind = Some(attr); + } + } } } } @@ -972,6 +1043,7 @@ impl Stmt { ty, kind, variants, + sendable, }] } EntityKind::VarDecl => { @@ -1247,6 +1319,8 @@ impl fmt::Display for Stmt { derives, mutability, skipped, + sendable: _, + mainthreadonly: _, } => { if *skipped { return Ok(()); @@ -1511,6 +1585,8 @@ impl fmt::Display for Stmt { availability, protocols, methods, + required_sendable: _, + required_mainthreadonly: _, } => { writeln!(f, "extern_protocol!(")?; write!(f, "{availability}")?; @@ -1573,6 +1649,7 @@ impl fmt::Display for Stmt { availability, boxable: _, fields, + sendable: _, } => { writeln!(f, "extern_struct!(")?; if let Some(encoding_name) = encoding_name { @@ -1597,6 +1674,7 @@ impl fmt::Display for Stmt { ty, kind, variants, + sendable: _, } => { let macro_name = match kind { None => "extern_enum", diff --git a/crates/header-translator/src/unexposed_attr.rs b/crates/header-translator/src/unexposed_attr.rs index 671ca0d38..879779eaf 100644 --- a/crates/header-translator/src/unexposed_attr.rs +++ b/crates/header-translator/src/unexposed_attr.rs @@ -20,6 +20,11 @@ pub enum UnexposedAttr { ReturnsRetained, ReturnsNotRetained, + + Sendable, + NonSendable, + UIActor, + NonIsolated, } impl UnexposedAttr { @@ -55,6 +60,18 @@ impl UnexposedAttr { "NS_RETURNS_RETAINED" | "CF_RETURNS_RETAINED" => Some(Self::ReturnsRetained), "NS_RETURNS_NOT_RETAINED" | "CF_RETURNS_NOT_RETAINED" => Some(Self::ReturnsNotRetained), "NS_RETURNS_INNER_POINTER" => None, + // This has two arguments: `sendability` and `nullability`. + // `nullability` is already exposed, so we won't bother with that. + // `sendability` is most for backwards-compatibility with older + // versions of system headers that didn't assign sendability. + "NS_HEADER_AUDIT_BEGIN" => { + let _ = get_arguments(); + None + } + "NS_SWIFT_SENDABLE" => Some(Self::Sendable), + "NS_SWIFT_NONSENDABLE" => Some(Self::NonSendable), + "NS_SWIFT_UI_ACTOR" => Some(Self::UIActor), + "NS_SWIFT_NONISOLATED" => Some(Self::NonIsolated), // TODO "NS_FORMAT_FUNCTION" | "NS_FORMAT_ARGUMENT" => { let _ = get_arguments(); @@ -140,7 +157,6 @@ impl UnexposedAttr { s if s.starts_with("FILEPROVIDER_API_AVAILABILITY_") => None, // Might be interesting in the future "CF_SWIFT_NAME" - | "NS_HEADER_AUDIT_BEGIN" | "NS_REFINED_FOR_SWIFT_ASYNC" | "NS_SWIFT_ASYNC_NAME" | "NS_SWIFT_ASYNC_THROWS_ON_FALSE" @@ -155,11 +171,7 @@ impl UnexposedAttr { "CF_REFINED_FOR_SWIFT" | "NS_REFINED_FOR_SWIFT" | "NS_SWIFT_DISABLE_ASYNC" - | "NS_SWIFT_NONISOLATED" - | "NS_SWIFT_NONSENDABLE" - | "NS_SWIFT_NOTHROW" - | "NS_SWIFT_SENDABLE" - | "NS_SWIFT_UI_ACTOR" => None, + | "NS_SWIFT_NOTHROW" => None, _ => return Err(()), }) } diff --git a/crates/header-translator/translation-config.toml b/crates/header-translator/translation-config.toml index 4037f6abd..31027f382 100644 --- a/crates/header-translator/translation-config.toml +++ b/crates/header-translator/translation-config.toml @@ -1607,3 +1607,9 @@ skipped-protocols = ["NSCopying", "NSMutableCopying"] skipped-protocols = ["NSCopying", "NSMutableCopying"] [class.NSPurgeableData] skipped-protocols = ["NSCopying", "NSMutableCopying"] + +# Uses `NS_SWIFT_UI_ACTOR` on a static, which is hard to support. +# +# Will have to be a method that takes `MainThreadMarker`. +[static.NSApp] +skipped = true diff --git a/crates/icrate/CHANGELOG.md b/crates/icrate/CHANGELOG.md index 7d41653ec..97636647d 100644 --- a/crates/icrate/CHANGELOG.md +++ b/crates/icrate/CHANGELOG.md @@ -24,6 +24,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Removed * **BREAKING**: Removed the `MainThreadMarker` argument from the closure passed to `MainThreadBound::get_on_main`. +* **BREAKING**: Removed the `NSApp` static for now - it will likely be + re-added later in another form. ## icrate 0.0.4 - 2023-07-31 diff --git a/crates/icrate/src/generated b/crates/icrate/src/generated index 5bbaa2d26..a8844894e 160000 --- a/crates/icrate/src/generated +++ b/crates/icrate/src/generated @@ -1 +1 @@ -Subproject commit 5bbaa2d266e0e0253347c45a39be2784b016892c +Subproject commit a8844894e7c9dca271c325429a57c9b2dde76dee From aa4f3fde3cd1691e03de1b96e8beb0495c7ed52e Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 1 Sep 2023 21:58:33 +0200 Subject: [PATCH 3/6] Parse NS_NOESCAPE --- crates/header-translator/src/id.rs | 4 ++ crates/header-translator/src/method.rs | 4 +- crates/header-translator/src/rust_type.rs | 57 +++++++++++++++++-- .../header-translator/src/unexposed_attr.rs | 5 +- 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/crates/header-translator/src/id.rs b/crates/header-translator/src/id.rs index 5ec83cdb9..3a36d92d8 100644 --- a/crates/header-translator/src/id.rs +++ b/crates/header-translator/src/id.rs @@ -119,6 +119,10 @@ impl ItemIdentifier { self.library == "Foundation" && self.name == "NSString" } + pub fn is_nscomparator(&self) -> bool { + self.library == "Foundation" && self.name == "NSComparator" + } + pub fn feature(&self) -> Option { struct ItemIdentifierFeature<'a>(&'a ItemIdentifier); diff --git a/crates/header-translator/src/method.rs b/crates/header-translator/src/method.rs index 78b91ac7b..7cda71d9f 100644 --- a/crates/header-translator/src/method.rs +++ b/crates/header-translator/src/method.rs @@ -401,6 +401,7 @@ impl<'tu> PartialMethod<'tu> { .get_objc_qualifiers() .map(MethodArgumentQualifier::parse); let mut sendable = None; + let mut no_escape = false; immediate_children(&entity, |entity, _span| match entity.get_kind() { EntityKind::ObjCClassRef @@ -417,6 +418,7 @@ impl<'tu> PartialMethod<'tu> { match attr { UnexposedAttr::Sendable => sendable = Some(true), UnexposedAttr::NonSendable => sendable = Some(false), + UnexposedAttr::NoEscape => no_escape = true, attr => error!(?attr, "unknown attribute"), } } @@ -427,7 +429,7 @@ impl<'tu> PartialMethod<'tu> { }); let ty = entity.get_type().expect("argument type"); - let ty = Ty::parse_method_argument(ty, qualifier, sendable, context); + let ty = Ty::parse_method_argument(ty, qualifier, sendable, no_escape, context); (name, ty) }) diff --git a/crates/header-translator/src/rust_type.rs b/crates/header-translator/src/rust_type.rs index 9fcf11170..3534246ab 100644 --- a/crates/header-translator/src/rust_type.rs +++ b/crates/header-translator/src/rust_type.rs @@ -501,10 +501,13 @@ enum Inner { }, Fn { is_variadic: bool, + no_escape: bool, arguments: Vec, result_type: Box, }, Block { + sendable: Option, + no_escape: bool, arguments: Vec, result_type: Box, }, @@ -546,7 +549,7 @@ impl Inner { match attr { Some(UnexposedAttr::NonIsolated | UnexposedAttr::UIActor) => { - // Ignored for now; these are almost always also emitted on the method/property, + // Ignored for now; these are usually also emitted on the method/property, // which is where they will be useful in any case. } Some(attr) => error!(?attr, "unknown attribute"), @@ -681,12 +684,15 @@ impl Inner { match Self::parse(ty, Lifetime::Unspecified, context) { Self::Fn { is_variadic: false, + no_escape, arguments, result_type, } => Self::Pointer { nullability, is_const, pointee: Box::new(Self::Block { + sendable: None, + no_escape, arguments, result_type, }), @@ -924,6 +930,7 @@ impl Inner { Self::Fn { is_variadic: ty.is_variadic(), + no_escape: false, arguments, result_type: Box::new(result_type), } @@ -1008,11 +1015,14 @@ impl Inner { // // } Self::Fn { + is_variadic: _, + no_escape: _, arguments, result_type, - .. } | Self::Block { + sendable: _, + no_escape: _, arguments, result_type, } => { @@ -1106,6 +1116,7 @@ impl fmt::Display for Inner { } => match &**pointee { Self::Fn { is_variadic, + no_escape: _, arguments, result_type, } => { @@ -1161,6 +1172,8 @@ impl fmt::Display for Inner { Enum { id } | Struct { id } | TypeDef { id } => write!(f, "{}", id.path()), Self::Fn { .. } => write!(f, "TodoFunction"), Self::Block { + sendable: _, + no_escape: _, arguments, result_type, } => { @@ -1213,10 +1226,44 @@ impl Ty { pub fn parse_method_argument( ty: Type<'_>, _qualifier: Option, - _sendable: Option, + mut arg_sendable: Option, + mut arg_no_escape: bool, context: &Context<'_>, ) -> Self { - let ty = Inner::parse(ty, Lifetime::Unspecified, context); + let mut ty = Inner::parse(ty, Lifetime::Unspecified, context); + + match &mut ty { + Inner::Pointer { pointee, .. } => match &mut **pointee { + Inner::Block { + sendable, + no_escape, + .. + } => { + *sendable = arg_sendable; + *no_escape = arg_no_escape; + arg_sendable = None; + arg_no_escape = false; + } + Inner::Fn { no_escape, .. } => { + *no_escape = arg_no_escape; + arg_no_escape = false; + } + _ => {} + }, + // Ignore NSComparator for now + Inner::TypeDef { id } if id.is_nscomparator() => { + arg_no_escape = false; + } + _ => {} + } + + if arg_sendable.is_some() { + warn!(?ty, "did not consume sendable in argument"); + } + + if arg_no_escape { + warn!(?ty, "did not consume no_escape in argument"); + } match &ty { Inner::Pointer { pointee, .. } => pointee.visit_lifetime(|lifetime| { @@ -1274,7 +1321,7 @@ impl Ty { } pub fn parse_function_argument(ty: Type<'_>, context: &Context<'_>) -> Self { - let mut this = Self::parse_method_argument(ty, None, None, context); + let mut this = Self::parse_method_argument(ty, None, None, false, context); this.kind = TyKind::FnArgument; this } diff --git a/crates/header-translator/src/unexposed_attr.rs b/crates/header-translator/src/unexposed_attr.rs index 879779eaf..d48db1933 100644 --- a/crates/header-translator/src/unexposed_attr.rs +++ b/crates/header-translator/src/unexposed_attr.rs @@ -25,6 +25,8 @@ pub enum UnexposedAttr { NonSendable, UIActor, NonIsolated, + + NoEscape, } impl UnexposedAttr { @@ -77,8 +79,7 @@ impl UnexposedAttr { let _ = get_arguments(); None } - // TODO - "NS_NOESCAPE" => None, + "NS_NOESCAPE" => Some(Self::NoEscape), // TODO: We could potentially automatically elide this argument // from the method call, though it's rare enough that it's // probably not really worth the effort. From 78f9941c1d27d0cb1584516ee30c67fc689d0720 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 2 Sep 2023 01:02:29 +0200 Subject: [PATCH 4/6] Use sendability attributes to implement Send + Sync --- crates/header-translator/src/stmt.rs | 47 +++++++++++++++++-- crates/icrate/CHANGELOG.md | 2 + .../icrate/src/additions/Foundation/error.rs | 4 -- .../icrate/src/additions/Foundation/number.rs | 5 -- .../icrate/src/additions/Foundation/uuid.rs | 4 -- crates/icrate/src/generated | 2 +- 6 files changed, 47 insertions(+), 17 deletions(-) diff --git a/crates/header-translator/src/stmt.rs b/crates/header-translator/src/stmt.rs index f7a18792d..7e79d700f 100644 --- a/crates/header-translator/src/stmt.rs +++ b/crates/header-translator/src/stmt.rs @@ -1319,7 +1319,7 @@ impl fmt::Display for Stmt { derives, mutability, skipped, - sendable: _, + sendable, mainthreadonly: _, } => { if *skipped { @@ -1429,6 +1429,20 @@ impl fmt::Display for Stmt { } writeln!(f, " }}")?; writeln!(f, ");")?; + + if *sendable && generics.is_empty() { + writeln!(f)?; + if let Some(feature) = &main_feature_gate { + writeln!(f, " #[cfg(feature = \"{feature}\")]")?; + } + writeln!(f, "unsafe impl Send for {} {{}}", id.name)?; + + writeln!(f)?; + if let Some(feature) = &main_feature_gate { + writeln!(f, " #[cfg(feature = \"{feature}\")]")?; + } + writeln!(f, "unsafe impl Sync for {} {{}}", id.name)?; + } } Self::Methods { cls, @@ -1608,6 +1622,15 @@ impl fmt::Display for Stmt { write!(f, "{}", protocol.path())?; } } + // TODO + // if *required_sendable { + // if protocols.is_empty() { + // write!(f, ": ")?; + // } else { + // write!(f, "+ ")?; + // } + // write!(f, "Send + Sync")?; + // } writeln!(f, " {{")?; for method in methods { @@ -1649,7 +1672,7 @@ impl fmt::Display for Stmt { availability, boxable: _, fields, - sendable: _, + sendable, } => { writeln!(f, "extern_struct!(")?; if let Some(encoding_name) = encoding_name { @@ -1667,6 +1690,14 @@ impl fmt::Display for Stmt { } writeln!(f, " }}")?; writeln!(f, ");")?; + + if let Some(true) = sendable { + writeln!(f)?; + writeln!(f, "unsafe impl Send for {} {{}}", id.name)?; + + writeln!(f)?; + writeln!(f, "unsafe impl Sync for {} {{}}", id.name)?; + } } Self::EnumDecl { id, @@ -1674,7 +1705,7 @@ impl fmt::Display for Stmt { ty, kind, variants, - sendable: _, + sendable, } => { let macro_name = match kind { None => "extern_enum", @@ -1698,6 +1729,16 @@ impl fmt::Display for Stmt { } writeln!(f, " }}")?; writeln!(f, ");")?; + + if let Some(true) = sendable { + if let Some(name) = &id.name { + writeln!(f)?; + writeln!(f, "unsafe impl Send for {name} {{}}")?; + + writeln!(f)?; + writeln!(f, "unsafe impl Sync for {name} {{}}")?; + } + } } Self::VarDecl { id, diff --git a/crates/icrate/CHANGELOG.md b/crates/icrate/CHANGELOG.md index 97636647d..dada78da5 100644 --- a/crates/icrate/CHANGELOG.md +++ b/crates/icrate/CHANGELOG.md @@ -12,6 +12,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Added * Added `MainThreadMarker` `From` implementation for `MainThreadOnly` types. +* Added `Send` and `Sync` implementations for a bunch more types (same as the + ones Swift marks as `@Sendable`). ### Changed * Moved the `ns_string!` macro to `icrate::Foundation::ns_string`. The old diff --git a/crates/icrate/src/additions/Foundation/error.rs b/crates/icrate/src/additions/Foundation/error.rs index 48be36a90..54d615ca4 100644 --- a/crates/icrate/src/additions/Foundation/error.rs +++ b/crates/icrate/src/additions/Foundation/error.rs @@ -9,10 +9,6 @@ use crate::Foundation::{ self, NSError, NSErrorDomain, NSErrorUserInfoKey, NSInteger, NSLocalizedDescriptionKey, }; -// SAFETY: Error objects are immutable data containers. -unsafe impl Sync for NSError {} -unsafe impl Send for NSError {} - impl UnwindSafe for NSError {} impl RefUnwindSafe for NSError {} diff --git a/crates/icrate/src/additions/Foundation/number.rs b/crates/icrate/src/additions/Foundation/number.rs index 386239da4..8a997e5ea 100644 --- a/crates/icrate/src/additions/Foundation/number.rs +++ b/crates/icrate/src/additions/Foundation/number.rs @@ -20,11 +20,6 @@ use objc2::encode::Encoding; use crate::common::*; use crate::Foundation::{CGFloat, NSNumber}; -// SAFETY: `NSNumber` is a wrapper around an integer/float/bool, and it is -// immutable. -unsafe impl Sync for NSNumber {} -unsafe impl Send for NSNumber {} - impl UnwindSafe for NSNumber {} impl RefUnwindSafe for NSNumber {} diff --git a/crates/icrate/src/additions/Foundation/uuid.rs b/crates/icrate/src/additions/Foundation/uuid.rs index 05d4385ca..df6036670 100644 --- a/crates/icrate/src/additions/Foundation/uuid.rs +++ b/crates/icrate/src/additions/Foundation/uuid.rs @@ -5,10 +5,6 @@ use core::panic::{RefUnwindSafe, UnwindSafe}; use crate::common::*; use crate::Foundation::{self, UuidBytes, NSUUID}; -// SAFETY: `NSUUID` is immutable. -unsafe impl Sync for NSUUID {} -unsafe impl Send for NSUUID {} - impl UnwindSafe for NSUUID {} impl RefUnwindSafe for NSUUID {} diff --git a/crates/icrate/src/generated b/crates/icrate/src/generated index a8844894e..bfe4ba4a9 160000 --- a/crates/icrate/src/generated +++ b/crates/icrate/src/generated @@ -1 +1 @@ -Subproject commit a8844894e7c9dca271c325429a57c9b2dde76dee +Subproject commit bfe4ba4a9871a46d1f9374854501b0a32eb608a6 From a897b422e60b9fa6d583f74ffa033a6c2728b948 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 2 Sep 2023 01:09:49 +0200 Subject: [PATCH 5/6] Automatically determine MainThreadOnly ClassType::Mutability --- crates/header-translator/src/stmt.rs | 48 +++++++++++++------ crates/icrate/CHANGELOG.md | 19 +++++++- crates/icrate/examples/delegate.rs | 5 +- crates/icrate/src/common.rs | 4 +- crates/icrate/src/fixes/AppKit/mod.rs | 2 +- .../src/fixes/AuthenticationServices/mod.rs | 6 +-- crates/icrate/src/generated | 2 +- 7 files changed, 61 insertions(+), 25 deletions(-) diff --git a/crates/header-translator/src/stmt.rs b/crates/header-translator/src/stmt.rs index 7e79d700f..ff06650d3 100644 --- a/crates/header-translator/src/stmt.rs +++ b/crates/header-translator/src/stmt.rs @@ -306,7 +306,7 @@ pub enum Mutability { MutableWithImmutableSuperclass(ItemIdentifier), #[default] InteriorMutable, - // MainThreadOnly, + MainThreadOnly, } impl Mutability { @@ -330,6 +330,7 @@ impl fmt::Display for Mutability { write!(f, "MutableWithImmutableSuperclass<{}>", superclass.path()) } Self::InteriorMutable => write!(f, "InteriorMutable"), + Self::MainThreadOnly => write!(f, "MainThreadOnly"), } } } @@ -349,7 +350,6 @@ pub enum Stmt { mutability: Mutability, skipped: bool, sendable: bool, - mainthreadonly: bool, }, /// @interface class_name (name) /// -> @@ -565,7 +565,7 @@ impl Stmt { context, ); - let (sendable, mainthreadonly) = parse_attributes(entity, context); + let (sendable, mut mainthreadonly) = parse_attributes(entity, context); let mut protocols = Default::default(); parse_protocols(entity, &mut protocols, context); @@ -579,7 +579,17 @@ impl Stmt { let superclasses: Vec<_> = superclasses_full .iter() - .map(|(id, generics, _)| (id.clone(), generics.clone())) + .map(|(id, generics, entity)| { + // Ignore sendability on superclasses; because it's an auto trait, it's propagated to subclasses anyhow! + let (_sendable, superclass_mainthreadonly) = + parse_attributes(entity, context); + + if superclass_mainthreadonly { + mainthreadonly = true; + } + + (id.clone(), generics.clone()) + }) .collect(); // Used for duplicate checking (sometimes the subclass @@ -592,8 +602,6 @@ impl Stmt { .filter_map(|(superclass_id, _, entity)| { let superclass_data = context.class_data.get(&superclass_id.name); - // let (sendable, mainthreadonly) = parse_attributes(entity, context); - // Explicitly keep going, even if the class itself is skipped // if superclass_data.skipped @@ -647,10 +655,13 @@ impl Stmt { superclasses, designated_initializers, derives: data.map(|data| data.derives.clone()).unwrap_or_default(), - mutability: data.map(|data| data.mutability.clone()).unwrap_or_default(), + mutability: if mainthreadonly { + Mutability::MainThreadOnly + } else { + data.map(|data| data.mutability.clone()).unwrap_or_default() + }, skipped: data.map(|data| data.definition_skipped).unwrap_or_default(), sendable: sendable.unwrap_or(false), - mainthreadonly, }) .chain(protocols.into_iter().map(|protocol| Self::ProtocolImpl { cls: id.clone(), @@ -694,7 +705,7 @@ impl Stmt { ); let (sendable, mainthreadonly) = parse_attributes(entity, context); - if sendable.is_some() { + if let Some(sendable) = sendable { error!(?sendable, "sendable on category"); } if mainthreadonly { @@ -710,7 +721,18 @@ impl Stmt { let superclasses: Vec<_> = parse_superclasses(entity, context) .into_iter() - .map(|(id, generics, _)| (id, generics)) + .map(|(id, generics, entity)| { + let (sendable, mainthreadonly) = parse_attributes(&entity, context); + + if let Some(sendable) = sendable { + error!(?sendable, "sendable on category superclass"); + } + if mainthreadonly { + error!("@UIActor on category superclass"); + } + + (id, generics) + }) .collect(); let subclass_methods = if let Mutability::ImmutableWithMutableSubclass(subclass) = @@ -719,8 +741,6 @@ impl Stmt { let subclass_data = context.class_data.get(&subclass.name); assert!(!subclass_data.map(|data| data.skipped).unwrap_or_default()); - // let (sendable, mainthreadonly) = parse_attributes(entity, context); - let (mut methods, _) = parse_methods( entity, |name| { @@ -1320,7 +1340,6 @@ impl fmt::Display for Stmt { mutability, skipped, sendable, - mainthreadonly: _, } => { if *skipped { return Ok(()); @@ -1337,7 +1356,8 @@ impl fmt::Display for Stmt { Mutability::Immutable | Mutability::Mutable | Mutability::ImmutableWithMutableSubclass(_) - | Mutability::InteriorMutable => id.feature(), + | Mutability::InteriorMutable + | Mutability::MainThreadOnly => id.feature(), }; let (superclass, superclasses_rest) = superclasses.split_at(1); diff --git a/crates/icrate/CHANGELOG.md b/crates/icrate/CHANGELOG.md index dada78da5..f091fb1d2 100644 --- a/crates/icrate/CHANGELOG.md +++ b/crates/icrate/CHANGELOG.md @@ -19,9 +19,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). * Moved the `ns_string!` macro to `icrate::Foundation::ns_string`. The old location in the crate root is deprecated. * Use SDK from Xcode 14.3.1 (previously Xcode 14.2). -* **BREAKING**: The following two methods on `MTLAccelerationStructureCommandEncoder` now take a nullable scratch buffer: +* **BREAKING**: The following two methods on + `MTLAccelerationStructureCommandEncoder` now take a nullable scratch buffer: - `refitAccelerationStructure_descriptor_destination_scratchBuffer_scratchBufferOffset` - `refitAccelerationStructure_descriptor_destination_scratchBuffer_scratchBufferOffset_options` +* **BREAKING**: Marked UI-related types as `MainThreadOnly`. This means that + they can now only be constructed on the main thread, meaning you have to + aquire a `MainThreadMarker` first. + + ```rust + // Before + let app = unsafe { NSApplication::sharedApplication() }; + let view = unsafe { NSView::initWithFrame(NSView::alloc(), frame) }; + // Do something with `app` and `view` + + // After + let mtm = MainThreadMarker::new().unwrap(); + let app = unsafe { NSApplication::sharedApplication(mtm) }; + let view = unsafe { NSView::initWithFrame(mtm.alloc(), frame) }; + // Do something with `app` and `view` + ``` ### Removed * **BREAKING**: Removed the `MainThreadMarker` argument from the closure diff --git a/crates/icrate/examples/delegate.rs b/crates/icrate/examples/delegate.rs index 6f3bef426..5615e7f13 100644 --- a/crates/icrate/examples/delegate.rs +++ b/crates/icrate/examples/delegate.rs @@ -24,9 +24,8 @@ declare_class!( mod ivars; unsafe impl ClassType for AppDelegate { - #[inherits(NSObject)] - type Super = icrate::AppKit::NSResponder; - type Mutability = mutability::InteriorMutable; + type Super = NSObject; + type Mutability = mutability::MainThreadOnly; const NAME: &'static str = "MyAppDelegate"; } diff --git a/crates/icrate/src/common.rs b/crates/icrate/src/common.rs index fdc9fdaae..b6ba9c38f 100644 --- a/crates/icrate/src/common.rs +++ b/crates/icrate/src/common.rs @@ -14,8 +14,8 @@ pub(crate) use std::os::raw::{ pub(crate) use objc2::ffi::{NSInteger, NSIntegerMax, NSUInteger, NSUIntegerMax, IMP}; #[cfg(feature = "objective-c")] pub(crate) use objc2::mutability::{ - Immutable, ImmutableWithMutableSubclass, InteriorMutable, IsIdCloneable, Mutable, - MutableWithImmutableSuperclass, + Immutable, ImmutableWithMutableSubclass, InteriorMutable, IsIdCloneable, MainThreadOnly, + Mutable, MutableWithImmutableSuperclass, }; #[cfg(feature = "objective-c")] pub(crate) use objc2::rc::{Allocated, DefaultId, Id}; diff --git a/crates/icrate/src/fixes/AppKit/mod.rs b/crates/icrate/src/fixes/AppKit/mod.rs index a4c965482..44975eff4 100644 --- a/crates/icrate/src/fixes/AppKit/mod.rs +++ b/crates/icrate/src/fixes/AppKit/mod.rs @@ -36,7 +36,7 @@ extern_class!( unsafe impl ClassType for NSPopover { #[inherits(NSObject)] type Super = crate::AppKit::NSResponder; - type Mutability = InteriorMutable; + type Mutability = MainThreadOnly; } ); diff --git a/crates/icrate/src/fixes/AuthenticationServices/mod.rs b/crates/icrate/src/fixes/AuthenticationServices/mod.rs index 3de214181..26657c8f6 100644 --- a/crates/icrate/src/fixes/AuthenticationServices/mod.rs +++ b/crates/icrate/src/fixes/AuthenticationServices/mod.rs @@ -19,7 +19,7 @@ extern_class!( #[cfg(feature = "AuthenticationServices_ASCredentialProviderViewController")] unsafe impl ClassType for ASCredentialProviderViewController { type Super = ASViewController; - type Mutability = InteriorMutable; + type Mutability = MainThreadOnly; } ); @@ -31,7 +31,7 @@ extern_class!( #[cfg(feature = "AuthenticationServices_ASAccountAuthenticationModificationViewController")] unsafe impl ClassType for ASAccountAuthenticationModificationViewController { type Super = ASViewController; - type Mutability = InteriorMutable; + type Mutability = MainThreadOnly; } ); @@ -43,6 +43,6 @@ extern_class!( #[cfg(feature = "AuthenticationServices_ASAuthorizationAppleIDButton")] unsafe impl ClassType for ASAuthorizationAppleIDButton { type Super = ASControl; - type Mutability = InteriorMutable; + type Mutability = MainThreadOnly; } ); diff --git a/crates/icrate/src/generated b/crates/icrate/src/generated index bfe4ba4a9..7205d0e7b 160000 --- a/crates/icrate/src/generated +++ b/crates/icrate/src/generated @@ -1 +1 @@ -Subproject commit bfe4ba4a9871a46d1f9374854501b0a32eb608a6 +Subproject commit 7205d0e7bd68d94c0b6a4effddc7b746df7eae52 From 53e3cc4afced588b630a039802a0837353d99647 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Sat, 2 Sep 2023 00:41:09 +0200 Subject: [PATCH 6/6] Automatically generate MainThreadMarker argument in methods --- crates/header-translator/src/cache.rs | 89 ++++++++++++++++++- crates/header-translator/src/method.rs | 15 +++- crates/header-translator/src/rust_type.rs | 33 +++++++ crates/icrate/Cargo.toml | 7 +- crates/icrate/examples/browser.rs | 42 ++++----- crates/icrate/examples/delegate.rs | 12 +-- crates/icrate/examples/metal.rs | 23 ++--- crates/icrate/src/additions/Foundation/mod.rs | 3 +- .../icrate/src/additions/Foundation/thread.rs | 14 ++- crates/icrate/src/generated | 2 +- crates/objc2/src/macros/__method_msg_send.rs | 5 +- crates/objc2/tests/macros_mainthreadmarker.rs | 4 +- 12 files changed, 195 insertions(+), 54 deletions(-) diff --git a/crates/header-translator/src/cache.rs b/crates/header-translator/src/cache.rs index 0aa9883b3..e3adf4c1c 100644 --- a/crates/header-translator/src/cache.rs +++ b/crates/header-translator/src/cache.rs @@ -1,4 +1,4 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::mem; use crate::config::Config; @@ -7,16 +7,38 @@ use crate::id::ItemIdentifier; use crate::method::Method; use crate::output::Output; use crate::stmt::Stmt; +use crate::Mutability; /// A helper struct for doing global analysis on the output. #[derive(Debug, PartialEq, Clone)] pub struct Cache<'a> { config: &'a Config, + mainthreadonly_classes: BTreeSet, } impl<'a> Cache<'a> { - pub fn new(_output: &Output, config: &'a Config) -> Self { - Self { config } + pub fn new(output: &Output, config: &'a Config) -> Self { + let mut mainthreadonly_classes = BTreeSet::new(); + + for library in output.libraries.values() { + for file in library.files.values() { + for stmt in file.stmts.iter() { + if let Stmt::ClassDecl { + id, + mutability: Mutability::MainThreadOnly, + .. + } = stmt + { + mainthreadonly_classes.insert(id.clone()); + } + } + } + } + + Self { + config, + mainthreadonly_classes, + } } pub fn update(&self, output: &mut Output) { @@ -68,6 +90,67 @@ impl<'a> Cache<'a> { } } + // Add `mainthreadonly` to relevant methods + for stmt in file.stmts.iter_mut() { + match stmt { + Stmt::Methods { + cls: id, methods, .. + } + | Stmt::ProtocolDecl { id, methods, .. } => { + for method in methods.iter_mut() { + let mut result_type_contains_mainthreadonly: bool = false; + method.result_type.visit_required_types(&mut |id| { + if self.mainthreadonly_classes.contains(id) { + result_type_contains_mainthreadonly = true; + } + }); + + match (method.is_class, self.mainthreadonly_classes.contains(id)) { + // MainThreadOnly class with static method + (true, true) => { + // Assume the method needs main thread + result_type_contains_mainthreadonly = true; + } + // Class with static method + (true, false) => { + // Continue with the normal check + } + // MainThreadOnly class with non-static method + (false, true) => { + // Method is already required to run on main + // thread, so no need to add MainThreadMarker + continue; + } + // Class with non-static method + (false, false) => { + // Continue with the normal check + } + } + + if result_type_contains_mainthreadonly { + let mut any_argument_contains_mainthreadonly: bool = false; + for (_, argument) in method.arguments.iter() { + // Important: We only visit the top-level types, to not + // include e.g. `Option<&NSView>` or `&NSArray`. + argument.visit_toplevel_types(&mut |id| { + if self.mainthreadonly_classes.contains(id) { + any_argument_contains_mainthreadonly = true; + } + }); + } + + // Apply main thread only, unless a (required) + // argument was main thread only. + if !any_argument_contains_mainthreadonly { + method.mainthreadonly = true; + } + } + } + } + _ => {} + } + } + // Fix up a few typedef + enum declarations let mut iter = mem::take(&mut file.stmts).into_iter().peekable(); while let Some(stmt) = iter.next() { diff --git a/crates/header-translator/src/method.rs b/crates/header-translator/src/method.rs index 7cda71d9f..5435b7c8d 100644 --- a/crates/header-translator/src/method.rs +++ b/crates/header-translator/src/method.rs @@ -249,14 +249,14 @@ pub struct Method { pub is_class: bool, is_optional_protocol: bool, memory_management: MemoryManagement, - arguments: Vec<(String, Ty)>, + pub(crate) arguments: Vec<(String, Ty)>, pub result_type: Ty, safe: bool, mutating: bool, is_protocol: bool, // Thread-safe, even on main-thread only (@MainActor/@UIActor) classes non_isolated: bool, - mainthreadonly: bool, + pub(crate) mainthreadonly: bool, } impl Method { @@ -636,6 +636,11 @@ impl fmt::Display for Method { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { let _span = debug_span!("method", self.fn_name).entered(); + // TODO: Use this somehow? + // if self.non_isolated { + // writeln!(f, "// non_isolated")?; + // } + // // Attributes // @@ -689,7 +694,11 @@ impl fmt::Display for Method { // Arguments for (param, arg_ty) in &self.arguments { let param = handle_reserved(&crate::to_snake_case(param)); - write!(f, "{param}: {arg_ty},")?; + write!(f, "{param}: {arg_ty}, ")?; + } + // FIXME: Skipping main thread only on protocols for now + if self.mainthreadonly && !self.is_protocol { + write!(f, "mtm: MainThreadMarker")?; } write!(f, ")")?; diff --git a/crates/header-translator/src/rust_type.rs b/crates/header-translator/src/rust_type.rs index 3534246ab..6099052d2 100644 --- a/crates/header-translator/src/rust_type.rs +++ b/crates/header-translator/src/rust_type.rs @@ -345,6 +345,12 @@ impl IdType { } } } + + fn visit_toplevel_types(&self, f: &mut impl FnMut(&ItemIdentifier)) { + if let Some(id) = self._id() { + f(id); + } + } } impl fmt::Display for IdType { @@ -1036,6 +1042,25 @@ impl Inner { _ => {} } } + + pub fn visit_toplevel_types(&self, f: &mut impl FnMut(&ItemIdentifier)) { + match self { + Self::Id { ty, .. } => { + ty.visit_toplevel_types(f); + } + Self::Pointer { + // Only visit non-null types + nullability: Nullability::NonNull, + is_const: _, + pointee, + } => { + pointee.visit_toplevel_types(f); + } + // TODO + Self::TypeDef { id } => f(id), + _ => {} + } + } } /// This is sound to output in (almost, c_void is not a valid return type) any @@ -1492,6 +1517,14 @@ impl Ty { self.ty.visit_required_types(f); } + + pub fn visit_toplevel_types(&self, f: &mut impl FnMut(&ItemIdentifier)) { + if let TyKind::MethodReturn { with_error: true } = &self.kind { + f(&ItemIdentifier::nserror()); + } + + self.ty.visit_toplevel_types(f); + } } impl Ty { diff --git a/crates/icrate/Cargo.toml b/crates/icrate/Cargo.toml index b9d9aa0b4..92f527d2c 100644 --- a/crates/icrate/Cargo.toml +++ b/crates/icrate/Cargo.toml @@ -131,8 +131,9 @@ unstable-example-basic_usage = [ unstable-example-delegate = [ "apple", "Foundation", - "Foundation_NSString", "Foundation_NSNotification", + "Foundation_NSString", + "Foundation_NSThread", "AppKit", "AppKit_NSApplication", ] @@ -165,6 +166,7 @@ unstable-example-browser = [ "Foundation", "Foundation_NSNotification", "Foundation_NSString", + "Foundation_NSThread", "Foundation_NSURL", "Foundation_NSURLRequest", "WebKit", @@ -177,10 +179,11 @@ unstable-example-metal = [ "AppKit_NSWindow", "Foundation", "Foundation_NSCoder", + "Foundation_NSDate", "Foundation_NSError", "Foundation_NSNotification", "Foundation_NSString", - "Foundation_NSDate", + "Foundation_NSThread", "Metal", "Metal_MTLCompileOptions", "Metal_MTLRenderPassDescriptor", diff --git a/crates/icrate/examples/browser.rs b/crates/icrate/examples/browser.rs index da51816fa..37c256ddf 100644 --- a/crates/icrate/examples/browser.rs +++ b/crates/icrate/examples/browser.rs @@ -12,15 +12,15 @@ use icrate::{ NSWindowStyleMaskResizable, NSWindowStyleMaskTitled, }, Foundation::{ - ns_string, NSNotification, NSObject, NSObjectProtocol, NSPoint, NSRect, NSSize, - NSURLRequest, NSURL, + ns_string, MainThreadMarker, NSNotification, NSObject, NSObjectProtocol, NSPoint, NSRect, + NSSize, NSURLRequest, NSURL, }, WebKit::{WKNavigation, WKNavigationDelegate, WKWebView}, }; use objc2::{ declare::{Ivar, IvarDrop}, declare_class, msg_send, msg_send_id, - mutability::InteriorMutable, + mutability::MainThreadOnly, rc::Id, runtime::{AnyObject, ProtocolObject, Sel}, sel, ClassType, @@ -47,7 +47,7 @@ declare_class!( unsafe impl ClassType for Delegate { type Super = NSObject; - type Mutability = InteriorMutable; + type Mutability = MainThreadOnly; const NAME: &'static str = "Delegate"; } @@ -68,9 +68,9 @@ declare_class!( #[method(applicationDidFinishLaunching:)] #[allow(non_snake_case)] unsafe fn applicationDidFinishLaunching(&self, _notification: &NSNotification) { + let mtm = MainThreadMarker::from(self); // create the app window let window = { - let this = NSWindow::alloc(); let content_rect = NSRect::new(NSPoint::new(0., 0.), NSSize::new(1024., 768.)); let style = NSWindowStyleMaskClosable | NSWindowStyleMaskResizable @@ -79,7 +79,7 @@ declare_class!( let flag = false; unsafe { NSWindow::initWithContentRect_styleMask_backing_defer( - this, + mtm.alloc(), content_rect, style, backing_store_type, @@ -90,16 +90,14 @@ declare_class!( // create the web view let web_view = { - let this = WKWebView::alloc(); let frame_rect = NSRect::ZERO; - unsafe { WKWebView::initWithFrame(this, frame_rect) } + unsafe { WKWebView::initWithFrame(mtm.alloc(), frame_rect) } }; // create the nav bar view let nav_bar = { let frame_rect = NSRect::ZERO; - let this = NSStackView::alloc(); - let this = unsafe { NSStackView::initWithFrame(this, frame_rect) }; + let this = unsafe { NSStackView::initWithFrame(mtm.alloc(), frame_rect) }; unsafe { this.setOrientation(NSUserInterfaceLayoutOrientationHorizontal); this.setAlignment(NSLayoutAttributeHeight); @@ -112,8 +110,7 @@ declare_class!( // create the nav buttons view let nav_buttons = { let frame_rect = NSRect::ZERO; - let this = NSStackView::alloc(); - let this = unsafe { NSStackView::initWithFrame(this, frame_rect) }; + let this = unsafe { NSStackView::initWithFrame(mtm.alloc(), frame_rect) }; unsafe { this.setOrientation(NSUserInterfaceLayoutOrientationHorizontal); this.setAlignment(NSLayoutAttributeHeight); @@ -130,7 +127,7 @@ declare_class!( let target = Some::<&AnyObject>(&web_view); let action = Some(sel!(goBack)); let this = - unsafe { NSButton::buttonWithTitle_target_action(title, target, action) }; + unsafe { NSButton::buttonWithTitle_target_action(title, target, action, mtm) }; unsafe { this.setBezelStyle(NSBezelStyleShadowlessSquare) }; this }; @@ -142,7 +139,7 @@ declare_class!( let target = Some::<&AnyObject>(&web_view); let action = Some(sel!(goForward)); let this = - unsafe { NSButton::buttonWithTitle_target_action(title, target, action) }; + unsafe { NSButton::buttonWithTitle_target_action(title, target, action, mtm) }; unsafe { this.setBezelStyle(NSBezelStyleShadowlessSquare) }; this }; @@ -155,8 +152,7 @@ declare_class!( // create the url text field let nav_url = { let frame_rect = NSRect::ZERO; - let this = NSTextField::alloc(); - let this = unsafe { NSTextField::initWithFrame(this, frame_rect) }; + let this = unsafe { NSTextField::initWithFrame(mtm.alloc(), frame_rect) }; unsafe { this.setDrawsBackground(true); this.setBackgroundColor(Some(&NSColor::lightGrayColor())); @@ -173,8 +169,7 @@ declare_class!( // create the window content view let content_view = { let frame_rect = unsafe { window.frame() }; - let this = NSStackView::alloc(); - let this = unsafe { NSStackView::initWithFrame(this, frame_rect) }; + let this = unsafe { NSStackView::initWithFrame(mtm.alloc(), frame_rect) }; unsafe { this.setOrientation(NSUserInterfaceLayoutOrientationVertical); this.setAlignment(NSLayoutAttributeWidth); @@ -217,7 +212,7 @@ declare_class!( menu_app_item.setSubmenu(Some(&menu_app_menu)); menu.addItem(&menu_app_item); - let app = NSApplication::sharedApplication(); + let app = NSApplication::sharedApplication(mtm); app.setMainMenu(Some(&menu)); } @@ -286,19 +281,20 @@ declare_class!( ); impl Delegate { - pub fn new() -> Id { - unsafe { msg_send_id![Self::alloc(), init] } + pub fn new(mtm: MainThreadMarker) -> Id { + unsafe { msg_send_id![mtm.alloc(), init] } } } unsafe impl NSObjectProtocol for Delegate {} fn main() { - let app = unsafe { NSApplication::sharedApplication() }; + let mtm = MainThreadMarker::new().unwrap(); + let app = unsafe { NSApplication::sharedApplication(mtm) }; unsafe { app.setActivationPolicy(NSApplicationActivationPolicyRegular) }; // initialize the delegate - let delegate = Delegate::new(); + let delegate = Delegate::new(mtm); // configure the application delegate unsafe { diff --git a/crates/icrate/examples/delegate.rs b/crates/icrate/examples/delegate.rs index 5615e7f13..bb954fb0d 100644 --- a/crates/icrate/examples/delegate.rs +++ b/crates/icrate/examples/delegate.rs @@ -3,7 +3,7 @@ use std::ptr::NonNull; use icrate::AppKit::{NSApplication, NSApplicationActivationPolicyRegular, NSApplicationDelegate}; use icrate::Foundation::{ - ns_string, NSCopying, NSNotification, NSObject, NSObjectProtocol, NSString, + ns_string, MainThreadMarker, NSCopying, NSNotification, NSObject, NSObjectProtocol, NSString, }; use objc2::declare::{Ivar, IvarBool, IvarDrop, IvarEncode}; use objc2::rc::Id; @@ -68,17 +68,19 @@ declare_class!( unsafe impl NSObjectProtocol for AppDelegate {} impl AppDelegate { - pub fn new(ivar: u8, another_ivar: bool) -> Id { - unsafe { msg_send_id![Self::alloc(), initWith: ivar, another: another_ivar] } + pub fn new(ivar: u8, another_ivar: bool, mtm: MainThreadMarker) -> Id { + unsafe { msg_send_id![mtm.alloc(), initWith: ivar, another: another_ivar] } } } fn main() { - let app = unsafe { NSApplication::sharedApplication() }; + let mtm: MainThreadMarker = MainThreadMarker::new().unwrap(); + + let app = unsafe { NSApplication::sharedApplication(mtm) }; unsafe { app.setActivationPolicy(NSApplicationActivationPolicyRegular) }; // initialize the delegate - let delegate = AppDelegate::new(42, true); + let delegate = AppDelegate::new(42, true, mtm); println!("{delegate:?}"); diff --git a/crates/icrate/examples/metal.rs b/crates/icrate/examples/metal.rs index c452a9ec0..8c64ae2cd 100644 --- a/crates/icrate/examples/metal.rs +++ b/crates/icrate/examples/metal.rs @@ -9,7 +9,8 @@ use icrate::{ NSWindowStyleMaskTitled, }, Foundation::{ - ns_string, NSDate, NSNotification, NSObject, NSObjectProtocol, NSPoint, NSRect, NSSize, + ns_string, MainThreadMarker, NSDate, NSNotification, NSObject, NSObjectProtocol, NSPoint, + NSRect, NSSize, }, Metal::{ MTLCommandBuffer, MTLCommandEncoder, MTLCommandQueue, MTLCreateSystemDefaultDevice, @@ -21,7 +22,7 @@ use icrate::{ use objc2::{ declare::{Ivar, IvarDrop}, declare_class, msg_send, msg_send_id, - mutability::InteriorMutable, + mutability::MainThreadOnly, rc::Id, runtime::ProtocolObject, ClassType, @@ -126,7 +127,7 @@ declare_class!( // declare the class type unsafe impl ClassType for Delegate { type Super = NSObject; - type Mutability = InteriorMutable; + type Mutability = MainThreadOnly; const NAME: &'static str = "Delegate"; } @@ -150,9 +151,9 @@ declare_class!( #[method(applicationDidFinishLaunching:)] #[allow(non_snake_case)] unsafe fn applicationDidFinishLaunching(&self, _notification: &NSNotification) { + let mtm = MainThreadMarker::from(self); // create the app window let window = { - let this = NSWindow::alloc(); let content_rect = NSRect::new(NSPoint::new(0., 0.), NSSize::new(768., 768.)); let style = NSWindowStyleMaskClosable | NSWindowStyleMaskResizable @@ -161,7 +162,7 @@ declare_class!( let flag = false; unsafe { NSWindow::initWithContentRect_styleMask_backing_defer( - this, + mtm.alloc(), content_rect, style, backing_store_type, @@ -183,9 +184,8 @@ declare_class!( // create the metal view let mtk_view = { - let this = MTKView::alloc(); let frame_rect = unsafe { window.frame() }; - unsafe { MTKView::initWithFrame_device(this, frame_rect, Some(&device)) } + unsafe { MTKView::initWithFrame_device(mtm.alloc(), frame_rect, Some(&device)) } }; // create the pipeline descriptor @@ -352,18 +352,19 @@ declare_class!( unsafe impl NSObjectProtocol for Delegate {} impl Delegate { - pub fn new() -> Id { - unsafe { msg_send_id![Self::alloc(), init] } + pub fn new(mtm: MainThreadMarker) -> Id { + unsafe { msg_send_id![mtm.alloc(), init] } } } fn main() { + let mtm = MainThreadMarker::new().unwrap(); // configure the app - let app = unsafe { NSApplication::sharedApplication() }; + let app = unsafe { NSApplication::sharedApplication(mtm) }; unsafe { app.setActivationPolicy(NSApplicationActivationPolicyRegular) }; // initialize the delegate - let delegate = Delegate::new(); + let delegate = Delegate::new(mtm); // configure the application delegate unsafe { diff --git a/crates/icrate/src/additions/Foundation/mod.rs b/crates/icrate/src/additions/Foundation/mod.rs index 568be789e..76c06be22 100644 --- a/crates/icrate/src/additions/Foundation/mod.rs +++ b/crates/icrate/src/additions/Foundation/mod.rs @@ -32,8 +32,9 @@ pub use self::range::NSRange; #[cfg(feature = "Foundation_NSThread")] #[cfg(feature = "dispatch")] pub use self::thread::MainThreadBound; +pub use self::thread::MainThreadMarker; #[cfg(feature = "Foundation_NSThread")] -pub use self::thread::{is_main_thread, is_multi_threaded, MainThreadMarker}; +pub use self::thread::{is_main_thread, is_multi_threaded}; #[cfg(feature = "Foundation_NSString")] #[doc(inline)] diff --git a/crates/icrate/src/additions/Foundation/thread.rs b/crates/icrate/src/additions/Foundation/thread.rs index df4d19fa4..cea25d217 100644 --- a/crates/icrate/src/additions/Foundation/thread.rs +++ b/crates/icrate/src/additions/Foundation/thread.rs @@ -1,19 +1,23 @@ -#![cfg(feature = "Foundation_NSThread")] use core::fmt; use core::marker::PhantomData; use core::mem::{self, ManuallyDrop}; use core::panic::{RefUnwindSafe, UnwindSafe}; use crate::common::*; +#[cfg(feature = "Foundation_NSThread")] use crate::Foundation::NSThread; use objc2::msg_send_id; use objc2::mutability::IsMainThreadOnly; +#[cfg(feature = "Foundation_NSThread")] unsafe impl Send for NSThread {} +#[cfg(feature = "Foundation_NSThread")] unsafe impl Sync for NSThread {} +#[cfg(feature = "Foundation_NSThread")] impl UnwindSafe for NSThread {} +#[cfg(feature = "Foundation_NSThread")] impl RefUnwindSafe for NSThread {} /// Whether the application is multithreaded according to Cocoa. @@ -68,7 +72,15 @@ fn make_multithreaded() { /// } /// /// // Usage +/// +/// // Create a new marker. This requires the `"Foundation_NSThread"` feature. +/// // If that is not available, create the marker unsafely with +/// // `new_unchecked`, after having checked that the thread is the main one +/// // through other means. +/// #[cfg(feature = "Foundation_NSThread")] /// let mtm = MainThreadMarker::new().expect("must be on the main thread"); +/// #[cfg(not(feature = "Foundation_NSThread"))] +/// let mtm = unsafe { MainThreadMarker::new_unchecked() }; /// unsafe { do_thing(obj, mtm) } /// ``` #[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)] diff --git a/crates/icrate/src/generated b/crates/icrate/src/generated index 7205d0e7b..24a19eb14 160000 --- a/crates/icrate/src/generated +++ b/crates/icrate/src/generated @@ -1 +1 @@ -Subproject commit 7205d0e7bd68d94c0b6a4effddc7b746df7eae52 +Subproject commit 24a19eb1494a964312ba5d26ebbffb05436973a5 diff --git a/crates/objc2/src/macros/__method_msg_send.rs b/crates/objc2/src/macros/__method_msg_send.rs index be56c91b9..0ae25bfcf 100644 --- a/crates/objc2/src/macros/__method_msg_send.rs +++ b/crates/objc2/src/macros/__method_msg_send.rs @@ -162,12 +162,13 @@ macro_rules! __method_msg_send_id { () () - () + // Possible to hit via. the MainThreadMarker branch + ($($already_parsed_retain_semantics:ident)?) ) => { $crate::__msg_send_id_helper! { @(send_message_id) @($receiver) - @($($retain_semantics)?) + @($($retain_semantics)? $($already_parsed_retain_semantics)?) @($sel) @() } diff --git a/crates/objc2/tests/macros_mainthreadmarker.rs b/crates/objc2/tests/macros_mainthreadmarker.rs index 546079dd0..dff31e4c4 100644 --- a/crates/objc2/tests/macros_mainthreadmarker.rs +++ b/crates/objc2/tests/macros_mainthreadmarker.rs @@ -51,7 +51,7 @@ struct MainThreadMarker(bool); extern_methods!( unsafe impl Cls { #[method_id(new)] - fn new() -> Id; + fn new(mtm: MainThreadMarker) -> Id; #[method(myMethod:)] fn method(mtm: MainThreadMarker, arg: i32, mtm2: MainThreadMarker) -> i32; @@ -63,8 +63,8 @@ extern_methods!( #[test] fn call() { - let obj1 = Cls::new(); let mtm = MainThreadMarker(true); + let obj1 = Cls::new(mtm); let res = Cls::method(mtm, 2, mtm); assert_eq!(res, 3);