Automatically mark objc2-foundation as safe

[madsmtm]

I've been improving header-translator over the past few weeks to allow it to have a notion of when a method/function signature is safe or unsafe. This is used to emit a relevant # Safety comment, see for example NSMenuItem::setAction (the messages here aren't perfect, but they're a lot better than nothing).

There's two cases that it cannot fully handle: bounds information and restrictions only present in documentation. But apart from these, I believe that header-translator knows about all the other classes of memory safety issues. I've gone over this in a lot more detail in this document.

This vastly simplifies the safety review process, since as a reviewer, you only have to look for APIs where one of the two aforementioned issues are present. By now, I have enough confidence in this that I propose to switch from "unsafe by default" to "safe by default" in reviewed framework crates. This means that instead of having to manually marking each and every API as safe, we now instead mark the few APIs that do odd things as unsafe.

In this PR, I have done this for Foundation, see objc2-foundation/translation-config.toml. The unsafe-default-safety annotations change the default, and the rest of the annotations mark APIs as unsafe that I found that don't follow the normal rules. I intend to do the same for other framework crates later.

---

This is completely at odds with how safety is normally done in Rust!¹ In Rust, we prefer unsafe blocks to be scoped as tightly as possible. But here, I'm effectively applying a huge unsafe block around the entire crate that says "all of this is safe because the signature is safe", and only selectively unmarking APIs as "wait, actually this isn't safe". A single slip-up, and boom, the library is unsound.

Let me give my reasoning for why I propose doing it anyway:

When users use one of the framework crates, they currently have to write unsafe all over the place. As an example, in winit-appkit, there's currently 166 uses of unsafe {. I estimate that around half of those would disappear if Foundation and AppKit were appropriately marked safe. And this is in a library which I maintain where I've previously marked huge swaths of these APIs as safe for this purpose, in other projects this problem is further exacerbated.

