-
Notifications
You must be signed in to change notification settings - Fork 65
Description
So... I've kinda considered this problem many times, #265 contains some of the previous discussion and my head contains even more, but I feel like we're still not at an optimum.
Currently, objc2 contains a lot of complex machinery to make classes able to opt-in to being &mut, and icrate uses this on NSMutableString, NSMutableAttributedString, NSMutableArray<T>, NSMutableSet<T>, NSMutableDictionary<K, V> and NSEnumerator<T> to make accessing the inner state of those types safe.
A few examples of current issues in icrate with this approach:
NSMutableAttributedString::mutableStringmust return a reference bound to the attributed string, as otherwise you'd be able to create twoId<NSMutableString>to the same string. But this doesn't work withId<T>, so we'd need a lifetime annotation in there somehow (or maybe resort to autorelease pools).NSTreeNode::mutableChildNodeshas to remainunsafe, as we have no way of preventing calling that multiple times. Or maybeNSTreeNodealso has to be mutable? But that in turn would pushNSTreeControllerto being mutable, which in turn needs us to changeNSEditor.- While
NSThread::threadDictionaryhas to remainunsafebecause of thread-safety concerns, it cannot even be used from the owning thread safely. - In user code, storing one of these mutable objects in a custom object needs to be done using a
RefCell.
So while &mut does work for the select few types we've introduced it on, my main problem is that this is not enough! &mut is infectious, and we need to introduce it consistently, everywhere!
And that, in turn, is just not feasible (time-wise, Apple has a lot of API surface)! Especially so with our goal of making header-translator a user-accessible tool, which means user-controlled code, and in turn means users would also have to add extra configuration to handle this for their mutable types, which is a tough ask.
Honestly, maybe we should abandon this endeavour, and instead go down the beaten path Swift has taken; do not try to do anything about mutability, i.e. mark NSString, NSMutableString and so on as InteriorMutable, and have all methods accept &self?
All of this is not to say that &mut doesn't have advantages; it does, very much so! And removing it does have several performance-related disadvantages, namely:
NSString::as_strwould have to beunsafe, since the internal storage for the string could now be changed through&self. This has a perf-impact on theimpl Display for NSString, as the&mut Formatter<'_>in there could contain a&NSMutableStringinternally that pointed to the same string, and hence we'll probably need some sort of allocation.NSData::byteswould have to beunsafe, which in turn means that the nice impls ofDeref,Index,Writeand so would be gone.- Other methods using
NS_RETURNS_INNER_POINTERwould have to beunsafe. - Getters on collections would have to retain the object (this is also what they do in Swift), whereas before we could avoid this since they're guaranteed to point into internal storage, and we knew we weren't changing that storage.
- Iterators would have to retain the element being iterated over, to avoid problems if the iterator is invalidated - see here for an example of the issue.
- Even then, are they sound? Is the fast iterator mutation detection enough? What about if the iterator is passed in and out of an autorelease pool?
- Will have to be researched (and fuzzed), but a safe (inefficient) fallback is always to just use
array.objectAtIndex(i).
NSString,NSMutableString,NSArray, and so on would loose theirSend + Sync-ness.
Some of these performance implications could be resolved by making extra unsafe methods with a safety precondition that the object is not mutated for the lifetime of the return value, although that is still a worse user-experience.