Skip to content

Commit

Permalink
Remove header-translator's ability to emit mutable classes/methods
Browse files Browse the repository at this point in the history 
Part of #563
  • Loading branch information
@madsmtm
madsmtm committed Sep 6, 2024
1 parent 4a3d690 commit 7853457
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 103 deletions.
23 changes: 0 additions & 23 deletions crates/header-translator/src/config.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,6 @@ pub struct MethodData {
pub unsafe_: bool,
#[serde(default = "skipped_default")]
pub skipped: bool,
pub mutating: Option<bool>,
}

impl MethodData {
Expand All @@ -254,7 +253,6 @@ impl MethodData {
// Only use `unsafe` from itself, never take if from the superclass
unsafe_: self.unsafe_,
skipped: self.skipped | superclass.skipped,
mutating: self.mutating.or(superclass.mutating),
}
}
}
Expand Down Expand Up @@ -295,7 +293,6 @@ impl Default for MethodData {
Self {
unsafe_: unsafe_default(),
skipped: skipped_default(),
mutating: None,
}
}
}
Expand Down Expand Up @@ -348,24 +345,6 @@ impl<'de> Deserialize<'de> for Mutability {
where
E: de::Error,
{
if let Some(value) = value.strip_prefix("ImmutableWithMutableSubclass(") {
let value = value
.strip_suffix(')')
.ok_or(de::Error::custom("end parenthesis"))?;
let item =
parse_itemidentifier(value).ok_or(de::Error::custom("requires ::"))?;
return Ok(Mutability::ImmutableWithMutableSubclass(item));
}

if let Some(value) = value.strip_prefix("MutableWithImmutableSuperclass(") {
let value = value
.strip_suffix(')')
.ok_or(de::Error::custom("end parenthesis"))?;
let item =
parse_itemidentifier(value).ok_or(de::Error::custom("requires ::"))?;
return Ok(Mutability::MutableWithImmutableSuperclass(item));
}

if let Some(value) = value.strip_prefix("InteriorMutableWithSubclass(") {
let value = value
.strip_suffix(')')
Expand All @@ -385,8 +364,6 @@ impl<'de> Deserialize<'de> for Mutability {
}

match value {
"Immutable" => Ok(Mutability::Immutable),
"Mutable" => Ok(Mutability::Mutable),
"InteriorMutable" => Ok(Mutability::InteriorMutable),
"MainThreadOnly" => Ok(Mutability::MainThreadOnly),
value => Err(de::Error::custom(format!("unknown variant {value:?}"))),
Expand Down
14 changes: 0 additions & 14 deletions crates/header-translator/src/method.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,6 @@ pub struct Method {
result_type: Ty,
is_error: bool,
safe: bool,
mutating: bool,
is_pub: bool,
// Thread-safe, even on main-thread only (@MainActor/@UIActor) classes
non_isolated: bool,
Expand Down Expand Up @@ -354,7 +353,6 @@ impl Method {
pub(crate) fn parse_method(
entity: Entity<'_>,
data: MethodData,
parent_is_mutable: bool,
parent_is_mainthreadonly: bool,
is_pub: bool,
context: &Context<'_>,
Expand Down Expand Up @@ -504,10 +502,6 @@ impl Method {
result_type,
is_error,
safe: !data.unsafe_,
// Mutable if the parent is mutable is a reasonable default,
// since immutable methods are usually either declared on an
// immutable subclass, or as a property.
mutating: data.mutating.unwrap_or(parent_is_mutable),
is_pub,
non_isolated: modifiers.non_isolated,
mainthreadonly,
Expand All @@ -520,7 +514,6 @@ impl Method {
property: PartialProperty<'_>,
getter_data: MethodData,
setter_data: Option<MethodData>,
parent_is_mutable: bool,
parent_is_mainthreadonly: bool,
is_pub: bool,
context: &Context<'_>,
Expand Down Expand Up @@ -581,9 +574,6 @@ impl Method {
result_type: ty,
is_error: false,
safe: !getter_data.unsafe_,
// Getters are usually not mutable, even if the class itself
// is, so let's default to immutable.
mutating: getter_data.mutating.unwrap_or(false),
is_pub,
non_isolated: modifiers.non_isolated,
mainthreadonly,
Expand Down Expand Up @@ -626,8 +616,6 @@ impl Method {
result_type,
is_error: false,
safe: !setter_data.unsafe_,
// Setters are usually mutable if the class itself is.
mutating: setter_data.mutating.unwrap_or(parent_is_mutable),
is_pub,
non_isolated: modifiers.non_isolated,
mainthreadonly,
Expand Down Expand Up @@ -722,8 +710,6 @@ impl fmt::Display for Method {
write!(f, "this: Allocated<Self>, ")?;
} else if self.is_class {
// Insert nothing; a class method is assumed
} else if self.mutating {
write!(f, "&mut self, ")?;
} else {
write!(f, "&self, ")?;
}
Expand Down
53 changes: 3 additions & 50 deletions crates/header-translator/src/stmt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,6 @@ pub(crate) fn method_or_property_entities<'tu>(
fn parse_methods(
entity: &Entity<'_>,
get_data: impl Fn(&str) -> MethodData,
is_mutable: bool,
thread_safety: &ThreadSafety,
is_pub: bool,
context: &Context<'_>,
Expand All @@ -235,7 +234,6 @@ fn parse_methods(
if let Some((designated_initializer, method)) = Method::parse_method(
entity,
data,
is_mutable,
thread_safety.inferred_mainthreadonly(),
is_pub,
context,
Expand All @@ -262,7 +260,6 @@ fn parse_methods(
partial,
getter_data,
setter_data,
is_mutable,
thread_safety.inferred_mainthreadonly(),
is_pub,
context,
Expand Down Expand Up @@ -298,10 +295,8 @@ pub(crate) fn items_required_by_decl(
for (superclass, _, _) in parse_superclasses(entity, context) {
items.push(superclass);
}
if let Some(
Mutability::ImmutableWithMutableSubclass(subclass)
| Mutability::InteriorMutableWithSubclass(subclass),
) = data.map(|data| &data.mutability)
if let Some(Mutability::InteriorMutableWithSubclass(subclass)) =
data.map(|data| &data.mutability)
{
items.push(subclass.clone());
}
Expand Down Expand Up @@ -388,10 +383,6 @@ fn verify_objc_decl(entity: &Entity<'_>, _context: &Context<'_>) {

#[derive(Debug, Clone, PartialEq, Eq, Hash, Default)]
pub enum Mutability {
Immutable,
Mutable,
ImmutableWithMutableSubclass(ItemIdentifier),
MutableWithImmutableSuperclass(ItemIdentifier),
#[default]
InteriorMutable,
InteriorMutableWithSubclass(ItemIdentifier),
Expand All @@ -400,33 +391,8 @@ pub enum Mutability {
}

impl Mutability {
pub fn is_mutable(&self) -> bool {
matches!(
self,
Mutability::Mutable | Mutability::MutableWithImmutableSuperclass(_)
)
}

fn display<'a>(&'a self, other_generics: impl Display + 'a) -> impl Display + 'a {
FormatterFn(move |f| match self {
Self::Immutable => write!(f, "Immutable"),
Self::Mutable => write!(f, "Mutable"),
Self::ImmutableWithMutableSubclass(subclass) => {
write!(
f,
"ImmutableWithMutableSubclass<{}{}>",
subclass.path(),
other_generics
)
}
Self::MutableWithImmutableSuperclass(superclass) => {
write!(
f,
"MutableWithImmutableSuperclass<{}{}>",
superclass.path(),
other_generics
)
}
Self::InteriorMutable => write!(f, "InteriorMutable"),
Self::InteriorMutableWithSubclass(subclass) => {
write!(
Expand Down Expand Up @@ -679,8 +645,6 @@ impl Stmt {
let (methods, designated_initializers) = parse_methods(
entity,
|name| ClassData::get_method_data(data, name),
data.map(|data| data.mutability.is_mutable())
.unwrap_or(false),
&thread_safety,
true,
context,
Expand Down Expand Up @@ -724,9 +688,6 @@ impl Stmt {
ClassData::get_method_data(superclass_data, name);
data.merge_with_superclass(superclass_data)
},
data.map(|data| data.mutability.is_mutable())
.or(superclass_data.map(|data| data.mutability.is_mutable()))
.unwrap_or(false),
&thread_safety,
true,
context,
Expand Down Expand Up @@ -876,8 +837,6 @@ impl Stmt {
let (methods, designated_initializers) = parse_methods(
entity,
|name| ClassData::get_method_data(data, name),
data.map(|data| data.mutability.is_mutable())
.unwrap_or(false),
&cls_thread_safety,
true,
context,
Expand All @@ -890,8 +849,7 @@ impl Stmt {
);
}

let extra_methods = if let Mutability::ImmutableWithMutableSubclass(subclass)
| Mutability::InteriorMutableWithSubclass(subclass) =
let extra_methods = if let Mutability::InteriorMutableWithSubclass(subclass) =
data.map(|data| data.mutability.clone()).unwrap_or_default()
{
let subclass_data = context
Expand All @@ -907,9 +865,6 @@ impl Stmt {
let subclass_data = ClassData::get_method_data(subclass_data, name);
subclass_data.merge_with_superclass(data)
},
data.map(|data| data.mutability.is_mutable())
.or(subclass_data.map(|data| data.mutability.is_mutable()))
.unwrap_or(false),
&cls_thread_safety,
true,
context,
Expand Down Expand Up @@ -982,7 +937,6 @@ impl Stmt {
let (methods, designated_initializers) = parse_methods(
entity,
|name| ClassData::get_method_data(data, name),
false,
&cls_thread_safety,
false,
context,
Expand Down Expand Up @@ -1057,7 +1011,6 @@ impl Stmt {
.copied()
.unwrap_or_default()
},
false,
&thread_safety,
false,
context,
Expand Down
2 changes: 1 addition & 1 deletion framework-crates/objc2-app-kit/translation-config.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ class.NSView.mutability = "MainThreadOnly"
# in that case, they can't be!
class.NSWindow.mutability = "MainThreadOnly"

# TODO: This should be one of MainThreadOnly or Immutable (+Send/Sync)
# TODO: This should probably be MainThreadOnly, or maybe +Send/Sync?
# class.NSAppearance.mutability = "MainThreadOnly"

# Documented Thread-Unsafe, but:
Expand Down
20 changes: 10 additions & 10 deletions framework-crates/objc2-foundation/src/tests/auto_traits.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@ fn assert_auto_traits<T: Send + Sync + UnwindSafe + RefUnwindSafe>() {
}

declare_class!(
struct ImmutableSendSyncObject;
struct SendSyncObject;

unsafe impl ClassType for ImmutableSendSyncObject {
unsafe impl ClassType for SendSyncObject {
type Super = NSObject;
type Mutability = mutability::Immutable;
const NAME: &'static str = "ImmutableSendSyncObject";
type Mutability = mutability::InteriorMutable;
const NAME: &'static str = "SendSyncObject";
}

impl DeclaredClass for ImmutableSendSyncObject {}
impl DeclaredClass for SendSyncObject {}
);

unsafe impl Send for ImmutableSendSyncObject {}
unsafe impl Sync for ImmutableSendSyncObject {}
unsafe impl Send for SendSyncObject {}
unsafe impl Sync for SendSyncObject {}

#[test]
fn test_generic_auto_traits() {
Expand All @@ -60,9 +60,9 @@ fn test_generic_auto_traits() {

// Collections are not Send + Sync, since they are interior mutable, i.e.
// mutable from `&self`.
assert_not_impl_any!(NSArray<ImmutableSendSyncObject>: Send, Sync);
assert_not_impl_any!(NSMutableArray<ImmutableSendSyncObject>: Send, Sync);
assert_not_impl_any!(NSDictionary<ImmutableSendSyncObject, ImmutableSendSyncObject>: Send, Sync);
assert_not_impl_any!(NSArray<SendSyncObject>: Send, Sync);
assert_not_impl_any!(NSMutableArray<SendSyncObject>: Send, Sync);
assert_not_impl_any!(NSDictionary<SendSyncObject, SendSyncObject>: Send, Sync);

// TODO: Make these UnwindSafe?
assert_not_impl_any!(NSDictionary<NSProcessInfo, NSProcessInfo>: UnwindSafe, RefUnwindSafe);
Expand Down
5 changes: 0 additions & 5 deletions framework-crates/objc2-quartz-core/translation-config.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,6 @@ class.CAEAGLLayer.skipped-protocols = ["EAGLDrawable"]
### Safety
###

# We probably technically had the choice of making these classes mutating
# methods require `&mut`, but as with so many things in Cocoa, that would
# make it difficult to use in a larger context (e.g. even after assigning a
# layer to a view you'd often still want to modify the layer).

fn.CACurrentMediaTime.unsafe = false

# SAFETY: Basic mathematical functions
Expand Down

0 comments on commit 7853457

Please sign in to comment.