search

madsmtm / objc2

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

Should sub-protocols (for i.e. `MTLAllocation`) turn on the parent feature? #663

Open MarijnS95 opened 2 hours ago

MarijnS95 commented 2 hours ago

Hi!

We recently patched our objc2-metal crate to the latest version in this repository. In https://github.com/madsmtm/objc2/commit/6442cbe990941450bbfe5b1612746f84ea9c2092 the new MTLAllocation protocol and feature flag were added. The effect is that, when we merely turn on features like MTLHeap and MTLResource, the code now compiles with "imports not found" for the respective types, because they're guarded behind MTLHeap and MTLAllocation.

While this is fixed by also turning on the MTLAllocation feature, doesn't it make more sense to have MTLHeap = ["MTLAllocation"] so that the feature is automatically enabled, for user convenience? Or if not, is there a thought-out reason why this should not be done?

Specifically, it took some digging through the new source code to understand all the places where these protocols were guarded, before realizing that it was now hidden by the new MTLAllocation feature.

MarijnS95 commented 2 hours ago

An unrelated side-issue to this is our "workaround" in https://github.com/Traverse-Research/gpu-allocator/commit/6a2f521 (and we later pushed a different workaround: https://github.com/Traverse-Research/gpu-allocator/commit/95b79c6). We have a codebase that uses gpu-allocator, where we patch objc2-metal and friends. We enable the new MTLAllocation feature there, and expect objc2-metal to be compiled just once for gpu-allocator and our internal use of the crate, yet gpu-allocator still fails to compile, as if rustc is able to track the origin of who requested a feature flag?

error[E0432]: unresolved imports `objc2_metal::MTLAccelerationStructure`, `objc2_metal::MTLBuffer`, `objc2_metal::MTLHeap`, `objc2_metal::MTLResource`, `objc2_metal::MTLTexture`
  --> .cargo/git/checkouts/gpu-allocator-a4a57137f341fd22/222b3d8/src/metal/mod.rs:12:5
   |
12 |     MTLAccelerationStructure, MTLBuffer, MTLCPUCacheMode, MTLDevice, MTLHeap, MTLHeapDescriptor,
   |     ^^^^^^^^^^^^^^^^^^^^^^^^  ^^^^^^^^^                              ^^^^^^^ no `MTLHeap` in the root
   |     |                         |
   |     |                         no `MTLBuffer` in the root
   |     no `MTLAccelerationStructure` in the root
   |     help: a similar name exists in the module: `MTLAccelerationStructureUsage`
13 |     MTLHeapType, MTLResource, MTLResourceOptions, MTLStorageMode, MTLTexture, MTLTextureDescriptor,
   |                  ^^^^^^^^^^^ no `MTLResource` in the root         ^^^^^^^^^^ no `MTLTexture` in the root