Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix protocol names #508

Merged
merged 3 commits into from
Sep 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions crates/header-translator/src/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,7 @@ pub enum Stmt {
/// extern_protocol!
ProtocolDecl {
id: ItemIdentifier,
actual_name: Option<String>,
availability: Availability,
protocols: BTreeSet<ItemIdentifier>,
methods: Vec<Method>,
Expand Down Expand Up @@ -797,9 +798,14 @@ impl Stmt {
.collect()
}
EntityKind::ObjCProtocolDecl => {
let id = ItemIdentifier::new(entity, context)
.map_name(|name| context.replace_protocol_name(name));
let data = context.protocol_data.get(&id.name);
let actual_id = ItemIdentifier::new(entity, context);
let data = context.protocol_data.get(&actual_id.name);
let actual_name = data
.map(|data| data.renamed.is_some())
.unwrap_or_default()
.then(|| actual_id.name.clone());

let id = actual_id.map_name(|name| context.replace_protocol_name(name));

if data.map(|data| data.skipped).unwrap_or_default() {
return vec![];
Expand Down Expand Up @@ -830,6 +836,7 @@ impl Stmt {

vec![Self::ProtocolDecl {
id,
actual_name,
availability,
protocols,
methods,
Expand Down Expand Up @@ -1616,6 +1623,7 @@ impl fmt::Display for Stmt {
}
Self::ProtocolDecl {
id,
actual_name,
availability,
protocols,
methods,
Expand Down Expand Up @@ -1683,7 +1691,13 @@ impl fmt::Display for Stmt {
}
writeln!(f, " }}")?;
writeln!(f)?;
writeln!(f, " unsafe impl ProtocolType for dyn {} {{}}", id.name)?;
writeln!(f, " unsafe impl ProtocolType for dyn {} {{", id.name)?;
if let Some(actual_name) = actual_name {
writeln!(f)?;
writeln!(f, " const NAME: &'static str = {actual_name:?};")?;
write!(f, " ")?;
}
writeln!(f, "}}")?;
writeln!(f, ");")?;
}
Self::StructDecl {
Expand Down
2 changes: 2 additions & 0 deletions crates/icrate/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
to use in such hashing collections.
* **BREAKING**: Added `HasStableHash` requirement on `NSDictionary` and
`NSSet` creation methods, fixing a long-standing soundess issue.
* Fixed the protocol names of `NSAccessibilityElementProtocol`,
`NSTextAttachmentCellProtocol` and `NSFileProviderItemProtocol`.


## icrate 0.0.4 - 2023-07-31
Expand Down
4 changes: 2 additions & 2 deletions crates/icrate/examples/browser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ declare_class!(
}
}

unsafe impl NSObjectProtocol for Delegate {}

unsafe impl NSApplicationDelegate for Delegate {
#[method(applicationDidFinishLaunching:)]
#[allow(non_snake_case)]
Expand Down Expand Up @@ -293,8 +295,6 @@ impl Delegate {
}
}

unsafe impl NSObjectProtocol for Delegate {}

fn main() {
let mtm = MainThreadMarker::new().unwrap();
let app = NSApplication::sharedApplication(mtm);
Expand Down
4 changes: 2 additions & 2 deletions crates/icrate/examples/delegate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ declare_class!(
}
}

unsafe impl NSObjectProtocol for AppDelegate {}

unsafe impl NSApplicationDelegate for AppDelegate {
#[method(applicationDidFinishLaunching:)]
fn did_finish_launching(&self, notification: &NSNotification) {
Expand All @@ -65,8 +67,6 @@ declare_class!(
}
);

unsafe impl NSObjectProtocol for AppDelegate {}

impl AppDelegate {
pub fn new(ivar: u8, another_ivar: bool, mtm: MainThreadMarker) -> Id<Self> {
unsafe { msg_send_id![mtm.alloc(), initWith: ivar, another: another_ivar] }
Expand Down
16 changes: 8 additions & 8 deletions crates/icrate/examples/metal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,21 @@ use objc2::{
#[rustfmt::skip]
const SHADERS: &str = r#"
#include <metal_stdlib>

struct SceneProperties {
float time;
};
};

struct VertexInput {
metal::packed_float3 position;
metal::packed_float3 color;
};

struct VertexOutput {
metal::float4 position [[position]];
metal::float4 color;
};

vertex VertexOutput vertex_main(
device const SceneProperties& properties [[buffer(0)]],
device const VertexInput* vertices [[buffer(1)]],
Expand All @@ -64,7 +64,7 @@ const SHADERS: &str = r#"
out.color = metal::float4(in.color, 1);
return out;
}

fragment metal::float4 fragment_main(VertexOutput in [[stage_in]]) {
return in.color;
}
Expand Down Expand Up @@ -155,6 +155,8 @@ declare_class!(
}
}

unsafe impl NSObjectProtocol for Delegate {}

// define the delegate methods for the `NSApplicationDelegate` protocol
unsafe impl NSApplicationDelegate for Delegate {
#[method(applicationDidFinishLaunching:)]
Expand Down Expand Up @@ -356,8 +358,6 @@ declare_class!(
}
);

unsafe impl NSObjectProtocol for Delegate {}

impl Delegate {
pub fn new(mtm: MainThreadMarker) -> Id<Self> {
unsafe { msg_send_id![mtm.alloc(), init] }
Expand Down
8 changes: 2 additions & 6 deletions crates/icrate/src/fixes/Foundation/copying.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ extern_protocol!(
Self: CounterpartOrSelf;
}

unsafe impl ProtocolType for dyn NSCopying {
const NAME: &'static str = "NSCopying";
}
unsafe impl ProtocolType for dyn NSCopying {}
);

// FIXME: Remove this hack which makes NSMutableDictionary tests work
Expand Down Expand Up @@ -86,7 +84,5 @@ extern_protocol!(
Self: CounterpartOrSelf;
}

unsafe impl ProtocolType for dyn NSMutableCopying {
const NAME: &'static str = "NSMutableCopying";
}
unsafe impl ProtocolType for dyn NSMutableCopying {}
);
2 changes: 1 addition & 1 deletion crates/icrate/src/generated
9 changes: 9 additions & 0 deletions crates/icrate/tests/renamed_protocols.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#![cfg(feature = "AppKit")]
use icrate::AppKit::NSAccessibilityElementProtocol;
use objc2::ProtocolType;

#[test]
fn accessibility_element_protocol() {
let actual: &str = <dyn NSAccessibilityElementProtocol>::NAME;
assert_eq!(actual, "NSAccessibilityElement");
}
7 changes: 7 additions & 0 deletions crates/objc2/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
];
```

### Fixed
* Fixed the name of the protocol that `NSObjectProtocol` references.

### Removed
* **BREAKING**: Removed `ProtocolType` implementation for `NSObject`.
Use the more precise `NSObjectProtocol` trait instead!


## 0.4.1 - 2023-07-31

Expand Down
50 changes: 37 additions & 13 deletions crates/objc2/src/__macro_helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,32 +580,56 @@ impl ClassProtocolMethodsBuilder<'_, '_> {
}
}

#[inline]
#[cfg(all(debug_assertions, feature = "verify"))]
pub fn __finish(self) {
#[cfg(all(debug_assertions, feature = "verify"))]
let superclass = self.builder.superclass();

if let Some(protocol) = self.protocol {
for desc in &self.required_instance_methods {
if !self.registered_instance_methods.contains(&desc.sel) {
panic!(
"must implement required protocol method -[{protocol} {}]",
desc.sel
)
if self.registered_instance_methods.contains(&desc.sel) {
continue;
}

// TODO: Don't do this when `NS_PROTOCOL_REQUIRES_EXPLICIT_IMPLEMENTATION`
if superclass
.and_then(|superclass| superclass.instance_method(desc.sel))
.is_some()
{
continue;
}

panic!(
"must implement required protocol method -[{protocol} {}]",
desc.sel
)
}
}

#[cfg(all(debug_assertions, feature = "verify"))]
if let Some(protocol) = self.protocol {
for desc in &self.required_class_methods {
if !self.registered_class_methods.contains(&desc.sel) {
panic!(
"must implement required protocol method +[{protocol} {}]",
desc.sel
)
if self.registered_class_methods.contains(&desc.sel) {
continue;
}

// TODO: Don't do this when `NS_PROTOCOL_REQUIRES_EXPLICIT_IMPLEMENTATION`
if superclass
.and_then(|superclass| superclass.class_method(desc.sel))
.is_some()
{
continue;
}

panic!(
"must implement required protocol method +[{protocol} {}]",
desc.sel
);
}
}
}

#[inline]
#[cfg(not(all(debug_assertions, feature = "verify")))]
pub fn __finish(self) {}
}

#[cfg(test)]
Expand Down
46 changes: 42 additions & 4 deletions crates/objc2/src/declare/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ impl ClassBuilder {
}

#[allow(unused)]
fn superclass(&self) -> Option<&AnyClass> {
pub(crate) fn superclass(&self) -> Option<&AnyClass> {
// SAFETY: Though the class is not finalized, `class_getSuperclass` is
// still safe to call.
unsafe { AnyClass::superclass_raw(self.cls.as_ptr()) }
Expand Down Expand Up @@ -612,7 +612,9 @@ impl ClassBuilder {
pub fn add_protocol(&mut self, proto: &AnyProtocol) {
let success = unsafe { ffi::class_addProtocol(self.as_mut_ptr(), proto.as_ptr()) };
let success = Bool::from_raw(success).as_bool();
assert!(success, "failed to add protocol {proto}");
if cfg!(not(feature = "gnustep-1-7")) {
assert!(success, "failed to add protocol {proto}");
}
}

// fn add_property(&self, name: &str, attributes: &[ffi::objc_property_attribute_t]);
Expand Down Expand Up @@ -770,7 +772,7 @@ mod tests {
use crate::mutability::Immutable;
use crate::rc::Id;
use crate::runtime::{NSObject, NSObjectProtocol};
use crate::{declare_class, extern_methods, msg_send, test_utils, ClassType};
use crate::{declare_class, extern_methods, msg_send, test_utils, ClassType, ProtocolType};

#[test]
fn test_alignment() {
Expand Down Expand Up @@ -900,7 +902,43 @@ mod tests {
}

#[test]
#[should_panic = "failed to add protocol NSObject"]
fn inheriting_does_not_implement_protocols() {
let builder = ClassBuilder::new(
"TestClassBuilderInheritingDoesNotImplementProtocols",
NSObject::class(),
)
.unwrap();

let cls = builder.register();
let conforms = cls.conforms_to(<dyn NSObjectProtocol>::protocol().unwrap());
if cfg!(feature = "gnustep-1-7") {
// FIXME: GNUStep works differently here!
assert!(conforms);
} else {
assert!(!conforms);
}
}

#[test]
fn inherit_nsobject_add_protocol() {
let mut builder = ClassBuilder::new(
"TestClassBuilderInheritNSObjectAddProtocol",
NSObject::class(),
)
.unwrap();

let protocol = <dyn NSObjectProtocol>::protocol().unwrap();

builder.add_protocol(protocol);
let cls = builder.register();
assert!(cls.conforms_to(protocol));
}

#[test]
#[cfg_attr(
not(feature = "gnustep-1-7"),
should_panic = "failed to add protocol NSObject"
)]
fn duplicate_protocol() {
let cls = test_utils::custom_class();
let mut builder = ClassBuilder::new("TestClassBuilderDuplicateProtocol", cls).unwrap();
Expand Down
5 changes: 2 additions & 3 deletions crates/objc2/src/macros/declare_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@
/// # }
/// }
///
/// unsafe impl NSObjectProtocol for MyCustomObject {}
///
/// # #[cfg(available_in_icrate)]
/// unsafe impl NSCopying for MyCustomObject {
/// #[method_id(copyWithZone:)]
Expand All @@ -274,9 +276,6 @@
/// }
/// );
///
/// // TODO: Allow moving this inside `declare_class!`
/// unsafe impl NSObjectProtocol for MyCustomObject {}
///
/// impl MyCustomObject {
/// pub fn new(foo: u8) -> Id<Self> {
/// unsafe { msg_send_id![Self::alloc(), initWithFoo: foo] }
Expand Down
8 changes: 4 additions & 4 deletions crates/objc2/src/protocol_type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ use crate::runtime::AnyProtocol;
///
/// ```
/// use objc2::ProtocolType;
/// use objc2::runtime::NSObject;
/// // Get a protocol object representing `NSObject`
/// let protocol = NSObject::protocol().expect("NSObject to have a protocol");
/// assert_eq!(NSObject::NAME, protocol.name());
/// use objc2::runtime::NSObjectProtocol;
/// // Get a protocol object representing the `NSObject` protocol
/// let protocol = <dyn NSObjectProtocol>::protocol().expect("NSObject to have a protocol");
/// assert_eq!(<dyn NSObjectProtocol>::NAME, protocol.name());
/// ```
///
/// Use the [`extern_protocol!`] macro to implement this trait for a type.
Expand Down
Loading