The true danger here is not so much the annoyance of the extra annotations, but that maybe around 40 of the remaining unsafe { instances in winit-appkit are actually unsafe, and require appropriate // SAFETY comments - but this is masked by the huge number of not-really-worth-bothering-with-unsafe.

Thus, to avoid our users becoming "safety blind", and no longer paying attention to bigger safety concerns like a transmute, a raw pointer deref, NSWindow::setReleasedWhenClosed or Retained::cast_unchecked, we need to mark methods safe whenever possible. There is, however, more than 6000 methods in Foundation and more than 10000 methods in AppKit, which is far too large an amount for anyone to confidently review.

We are in a bind! The project's goals of soundness and not pushing safety onto users are in tension, and something has to give! I believe that endangering perfect soundness here will ultimately make user's code much safer.

That is not to say I haven't reviewed this; I have, and I'm fairly confident that I caught the most egregious instances. But again, there is 4000 methods that I'm marking safe in this PR, there's bound to be mistakes somewhere. I have created #782 to track the ones that we discover.

---

CC people that might have an opinion on this: @simlay @silvanshade @not-jan @MarijnS95 @pcwalton @waywardmonkeys @extrawurst @tmandry @PaulDance @ryanmcgrath @complexspaces @pronebird @kvark @nicoburns @amodm @jdm @mrobinson

This is in a sense a policy change in the project, hence why I'm opening this for wider discussion. What do you think? Is this a no-go for you? Should I do it differently? Is there something that is unclear?

I'd also be curious if you know of APIs (Foundation or otherwise) that should definitely be marked unsafe, even though their signature might appear safe.

Footnotes

1. That there is a bit of precedent in the ecosystem for this in autocxx::safety!, but it is far from widespread. ↩

[nicoburns]

Hmm... I have mixed feelings about this. I would definitely be keen to reduce "unsafe fatigue". But I do worry somewhat about it being "safe by default".

You mention some invariants are only contained in documentation, and I wonder if we could look at automatically pulling the documentation into doc comments. That might make it more obvious to people that there are invariants to uphold (when there are).

Apple seems to make their docs available in JSON format (e.g. https://developer.apple.com/tutorials/data/documentation/foundation/nsstring/linerange(for:).json?language=objc), and https://github.com/NSHipster/sosumi.ai/blob/main/src/lib/fetch.ts has more details on the URL conventions.

(looks like they're not intentionally making the docs available in machine-readable format, it's just that their official docs are an SPA backed by an API)

[madsmtm]

Apple seems to make their docs available in JSON format

Damn, I have been looking for an actual machine-readable format like that for ages, thanks for the link! Maybe they changed it recently? In any case, definitely something I'll look into doing - tracking in #309.

[tmandry]

Your doc and your work up to this point give me confidence that this is pretty close to the mark. I think you should do this if you're confident enough that you are willing to accept the semver consequences to you and your users when the annotation is wrong. In other words, if you have to mark a function as unsafe, that's "allowed breakage" to your users who will have to go update their code if they call it, and may result in some unfortunate version pinning situationsdownstream. Releasing a new major version is probably unworkable. I don't see a better alternative. No one is going to review all 4000 functions by hand, and the consequence of unsafe-by-default is that the unsafe is just noise that people work around to get their job done. Conversely, if someone sees a function marked safe that they know should be unsafe I think they would be quick to file a bug! My hope is that when you do find mistakes, those give you patterns that can help you find more (or they're one off exceptions without similar cases; hopefully there aren't very many of those).

[madsmtm]

My hope is that when you do find mistakes, those give you patterns that can help you find more (or they're one off exceptions without similar cases; hopefully there aren't very many of those).

The mistakes currently listed in #782 are actually found by header-translator after I "distilled" what's safe and what's not into it. And as you say, I suspect I'll be able to do the same for future errors.

[jdm]

I share the feelings of nicoburns and tmandry. This seems like a pragmatic choice.

[complexspaces]

I am in the same boat as the others who saw the email notification first, this seems to be the right direction to me and the detail of research put into getting the project this far is pretty incredible. As long as version churn is minimized (especially as objc2-* makes its way into more and more of the ecosystem), I don't see any issues here.

I also want to explicitly second the callout about unsafe fatigue: its a very real problem. For example at my current employer we have ended up with a lot of functions that just have //SAFETY: Unsafe block needed to call into Objective-C/Swift as a result of mixing together "trivially safe" calls with others that actually require reasoning. Its also a burden on reviewers. With the right preparation I'm quite happy to see approaches like this to cut down on both sides of the problem of too much ambient unsafe.

[pronebird]

I concur. Rust treats any FFI boundary as unsafe. There is nothing unsafe about Objective-C/Swift frameworks unless there are internal preconditions that cause exceptions to be thrown. We have tooling to ensure that signatures are correct, Rust can be used to provide guarantees that pointers are correct. The rest is on the foreign code.

Also note that Rust doesn't consider its own code as being unsafe if there is a panic! put somewhere in the middle and how's panic different from exception thrown by Objective-C code in such case.

[PaulDance]

+1

[amodm]

I'd be in favour of the change. I'd normally be against any crate doing this, but when dealing with a surface area as large as objc2-foundation, it does seem a better tradeoff here. As a user of this crate, I'll be comfortable with this.

Thanks for actively soliciting feedback here, @madsmtm.

[mrobinson]

I agree that the pragmatic approach seems best in this case. It could be the case that a a method might be marked safe when it should be marked as unsafe with this approach, but these cases can be fixed iteratively. Additionally, the unsafe marking doesn't automatically fix any problems that the crate user has created via a misuse of the API and, as you say @madsmtm, can hide other ones.

[madsmtm]

Thanks so much all of you for taking the time to comment! It's really encouraging to know that there are people invested in the project, and very nice to have someone that I can call upon for hard decisions like these!

I'll leave the PR open for a week or two, to let others chime in (I'd have liked to give it more time, but if this PR is going to land, I would like it to be in v0.3.2 along with Xcode 26).

[simlay]

I think this is a pretty pragmatic plan. In my various hackery, I frequently commit a bad practices and ignore the "unsafeness" a lot when I use various objc2 generated bindings and throw a "todo" line in for future me to forget about. I know that a lot of these calls aren't all that dangerous but I feel bad about it. If all the things are unsafe and we ignore them, we're degrading the definition of unsafe.

Tangentially related, there's a bit of similarity between objc2 and jni-rs. Certain things I look at with jni-rs and feel less safe about (this is not a slight at rib/jni-rs, rib is awesome). If there's a typo in a java class name string, it'll blow up/return an error which is basically what happens with msg_send (requires unsafe) in objc2. Similarly there are some unsafe functions and sometimes why they're unsafe and the arguments follow similar arguments mentioned in the framework soundness doc.

Anyway, I guess I don't know how to go about validating/invalidating the safety of a given class/function/etc. Like:

# Foundation's properties are well-behaved.
unsafe-default-safety.property-getters = true
unsafe-default-safety.property-setters = true

Which I trust but how do we go about this for future frameworks? It's outside the scope of this issue and I know it was partially raised in #685.

[madsmtm]

how do we go about this for future frameworks

I don't know the full answer to that, so for now, it'll probably be a case-by-case thing. For example, in AppKit, the getters are safe, but a few setters like -[NSWindow setReleasedWhenClosed:] are unsafe.

[madsmtm]

It seems the consensus is to go forwards with this change, so I will do so. Again, thanks all for the input, I truly appreciate it!

And of course, if you at some point find a safety mistake, please do tell me about it in #782.