Skip to content

Commit

Permalink
Handle CF_RETURNS_[NOT_]RETAINED in methods
Browse files Browse the repository at this point in the history
  • Loading branch information
madsmtm committed Jan 22, 2025
1 parent c8cc5bd commit 5dc0697
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 57 deletions.
6 changes: 3 additions & 3 deletions crates/header-translator/src/id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ impl ItemTree {
}
_ => {
// Assume that the library handles this itself internally.
debug!(
trace!(
?libraries,
?self,
"nested required_crate_features enablement"
Expand Down Expand Up @@ -863,7 +863,7 @@ impl ItemTree {
}
_ => {
// Assume that the library handles this itself internally.
debug!(?libraries, ?self, "nested enabled_features enablement");
trace!(?libraries, ?self, "nested enabled_features enablement");
}
});
enabled_features.into_iter()
Expand Down Expand Up @@ -918,7 +918,7 @@ impl ItemTree {
}
_ => {
// Assume that the library handles this itself internally.
debug!(?libraries, ?self, "nested cfg_features enablement");
trace!(?libraries, ?self, "nested cfg_features enablement");
}
});
feature_names.into_iter()
Expand Down
126 changes: 87 additions & 39 deletions crates/header-translator/src/method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,14 +147,12 @@ impl MethodModifiers {
/// This also encodes the "method family" that a method belongs to.
#[derive(Debug, PartialEq, Eq, Hash, Clone, Copy)]
pub enum MemoryManagement {
IdCopy,
IdMutableCopy,
IdNew,
IdInit,
IdOther,
// TODO:
// IdReturnsRetained,
// IdReturnsNotRetained,
RetainedAlloc,
RetainedCopy { returns_not_retained: bool },
RetainedMutableCopy { returns_not_retained: bool },
RetainedNew { returns_not_retained: bool },
RetainedInit,
RetainedNone { returns_retained: bool },
Normal,
}

Expand All @@ -169,7 +167,7 @@ impl MemoryManagement {
// If:
// > its selector falls into the corresponding selector family
let bytes = selector.as_bytes();
let id_type = match (
let selector_family = match (
in_selector_family(bytes, b"alloc"),
in_selector_family(bytes, b"copy"),
in_selector_family(bytes, b"mutableCopy"),
Expand All @@ -181,13 +179,13 @@ impl MemoryManagement {
// they're only defined on `NSObject` and `NSProxy`, and we
// have it in `ClassType` anyhow.
error!("the `alloc` method-family requires manual handling");
Self::IdOther
"alloc"
}
(false, true, false, false, false) => Self::IdCopy,
(false, false, true, false, false) => Self::IdMutableCopy,
(false, false, false, true, false) => Self::IdNew,
(false, false, false, false, true) => Self::IdInit,
(false, false, false, false, false) => Self::IdOther,
(false, true, false, false, false) => "copy",
(false, false, true, false, false) => "mutableCopy",
(false, false, false, true, false) => "new",
(false, false, false, false, true) => "init",
(false, false, false, false, false) => "none",
_ => unreachable!(),
};

Expand All @@ -205,11 +203,27 @@ impl MemoryManagement {
modifiers.returns_retained,
modifiers.returns_not_retained,
modifiers.designated_initializer,
id_type,
selector_family,
) {
(false, false, true, false, false, Self::IdCopy) => Self::IdCopy,
(false, false, true, false, false, Self::IdMutableCopy) => Self::IdMutableCopy,
(false, false, true, false, false, Self::IdNew) => Self::IdNew,
(false, false, true, false, false, "alloc") => Self::RetainedAlloc,
(false, false, true, false, false, "copy") => Self::RetainedCopy {
returns_not_retained: false,
},
(false, false, false, true, false, "copy") => Self::RetainedCopy {
returns_not_retained: true,
},
(false, false, true, false, false, "mutableCopy") => Self::RetainedMutableCopy {
returns_not_retained: false,
},
(false, false, false, true, false, "mutableCopy") => Self::RetainedMutableCopy {
returns_not_retained: true,
},
(false, false, true, false, false, "new") => Self::RetainedNew {
returns_not_retained: false,
},
(false, false, false, true, false, "new") => Self::RetainedNew {
returns_not_retained: true,
},
// For the `init` family there's another restriction:
// > must be instance methods
//
Expand All @@ -218,17 +232,27 @@ impl MemoryManagement {
// methods that are not correct super/subclasses, but we don't
// need to handle that since the header would fail to compile
// in `clang` if that was the case.
(false, true, true, false, _, Self::IdInit) => {
(false, true, true, false, _, "init") => {
if is_class {
Self::IdOther
// TODO: Is this actually correct, or should we still
// emit #[unsafe(method_family = init)] here?
Self::RetainedNone {
returns_retained: false,
}
} else {
Self::IdInit
Self::RetainedInit
}
}
(false, false, false, false, false, Self::IdOther) => Self::IdOther,
// Don't care if the user specified CF_RETURNS_NOT_RETAINED,
// that is the default for this selector anyhow.
(false, false, returns_retained, _, false, "none") => {
Self::RetainedNone { returns_retained }
}
data => {
error!(?data, "invalid MemoryManagement id attributes");
Self::IdOther
error!(?data, "invalid MemoryManagement retainable attributes");
Self::RetainedNone {
returns_retained: false,
}
}
}
} else if let MethodModifiers {
Expand Down Expand Up @@ -488,9 +512,9 @@ impl Method {
// Related result types.
// <https://clang.llvm.org/docs/AutomaticReferenceCounting.html#related-result-types>
let is_related = if is_class {
matches!(memory_management, MemoryManagement::IdNew)
matches!(memory_management, MemoryManagement::RetainedNew { .. })
} else {
matches!(memory_management, MemoryManagement::IdInit) || selector == "self"
matches!(memory_management, MemoryManagement::RetainedInit) || selector == "self"
};

if is_related {
Expand Down Expand Up @@ -681,7 +705,7 @@ impl Method {
if self.is_class {
true
} else {
self.memory_management == MemoryManagement::IdInit
self.memory_management == MemoryManagement::RetainedInit
}
}

Expand Down Expand Up @@ -774,17 +798,41 @@ impl fmt::Display for Method {
self.selector, error_trailing
)?;

let method_family = match &self.memory_management {
// MemoryManagement::IdAlloc => "alloc", // Unsupported
MemoryManagement::IdCopy => "copy",
MemoryManagement::IdMutableCopy => "mutableCopy",
MemoryManagement::IdNew => "new",
MemoryManagement::IdInit => "init",
MemoryManagement::IdOther => "none",
MemoryManagement::Normal => "none",
let (method_family, attr) = match &self.memory_management {
MemoryManagement::RetainedAlloc => ("alloc", None),
MemoryManagement::RetainedCopy {
returns_not_retained: false,
} => ("copy", None),
MemoryManagement::RetainedCopy {
returns_not_retained: true,
} => ("none", Some("returns_not_retained")),
MemoryManagement::RetainedMutableCopy {
returns_not_retained: false,
} => ("mutableCopy", None),
MemoryManagement::RetainedMutableCopy {
returns_not_retained: true,
} => ("none", Some("returns_not_retained")),
MemoryManagement::RetainedNew {
returns_not_retained: false,
} => ("new", None),
MemoryManagement::RetainedNew {
returns_not_retained: true,
} => ("none", Some("returns_not_retained")),
MemoryManagement::RetainedInit => ("init", None),
MemoryManagement::RetainedNone {
returns_retained: false,
} => ("none", None),
MemoryManagement::RetainedNone {
returns_retained: true,
} => ("copy", Some("returns_retained")),
MemoryManagement::Normal => ("none", None),
};
// TODO: Be explicit about when we emit this for better
// compile-times, and when we do it for soundness.
if let Some(attr) = attr {
writeln!(
f,
" // required for soundness, method has `{attr}` attribute."
)?;
}
writeln!(f, " #[unsafe(method_family = {method_family})]")?;

//
Expand All @@ -802,7 +850,7 @@ impl fmt::Display for Method {
write!(f, "fn {}(", handle_reserved(&self.fn_name))?;

// Receiver
if let MemoryManagement::IdInit = self.memory_management {
if let MemoryManagement::RetainedInit = self.memory_management {
write!(f, "this: Allocated<Self>, ")?;
} else if self.is_class {
// Insert nothing; a class method is assumed
Expand Down
1 change: 1 addition & 0 deletions framework-crates/objc2-core-image/Cargo.toml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 0 additions & 10 deletions framework-crates/objc2-core-image/translation-config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,6 @@ visionos = "1.0"

external.MLModel.module = "CoreML.MLModel"

# CF_RETURNS_NOT_RETAINED
class.CIColor.methods.colorSpace.skipped = true
class.CIImage.methods.colorSpace.skipped = true

# CF_RETURNS_RETAINED
class.CIContext.methods."createCGLayerWithSize:info:".skipped = true
class.CIContext.methods."createCGImage:fromRect:".skipped = true
class.CIContext.methods."createCGImage:fromRect:format:colorSpace:".skipped = true
class.CIContext.methods."createCGImage:fromRect:format:colorSpace:deferred:".skipped = true

# Breaks usual conventions for errors, the return value is _not_ nullable, so
# the user must always check the error variable.
class.CIFilter.methods."filterArrayFromSerializedXMP:inputImageExtent:error:".skipped = true
Expand Down
3 changes: 0 additions & 3 deletions framework-crates/objc2-event-kit/translation-config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,5 @@ visionos = "1.0"
external.NSColor.module = "AppKit.NSColor"
external.MKMapItem.module = "MapKit.MKMapItem"

# CF_RETURNS_NOT_RETAINED
class.EKParticipant.methods."ABRecordWithAddressBook:".skipped = true

# Needs the `AddressBook` framework
class.EKParticipant.methods."ABPersonInAddressBook:".skipped = true
1 change: 0 additions & 1 deletion framework-crates/objc2-ui-kit/translation-config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ external.CLRegion.module = "CoreLocation.CLRegion"

# CF_RETURNS_NOT_RETAINED
fn.UIGraphicsGetCurrentContext.skipped = true
class.UIGraphicsRenderer.methods."contextWithFormat:".skipped = true

# Both property and method
class.NSDiffableDataSourceSectionSnapshot.methods.items.skipped = true
Expand Down

0 comments on commit 5dc0697

Please sign in to comment.