search

madsmtm / objc2

Bindings to Apple's frameworks in Rust
https://docs.rs/objc2/
MIT License
363 stars 40 forks source link

How is downcasting expected to work #658

Open Jasper-Bekkers opened 3 weeks ago

Jasper-Bekkers commented 3 weeks ago

I'm currently on the latest commit, trying to create a MTLAllocation from a MTLHeap can I can't figure it out.

Technically a ProtocolObject<dyn MTLHeap>, I seem to hit this problem every so often and need increasingly arcane methods of casting these down. Similar for Retained objects. Both would be extremely welcome to have a cast function that would convert from U to T where U:T.

Jasper-Bekkers commented 3 weeks ago

ProtocolObject::<dyn MTLEvent>::from_retained or ProtocolObject::<dyn MTLEvent>::from_ref

madsmtm commented 3 weeks ago

Yup, that's the ones.

I'll re-open this though, and use it to track finding a nicer way of doing these conversions. I'm considering AsRef and/or Into implementations, though not sure if coherence will allow us to add them?

Jasper-Bekkers commented 3 weeks ago

Potentially just having better method names for these would help, I ultimately found them on Retained::downcast (which would be a word I'd look for when casting).

MarijnS95 commented 3 weeks ago

I do wonder how this protocol type checking works. We have U: T where MTLHeap and MTLResource are known to implement the MTLAllocation protocol. But it's a new protocol that showed up in the XCode 16 beta, and is only available on MacOS 15 onwards (https://developer.apple.com/documentation/metal/mtlallocation) whereas these protocols (MTLHeap and MTLResource) are already available on older releases.

On those older releases MTLAllocation doesn't exist, so there's no backwards-compatible given way to upcast to a "higher" protocol in the trait chain, unless this happens in a fallible manner?

madsmtm commented 3 weeks ago

I do wonder how this protocol type checking works. We have U: T where MTLHeap and MTLResource are known to implement the MTLAllocation protocol. But it's a new protocol that showed up in the XCode 16 beta, and is only available on MacOS 15 onwards (https://developer.apple.com/documentation/metal/mtlallocation) whereas these protocols (MTLHeap and MTLResource) are already available on older releases.

On those older releases MTLAllocation doesn't exist, so there's no backwards-compatible given way to upcast to a "higher" protocol in the trait chain, unless this happens in a fallible manner?

To start with, upcasting to a protocol (e.g. ProtocolObject<dyn MTLResource> -> ProtocolObject<dyn MTLAllocation> or NSString -> ProtocolObject<dyn NSCopying>) with from_retained / from_ref exist purely in the type-system, and don't actually do anything at runtime. Downcasting protocols (e.g. NSObject -> ProtocolObject<dyn MTLAllocation>) isn't yet implemented, but would have to be a runtime check, and yes, that would fail on older macOS versions.

To more specifically answer your question, upcasting to ProtocolObject<dyn MTLAllocation> is safe/sound in older versions, as protocols are mostly a concept that exists in the type-system to make it safer than passing around AnyObject. That said, if you actually call any methods on the protocol, such as the allocatedSize, that will of course not work in older versions.

MarijnS95 commented 3 weeks ago

That said, if you actually call any methods on the protocol, such as the allocatedSize, that will of course not work in older versions.

That's where the crux lies; this purely appears to be a getter which likely does not need to be unsafe. So what would actually happen if we did this on MacOS 14?

Hence, if upcasting (as expected) doesn't do anything at runtime:

Just wanted to make sure an Into for these things probably wouldn't fly.

madsmtm commented 3 weeks ago

That said, if you actually call any methods on the protocol, such as the allocatedSize, that will of course not work in older versions.

That's where the crux lies; this purely appears to be a getter which likely does not need to be unsafe. So what would actually happen if we did this on MacOS 14?

Same thing as happens elsewhere if you call a method on an object that isn't available; the runtime sees that there's no selector with that name, and you crash (safely, so not UB). So it would be perfectly fine for us to mark it as safe.

If you want to call the method with a check at runtime first, you need to do the availability checking yourself:

We cannot do runtime checking to return an Option on older MacOS?

We could, conformsToProtocol exists for that kind of thing, but I'd argue it's unnecessary, as again, the ProtocolObject<dyn T> is literally just AnyObject that allows access to the methods on the protocol.

Besides, it'd be overly restrictive, there's many cases where Apple has introduced a protocol into an API, in a later version than the API itself was introduced.

  • Hence casting helpers (i.e. based on ProtocolObject::from_ref()) must be unsafe because unlike conventional trait/object hierarchies, it isn't guaranteed to implement the parent that we upcast to.

Just wanted to make sure an Into for these things probably wouldn't fly.

No, as outlined above, the safety of an Into impl would not be affected by the runtime availability of the API.

MarijnS95 commented 3 weeks ago

Not the safety, but the usefulness. Assuming a developer is on the latest (Beta) version of MacOS and relies on an infallible .into() to go from MTLHeap to MTLAllocation, only to see it consistently crash (not UB, but still, a crash) on older end-user machines seems strange to me.

madsmtm commented 3 weeks ago

Not the safety, but the usefulness. Assuming a developer is on the latest (Beta) version of MacOS and relies on an infallible .into() to go from MTLHeap to MTLAllocation, only to see it consistently crash (not UB, but still, a crash) on older end-user machines seems strange to me.

Yeah, that's a fair point. Again, the Into conversion itself wouldn't fail, but method calls would.

I once considered making method calls return an Option everywhere that they might be unavailable, but I think that would be prohibitively annoying, and the user would likely just end up doing a bunch of .unwrap()s. Besides, this idea wouldn't work well with deployment targets, as users that opt in to newer versions would then still have to do the checks, and the API would need to break if we raised the minimum supported version.

The way Swift and Objective-C solves this whole ordeal is with compiler warnings, which we can't do as we're just a library. I am thinking of ways to work around this (see again https://github.com/madsmtm/objc2/issues/266), but it's fairly difficult, and my current ideas would still require testing at runtime.

(I'm not sure how my tone sounds here, I fear I may sound a little dismissive because I feel pretty certain in the chosen solution at this point (i.e. do nothing), but wanted to state that I'm glad to have this discussion (!), and I am open to try changing my viewpoint)

MarijnS95 commented 3 weeks ago

I align exactly with the chosen solution, i.e. do nothing. Specifically because, while the original issue description may make it sound like this is an ordinary U: T inheritance chain that one might find in a trait hierarchy, C++ inheritance hierarchy, or COM hierarchy, where upcasting is infallible, that appears to not the case in Objective-C (as I learned before catching up to this issue).

One might hence expect any upcasting-like API like the proposed Into conversion to fail (by means of a panic or better yet, TryInto) or otherwise have callable methods; not for the conversion to a specific protocol to succeed (and "own" an instance of that protocol object) only to fail when any of its methods are called.

I'll leave it to you to consider the best API to describe this, just glad that we're on the same page when it comes to these "inherited protocols" to really be optional.

Jasper-Bekkers commented 3 weeks ago

For what it's worth, for me this issue was mostly about the discoverability of a casting API (either direction), not so much about the possibility of being able to.

The only casting api I could find at the time was on Retained, while I needed to casting a ProtocolObject. I had seen the from_ref and from_retained apis before but never connected the dots that these were for casting.