Skip to content

macOS: migrate from objc to objc2#232

Closed
paravozz wants to merge 1 commit intoRustAudio:masterfrom
paravozz:migrate/objc2-macros
Closed

macOS: migrate from objc to objc2#232
paravozz wants to merge 1 commit intoRustAudio:masterfrom
paravozz:migrate/objc2-macros

Conversation

@paravozz
Copy link
Copy Markdown

Summary

  • Replaces objc 0.2.7 runtime types and macros with objc2 0.6 across src/macos/ and src/gl/macos.rs. Keeps cocoa = "0.24" for framework type bindings (NSEvent, NSView, NSWindow, etc.); pure macros migration, no public-API or behavioral change.
  • Drops #![allow(unexpected_cfgs)] from both files (no longer needed: objc2's macros don't expand the buggy cargo-clippy cfg, so the warning that justified it goes silent at the source).
  • Resolves the unexpected_cfgs lint failure that was making CI fail under RUSTFLAGS=-D warnings. Related to macOS builds failing with "unexpected cfg condition value: cargo-clippy" #229; this PR is one of two paths discussed there (macros-only vs. full cocoa replacement) and intentionally takes the smaller-scope macros-only route to leave the cocoa replacement decision open. Supersedes ci: register cargo-clippy cfg to silence objc 0.2.7 macro warnings #231 (the build.rs workaround), same problem fixed at the source rather than masked.
  • Migration also surfaces and fixes several latent bugs in keyboard.rs and view.rs that objc 0.2's lax verification accepted but Apple's actual @encode requires; details below.

What changed

Direct replacements (mechanical):

  • objc::msg_send!objc2::msg_send! (~54 sites across view.rs, window.rs, keyboard.rs, gl/macos.rs)
  • objc::class!objc2::class! (3 sites)
  • objc::sel!objc2::sel! (26 sites)
  • objc::declare::ClassDeclobjc2::runtime::ClassBuilder
  • objc::runtime::Object / Class / Sel / Protocolobjc2::runtime::AnyObject / AnyClass / Sel / AnyProtocol
  • objc::{Encode, Encoding}objc2::{Encode, Encoding}
  • objc::runtime::objc_disposeClassPairobjc2::ffi::objc_disposeClassPair

Less-mechanical patterns required by objc2's stricter API:

  • Callback signatures use *const AnyObject / *mut AnyObject instead of &AnyObject / &mut AnyObject. objc2 0.6's MethodImplementation trait impls aren't HRT-compatible, and casting an elided-lifetime extern "C" fn(&AnyObject, ...) produces a for<'a> HRT type that fails the bound. Raw pointers carry no lifetime, dodge the issue, and faithfully match the objc_msgSend ABI.
  • Callbacks marked extern "C-unwind" rather than extern "C". Plain extern "C" is undefined behavior if a foreign exception unwinds through it (Itanium ABI on macOS Apple Silicon makes this concrete). objc2's own examples use extern "C-unwind" throughout.
  • bool is not objc2::Encode (objc2 docs: "bool [...] cannot be safely used in custom defined methods"), so BOOL-returning callbacks return objc2::runtime::Bool and use .as_bool() at comparison sites.
  • Local CgPoint / CgSize / CgRect #[repr(C)] wrappers for the geometric types used as msg_send! returns. cocoa's NSPoint / NSSize / NSRect are external types and can't implement objc2's external Encode trait (orphan rule), so the wrappers carry the impl. From conversions to/from cocoa types at the boundary.
  • ClassBuilder::new, class.add_ivar, and AnyClass::get / AnyProtocol::get take &CStr (was &str in objc 0.2). CString::new(...) for the runtime-built per-instance class name; CStr::from_bytes_with_nul(b"...\0").unwrap() for compile-time-known names (avoids c"..." literals which need edition 2021+).

Latent bugs surfaced and fixed

The migration's stricter unwinding (extern "C-unwind") and stricter @encode verification (debug_assertions mode) surfaced several bugs that have been latent in master for a while. I'm fixing them inside this PR rather than as separate PRs because they're directly entangled with the migration's stricter checks.

  1. -[NSEvent isARepeat] called eagerly on flagsChanged: events (keyboard.rs::process_native_event). Per Apple docs, isARepeat raises NSInternalInconsistencyException when sent to non-key events. The result was only used for event_type == NSKeyDown (&& short-circuit), but the call was unconditional. Under extern "C" + objc 0.2 the foreign exception's behavior on macOS aarch64 happened to be survivable; under extern "C-unwind" the unwind is well-defined and silently swallows the modifier-state KeyboardEvent for every Alt/Cmd/Shift press, breaking downstream modifier tracking (textbox consumers can't see Cmd+Backspace as cx.modifiers.logo()). Verified end-to-end: an instrumented trace showed flagsChanged ENTER firing then no follow-up; gating on event_type == NSKeyDown restores the modifier-press KeyboardEvent.

  2. -[NSEvent keyCode] called eagerly before event-type filter (keyboard.rs::process_native_event). Same exception class as isARepeat (Apple docs: "raises an NSInternalInconsistencyException if sent to non-key events.") The function already had a _ => return None arm acknowledging that AppKit occasionally dispatches non-key events into keyDown:/keyUp:/flagsChanged: selectors (NSAppKitDefined, NSSystemDefined, sync events around Cmd-Tab and input-source switches), but keyCode() was called before reaching that match, making the fallback dead code. Fix: hoist event-type filter above keyCode().

  3. NSArray::objectAtIndex(tracking_areas, 0) without count check (view.rs::update_tracking_areas). Raises NSRangeException if index >= count. The companion site in view_will_move_to_window already guards on count != 0; mirror the same guard here for defense-in-depth.

  4. Three return-type vs Apple @encode mismatches surfaced by objc2's debug-mode runtime verification:

    • view.rs::create_view: registerForDraggedTypes: declared @ (id), Apple's signature returns void (v). Fix: declare ().
    • window.rs::open_parented: addSubview: declared @, Apple returns void. Fix: declare ().
    • gl/macos.rs::create: retain declared (), Apple returns instancetype (@). Fix: declare *mut AnyObject.

    objc 0.2's msg_send! does not verify return-type encodings; objc2's does (under debug_assertions), so these mismatches that have shipped silently now panic on first debug run of cargo run --example open_window and cargo run --example render_femtovg.

Test plan

  • cargo build --workspace --all-targets --all-features with RUSTFLAGS=-D warnings clean
  • cargo fmt --all -- --check clean
  • cargo test --workspace --all-targets --all-features clean
  • cargo run --example open_window opens cleanly, dispatches mouse + keyboard events, modifier-state tracking works (Alt+G correctly synthesizes © with Modifiers(ALT), Alt release synthesizes Modifiers(0x0))
  • cargo run --example render_femtovg opens cleanly, OpenGL rendering through gl/macos.rs works, mouse events dispatch
  • DAW integration test against an in-tree consumer using Window::open_parented with Skia/Metal: plugin opens cleanly in Ableton Live + REAPER (with and without "Send all keyboard input to plug-in"); all keyboard/mouse/drag-drop functionality verified including modifier-held shortcuts (Cmd+Backspace, Alt+Backspace, Cmd+Left, etc.); pluginval --strictness-level 10 passes
  • cargo run --example open_parented panics in softbuffer/cg.rs:36:26 null pointer dereference. Confirmed unchanged from master (same panic on a clean checkout of the pre-migration code), so this is a pre-existing softbuffer-vs-baseview lifecycle issue, not a migration regression. Worth a separate issue/PR.

What's not in this PR

Intentionally minimal. cocoa is kept as the source of framework type bindings; cocoa::base::id is kept at the public surface for raw pointer crossings; Retained<T> and MainThreadMarker patterns are NOT introduced. A follow-up PR can replace cocoa with objc2-app-kit / objc2-foundation for typed framework methods, modern memory model, and current-Xcode SDK bindings, but that's a larger refactor with its own design discussion (and probably the right call to align on with @micahrj before pursuing).

Related to #229. Supersedes #231.

Replaces objc 0.2.7 runtime types and macros with objc2 0.6 across
src/macos/ and src/gl/macos.rs. Keeps cocoa = "0.24" for framework
type bindings (NSEvent, NSView, NSWindow, etc.); pure macros migration,
no public-API or behavioral change.

Drops #![allow(unexpected_cfgs)] from src/macos/mod.rs and
src/gl/macos.rs (no longer needed: objc2's macros don't expand the
buggy cargo-clippy cfg, so the warning that justified it goes silent
at the source). Resolves the unexpected_cfgs lint failure that was
making CI fail under RUSTFLAGS=-D warnings.

Direct replacements:
- objc::msg_send! -> objc2::msg_send! (~54 sites)
- objc::class! -> objc2::class! (3 sites)
- objc::sel! -> objc2::sel! (26 sites)
- objc::declare::ClassDecl -> objc2::runtime::ClassBuilder
- objc::runtime::Object/Class/Sel/Protocol -> objc2 AnyObject/AnyClass/Sel/AnyProtocol
- objc::{Encode, Encoding} -> objc2::{Encode, Encoding}
- objc::runtime::objc_disposeClassPair -> objc2::ffi::objc_disposeClassPair

Less-mechanical patterns required by objc2's stricter API:
- Callbacks use *const AnyObject / *mut AnyObject instead of
  &AnyObject / &mut AnyObject. objc2 0.6's MethodImplementation
  trait impls aren't HRT-compatible; raw pointers carry no
  lifetime, dodge the issue, and faithfully match the
  objc_msgSend ABI.
- Callbacks marked extern "C-unwind" rather than extern "C".
  Plain extern "C" is undefined behavior if a foreign exception
  unwinds through it.
- bool is not objc2::Encode, so BOOL-returning callbacks return
  objc2::runtime::Bool with .as_bool() at comparison sites.
- Local CgPoint/CgSize/CgRect #[repr(C)] wrappers for geometric
  types used as msg_send! returns, since cocoa's NSPoint/NSSize/NSRect
  are external types and can't impl objc2's external Encode trait.
- ClassBuilder::new and AnyClass::get / AnyProtocol::get take &CStr
  (was &str in objc 0.2).

Also fixes several latent bugs surfaced by objc2's stricter unwinding
(extern "C-unwind") and stricter @encode verification (debug_assertions):

- Gate -[NSEvent isARepeat] on event_type == NSKeyDown to avoid
  raising NSInternalInconsistencyException on flagsChanged events.
  Under extern "C" + objc 0.2 the foreign exception's behavior on
  macOS aarch64 happened to be survivable; under extern "C-unwind"
  the unwind silently swallows the modifier-state KeyboardEvent for
  every Alt/Cmd/Shift press, breaking downstream modifier tracking.

- Gate -[NSEvent keyCode] on event-type filter (mirrors isARepeat).
  Same exception class; the existing _ => return None arm in the
  event_type match was dead code because keyCode() ran before it.

- Guard NSArray::objectAtIndex in update_tracking_areas with a count
  check (NSRangeException) for defense-in-depth, mirroring the
  guard already present in view_will_move_to_window.

- Fix three return-type vs Apple @encode mismatches:
    view.rs registerForDraggedTypes: declared @ -> void
    window.rs addSubview: declared @ -> void
    gl/macos.rs retain declared () -> instancetype (@)

These four fix categories ship together with the migration because
they are entangled with the migration's stricter checks (the fixes
become observable only once objc2's verification runs).

Test plan, pattern rationale, and Apple doc citations in the PR
description.

Related to RustAudio#229. Supersedes RustAudio#231.
paravozz added a commit to paravozz/baseview that referenced this pull request Apr 25, 2026
Backports four fixes from the upstream objc2 migration PR
(RustAudio#232) so consumers of the integration/bitform
fork branch get them independently of the upstream review timeline.

These fixes are independent of the macros migration itself; they're
correctness improvements that apply equally to objc 0.2 and objc2,
just made visible by objc2's stricter unwinding and @encode
verification.

1. Gate -[NSEvent isARepeat] on event_type == NSKeyDown to avoid
   raising NSInternalInconsistencyException on flagsChanged events.
   Per Apple docs, isARepeat is only valid for keyDown/keyUp; calling
   it on flagsChanged raises and unwinds. Affected end-user behavior:
   modifier-state KeyboardEvents (Alt/Cmd/Shift press/release) were
   silently dropped, which broke downstream modifier tracking
   (e.g. textbox consumers couldn't see Cmd+Backspace as
   cx.modifiers.logo()).

2. Gate -[NSEvent keyCode] on event-type filter (mirrors isARepeat).
   Same exception class. The existing _ => return None arm in the
   match was dead code because keyCode() ran before reaching it.

3. Guard NSArray::objectAtIndex in update_tracking_areas with a count
   check (NSRangeException) for defense-in-depth, mirroring the guard
   already present in view_will_move_to_window.

4. Correct three return-type annotations to match Apple's @encode:
     view.rs registerForDraggedTypes: id -> ()  (Apple returns void)
     window.rs addSubview: id -> ()             (Apple returns void)
     gl/macos.rs retain: () -> id               (Apple returns instancetype)
   objc 0.2's msg_send! does not verify return-type encodings, so the
   wrong annotations have shipped silently; this corrects them.

Becomes redundant once RustAudio#232 merges and vizia's baseview pin moves
past it.
paravozz added a commit to paravozz/baseview that referenced this pull request Apr 25, 2026
…bitform)

Applies RustAudio#232 (objc2 macros migration + four latent-bug fixes) on top
of integration/bitform's existing four commits (keyboard-types bump,
RustAudio#226 NSTextInputClient, RustAudio#228 hitTest first-click, RustAudio#230 panic-fix).

Bitform's [patch.baseview] points at this branch via git source for
downstream consumption while RustAudio#232 is in upstream review.

Includes both the macros migration itself and the four latent-bug
fixes (isARepeat gate, keyCode hoist, tracking-areas guard, three
@encode mismatches), translated to apply on top of integration/bitform's
NSTextInputClient additions (which RustAudio#232's upstream-PR base does not
have, since RustAudio#232 is filed against upstream/master).

Replaces 5816b44 (was a fixes-only-no-migration attempt) since
that was the wrong shape; the integration branch needs the full
migration to align with RustAudio#232's eventual landing.

Becomes redundant when RustAudio#232 merges and vizia's baseview pin moves
past it.
@micahrj
Copy link
Copy Markdown
Member

micahrj commented Apr 25, 2026

Forgive me if my assumption is incorrect, but this looks like a largely LLM-generated PR. This is much too big and consequential a change for me to trust an LLM to do it, and properly reviewing it will take as much work or more than just making the change myself. I would also rather make a complete move from objc and cocoa to the objc2 ecosystem rather than doing it piecemeal like this. Since this change is not actually necessary to fix the failing CI builds (#233 is enough), I am going to close this PR.

@micahrj micahrj closed this Apr 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants