Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement runtime availability checking #661

Merged
merged 2 commits into from
Oct 27, 2024
Merged

Conversation

madsmtm
Copy link
Owner

@madsmtm madsmtm commented Oct 3, 2024

Add macro for runtime availability checking, with static assertions against the deployment target that allows eliding the runtime checks when unnecessary.

Part of #266.

Usage:

if available!(macos = 15.0, ios = 18.0, tvos = 18.0, watchos = 11.0, visonos = 2.0) {
    // Use API added in Xcode 16.0
}

// Or

#[cfg(target_os = "macos")]
if available!(macos = 15.0) {
    // Use API that's only relevant on macOS
}

// Or

if available!(macos = 10.7, ..) { // Note the `..`
    // Use API introduced in macOS 10.7, and take this branch on all other platforms too
}

Runtime lookup implementation tested on:

  • macOS 14.7, M1 Macbook
    • aarch64-apple-darwin
    • aarch64-apple-ios-macabi
    • x86_64-apple-ios-macabi
    • aarch64-apple-ios-sim
  • macOS 10.14.6, Intel Macbook
    • x86_64-apple-darwin
    • i686-apple-darwin
    • x86_64-apple-ios
  • iOS 9.3.6, 1st generation iPad Mini
    • armv7-apple-ios

TODO:

  • Implement runtime lookup.
  • Optimize runtime lookup with sysctl.
  • Decide on the naming and syntax of the macro. Alternatives that I can think of are:
    • os_version!(macos = 15.0, ..)
    • version!(macos = 15.0, ..)
    • os_available!(macos = 15.0, ..)
    • available!(macos(15.0), ..)
    • available!(macos 15.0, ..)
    • available!(macOS = 15.0, ..)
    • available!(macos = 15.0, *)
    • available!(macos = 15.0, _)
  • Use macro in examples. No current examples where it makes sense.
  • Decide if a compilation error when using available!(macos = 15.0) on e.g. iOS is desirable, or if we just want to return false here?
  • Document macro.
  • Optimize runtime lookup with atomics.

@madsmtm madsmtm added enhancement New feature or request A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates labels Oct 3, 2024
@madsmtm madsmtm force-pushed the availability-runtime branch 2 times, most recently from 21bbd88 to 6e50923 Compare October 4, 2024 06:31
@madsmtm
Copy link
Owner Author

madsmtm commented Oct 4, 2024

Noting my reasoning behind the name/syntax:

  1. The name "available" more closely matches the semantics, esp. at usage sites. E.g. "if available macos 15" vs "if os version macos 15". os_version sounds more like it'd return the actual OSVersion struct.
  2. macos = 15.0 looks more cfg-like, and matches general expectations of that. Compare with e.g. cfg!(target_feature = "crt-static"). macos 15.0, while terser, looks odd compared to the usual Rust parse.
  3. I chose lowercase macos because it matches the value of cfg!(target_os = "macos").
  4. For the optional trailing .. vs. _, I chose .. because it generally carries the meaning of an "all wildcard", see both the the docs on destructuring and RFC 3681.
    *, as Swift uses, is overloaded in Rust to mean many more things, so I think that'd be confusing.

@madsmtm
Copy link
Owner Author

madsmtm commented Oct 4, 2024

So, I tested using the available! macro a bit, both in winit and in wgpu, and found that making available!(macos = 10.13) a compilation error on iOS doesn't really make sense, there were many times where I wanted a piece of code to only run on a specific macOS version, and where #[cfg(target_os = "macos")] felt like overkill.

So instead, available!() without .. should return false for unspecified platforms, and with .. it should return true for unspecified platforms. I believe this also matches what Swift does.

@madsmtm madsmtm force-pushed the availability-runtime branch 2 times, most recently from 6bfa34c to e8eab1e Compare October 8, 2024 19:23
@madsmtm madsmtm marked this pull request as ready for review October 8, 2024 19:26
@madsmtm madsmtm requested a review from BlackHoleFox October 8, 2024 19:26
@madsmtm
Copy link
Owner Author

madsmtm commented Oct 8, 2024

Thanks for the review before @BlackHoleFox, really useful to get some feedback - if you feel like giving it another review, then feel free to do so now, I think I'm done with everything now.

I will try to upstream this implementation to the standard library later (perhaps under std::os::darwin?), but it's useful to have in objc2, both to get real-world testing, and to have it available now (instead of having to wait for it to be stable in std).

Copy link

@BlackHoleFox BlackHoleFox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cant really speak for the macro implementation details since I'm bad at them, but this looks nice and the usability of the macro seems fine 👀

I will try to upstream this implementation to the standard library later

std would probably want the CoreFoundation approach like libcompiler-rt has since it avoids needing to link and load the entirely of libobjc when only writing normal Rust apps. iirc you just recently removed that linkage from std too. but that doesn't matter for this since its literally an objective-c crate :)

crates/objc2/src/__macro_helpers/os_version.rs Outdated Show resolved Hide resolved
@madsmtm madsmtm force-pushed the availability-runtime branch from e8eab1e to 35653b4 Compare October 26, 2024 11:40
@madsmtm
Copy link
Owner Author

madsmtm commented Oct 26, 2024

cant really speak for the macro implementation details since I'm bad at them, but this looks nice and the usability of the macro seems fine 👀

Thanks for the review though!

I will try to upstream this implementation to the standard library later

std would probably want the CoreFoundation approach like libcompiler-rt has since it avoids needing to link and load the entirely of libobjc when only writing normal Rust apps. iirc you just recently removed that linkage from std too. but that doesn't matter for this since its literally an objective-c crate :)

Yeah, unsure exactly which approach would be best there, since the only part that's really needed in std is just something that can read the version in /System/Library/CoreServices/SystemVersion.plist (and that can be gotten from either CoreFoundation, Foundation, the plist crate or likely just re-implemented manually).

This includes internal methods for looking up the OS version at runtime,
and for looking up the deployment target at compile time.
Use atomics directly instead of the heavy `OnceLock`.
@madsmtm madsmtm force-pushed the availability-runtime branch from 35653b4 to 4485634 Compare October 27, 2024 13:35
@madsmtm madsmtm merged commit ca864ca into master Oct 27, 2024
19 checks passed
@madsmtm madsmtm deleted the availability-runtime branch October 27, 2024 13:41
@madsmtm madsmtm added this to the objc2 v0.6 / frameworks v0.3 milestone Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-objc2 Affects the `objc2`, `objc2-exception-helper` and/or `objc2-encode` crates enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants