Skip to content

Commit d4cd63b

Browse files
madsmtmmadsmtm
authored
Merge pull request #509 from madsmtm/protocol-requirements
Fix main thread requirements on protocols
2 parents 67f09bf + fe33f6c commit d4cd63b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+882
-341
lines changed

‎crates/header-translator/src/cache.rs‎

Lines changed: 68 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -13,31 +13,52 @@ use crate::Mutability;
1313
#[derive(Debug, PartialEq, Clone)]
1414
pub struct Cache<'a> {
1515
config: &'a Config,
16-
mainthreadonly_classes: BTreeSet<ItemIdentifier>,
16+
mainthreadonly_items: BTreeSet<ItemIdentifier>,
1717
}
1818

1919
impl<'a> Cache<'a> {
2020
pub fn new(output: &Output, config: &'a Config) -> Self {
21-
let mut mainthreadonly_classes = BTreeSet::new();
21+
let mut mainthreadonly_items = BTreeSet::new();
2222

2323
for library in output.libraries.values() {
2424
for file in library.files.values() {
2525
for stmt in file.stmts.iter() {
26-
if let Stmt::ClassDecl {
27-
id,
28-
mutability: Mutability::MainThreadOnly,
29-
..
30-
} = stmt
31-
{
32-
mainthreadonly_classes.insert(id.clone());
26+
match stmt {
27+
Stmt::ClassDecl {
28+
id,
29+
mutability: Mutability::MainThreadOnly,
30+
..
31+
} => {
32+
mainthreadonly_items.insert(id.clone());
33+
}
34+
Stmt::ProtocolDecl {
35+
id,
36+
required_mainthreadonly: true,
37+
..
38+
} => {
39+
mainthreadonly_items.insert(id.clone());
40+
}
41+
Stmt::ProtocolDecl {
42+
id,
43+
required_mainthreadonly: false,
44+
protocols,
45+
..
46+
} => {
47+
for protocol in protocols {
48+
if mainthreadonly_items.contains(protocol) {
49+
let _ = mainthreadonly_items.insert(id.clone());
50+
}
51+
}
52+
}
53+
_ => {}
3354
}
3455
}
3556
}
3657
}
3758

3859
Self {
3960
config,
40-
mainthreadonly_classes,
61+
mainthreadonly_items,
4162
}
4263
}
4364

@@ -100,50 +121,51 @@ impl<'a> Cache<'a> {
100121
for method in methods.iter_mut() {
101122
let mut result_type_contains_mainthreadonly: bool = false;
102123
method.result_type.visit_required_types(&mut |id| {
103-
if self.mainthreadonly_classes.contains(id) {
124+
if self.mainthreadonly_items.contains(id) {
104125
result_type_contains_mainthreadonly = true;
105126
}
106127
});
107128

108-
match (method.is_class, self.mainthreadonly_classes.contains(id)) {
109-
// MainThreadOnly class with static method
110-
(true, true) => {
111-
// Assume the method needs main thread
112-
result_type_contains_mainthreadonly = true;
113-
}
114-
// Class with static method
115-
(true, false) => {
116-
// Continue with the normal check
117-
}
118-
// MainThreadOnly class with non-static method
119-
(false, true) => {
120-
// Method is already required to run on main
121-
// thread, so no need to add MainThreadMarker
122-
continue;
123-
}
124-
// Class with non-static method
125-
(false, false) => {
126-
// Continue with the normal check
127-
}
129+
let mut any_argument_contains_mainthreadonly: bool = false;
130+
for (_, argument) in method.arguments.iter() {
131+
// Important: We only visit the top-level types, to not
132+
// include optional arguments like `Option<&NSView>` or
133+
// `&NSArray<NSView>`.
134+
argument.visit_toplevel_types(&mut |id| {
135+
if self.mainthreadonly_items.contains(id) {
136+
any_argument_contains_mainthreadonly = true;
137+
}
138+
});
128139
}
129140

130-
if result_type_contains_mainthreadonly {
131-
let mut any_argument_contains_mainthreadonly: bool = false;
132-
for (_, argument) in method.arguments.iter() {
133-
// Important: We only visit the top-level types, to not
134-
// include e.g. `Option<&NSView>` or `&NSArray<NSView>`.
135-
argument.visit_toplevel_types(&mut |id| {
136-
if self.mainthreadonly_classes.contains(id) {
137-
any_argument_contains_mainthreadonly = true;
138-
}
139-
});
141+
if self.mainthreadonly_items.contains(id) {
142+
if method.is_class {
143+
// Assume the method needs main thread if it's
144+
// declared on a main thread only class.
145+
result_type_contains_mainthreadonly = true;
146+
} else {
147+
// Method takes `&self` or `&mut self`, or is
148+
// an initialization method, all of which
149+
// already require the main thread.
150+
//
151+
// Note: Initialization methods can be passed
152+
// `None`, but in that case the return will
153+
// always be NULL.
154+
any_argument_contains_mainthreadonly = true;
140155
}
156+
}
141157

142-
// Apply main thread only, unless a (required)
143-
// argument was main thread only.
144-
if !any_argument_contains_mainthreadonly {
145-
method.mainthreadonly = true;
146-
}
158+
if any_argument_contains_mainthreadonly {
159+
// MainThreadMarker can be retrieved from
160+
// `MainThreadMarker::from` inside these methods,
161+
// and hence passing it is redundant.
162+
method.mainthreadonly = false;
163+
} else if result_type_contains_mainthreadonly {
164+
method.mainthreadonly = true;
165+
} else {
166+
// If neither, then we respect any annotation
167+
// the method may have had before
168+
// method.mainthreadonly = method.mainthreadonly;
147169
}
148170
}
149171
}

‎crates/header-translator/src/config.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,9 @@ pub struct ProtocolData {
146146
#[serde(default)]
147147
pub skipped: bool,
148148
#[serde(default)]
149+
#[serde(rename = "requires-mainthreadonly")]
150+
pub requires_mainthreadonly: Option<bool>,
151+
#[serde(default)]
149152
pub methods: HashMap<String, MethodData>,
150153
}
151154

‎crates/header-translator/src/data/AppKit.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,13 @@ data! {
3030
// `run` cannot be safe, the user must ensure there is no re-entrancy.
3131
}
3232

33+
class NSController: MainThreadOnly {}
34+
class NSObjectController: MainThreadOnly {}
35+
class NSArrayController: MainThreadOnly {}
36+
class NSDictionaryController: MainThreadOnly {}
37+
class NSTreeController: MainThreadOnly {}
38+
class NSUserDefaultsController: MainThreadOnly {}
39+
3340
// Documentation says:
3441
// > Color objects are immutable and thread-safe
3542
//
@@ -38,6 +45,8 @@ data! {
3845
unsafe -clear;
3946
}
4047

48+
class NSColorPicker: MainThreadOnly {}
49+
4150
class NSControl {
4251
unsafe -isEnabled;
4352
unsafe -setEnabled;
@@ -81,6 +90,8 @@ data! {
8190

8291
}
8392

93+
class NSFontManager: MainThreadOnly {}
94+
8495
// Documented Thread-Unsafe, but:
8596
// > One thread can create an NSImage object, draw to the image buffer,
8697
// > and pass it off to the main thread for drawing. The underlying image

‎crates/header-translator/src/data/Automator.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
data! {
2+
class AMWorkflowController: MainThreadOnly {}
23
}

‎crates/header-translator/src/data/OSAKit.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
data! {
2+
class OSAScriptController: MainThreadOnly {}
23
}

‎crates/header-translator/src/id.rs‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
use core::cmp::Ordering;
12
use core::fmt;
3+
use core::hash;
24

35
use clang::Entity;
46

@@ -20,7 +22,7 @@ impl ToOptionString for Option<String> {
2022
}
2123
}
2224

23-
#[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
25+
#[derive(Debug, Clone)]
2426
pub struct ItemIdentifier<N = String> {
2527
/// Names in Objective-C are global, so this is always enough to uniquely
2628
/// identify the item.
@@ -31,6 +33,32 @@ pub struct ItemIdentifier<N = String> {
3133
pub file_name: Option<String>,
3234
}
3335

36+
impl<N: PartialEq> PartialEq for ItemIdentifier<N> {
37+
fn eq(&self, other: &Self) -> bool {
38+
self.name == other.name
39+
}
40+
}
41+
42+
impl<N: Eq> Eq for ItemIdentifier<N> {}
43+
44+
impl<N: hash::Hash> hash::Hash for ItemIdentifier<N> {
45+
fn hash<H: hash::Hasher>(&self, state: &mut H) {
46+
self.name.hash(state);
47+
}
48+
}
49+
50+
impl<N: Ord> PartialOrd for ItemIdentifier<N> {
51+
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
52+
Some(self.cmp(other))
53+
}
54+
}
55+
56+
impl<N: Ord> Ord for ItemIdentifier<N> {
57+
fn cmp(&self, other: &Self) -> Ordering {
58+
self.name.cmp(&other.name)
59+
}
60+
}
61+
3462
impl<N: ToOptionString> ItemIdentifier<N> {
3563
pub fn from_raw(name: N, library: String) -> Self {
3664
Self {

‎crates/header-translator/src/method.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -700,8 +700,7 @@ impl fmt::Display for Method {
700700
let param = handle_reserved(&crate::to_snake_case(param));
701701
write!(f, "{param}: {arg_ty}, ")?;
702702
}
703-
// FIXME: Skipping main thread only on protocols for now
704-
if self.mainthreadonly && !self.is_protocol {
703+
if self.mainthreadonly {
705704
write!(f, "mtm: MainThreadMarker")?;
706705
}
707706
write!(f, ")")?;

0 commit comments

Comments
 (0)