Skip to content

Update MovableLock SPI implementation#870

Merged
Kyle-Ye merged 8 commits into
mainfrom
optimize/movablelock
May 23, 2026
Merged

Update MovableLock SPI implementation#870
Kyle-Ye merged 8 commits into
mainfrom
optimize/movablelock

Conversation

@Kyle-Ye
Copy link
Copy Markdown
Member

@Kyle-Ye Kyle-Ye commented May 23, 2026

Agent Summary

  • Update MovableLock SPI layout, naming, typed allocation flags, and Swift importer surface
  • Preserve public C symbol visibility while removing redundant export/refined-for-Swift annotations
  • Import _MovableLockCreate as MovableLock.init() and update call sites/tests

@github-actions github-actions Bot added the type: maintenance Refactor, cleanup, dependency bump, NFC, or internal maintenance. label May 23, 2026
@augmentcode
Copy link
Copy Markdown

augmentcode Bot commented May 23, 2026

🤖 Augment PR Summary

Summary: This PR refactors the MovableLock SPI to match the SwiftUICore 6.5.4 layout/symbol surface and updates Swift call sites accordingly.

Changes:

  • Updates shared build settings to enable typed allocation / typed new-delete flags on Darwin for both C and C++ targets.
  • Changes OpenSwiftUICore’s `Update` synchronization lock creation to use the imported `MovableLock()` initializer.
  • Renames and reshapes the MovableLock C implementation (field names, condition variables, bookkeeping flags) to align with the reverse‑engineered 6.5.4 behavior.
  • Makes internal helper functions `static` and adjusts wait/sync paths and condition signaling.
  • Updates the C header Swift importer surface: `_MovableLockCreate` is imported as `MovableLock.init()` and `isOuterMostOwner` becomes `isOutermostOwner`.
  • Updates SPI tests to use the new initializer and renamed property.

Technical Notes: The changes aim to preserve C symbol names while refining the Swift-facing API and matching observed assembly behavior.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode Bot left a comment

Choose a reason for hiding this comment

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

Review completed. 2 suggestions posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

pthread_mutex_lock(&lock->mutex);
while (lock->owner) {
while (lock->owner_thread) {
[[clang::noinline]]
Copy link
Copy Markdown

@augmentcode augmentcode Bot May 23, 2026

Choose a reason for hiding this comment

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

Sources/OpenSwiftUI_SPI/Util/MovableLock.c:68: [[clang::noinline]] is used as a standalone statement attribute here; depending on the C standard/toolchain this can be rejected or downgraded to a warning (which may become an error under -Werror). If the intent is to prevent inlining, it’s safer to ensure noinline is applied in a compiler-supported way for C (also applies to the other occurrences in this PR).

Severity: medium

Other Locations
  • Sources/OpenSwiftUI_SPI/Util/MovableLock.c:124
  • Sources/OpenSwiftUI_SPI/Util/MovableLock.c:169
  • Sources/OpenSwiftUI_SPI/Util/MovableLock.c:172

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

OPENSWIFTUI_EXPORT
OPENSWIFTUI_REFINED_FOR_SWIFT
MovableLock _MovableLockCreate(void) OPENSWIFTUI_SWIFT_NAME(MovableLock.create());
MovableLock _MovableLockCreate(void) OPENSWIFTUI_SWIFT_NAME(MovableLock.init());
Copy link
Copy Markdown

@augmentcode augmentcode Bot May 23, 2026

Choose a reason for hiding this comment

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

Sources/OpenSwiftUI_SPI/Util/MovableLock.h:33: Removing OPENSWIFTUI_EXPORT from these declarations may change DLL import/export behavior on Windows (OPENSWIFTUI_EXPORT maps to __declspec(dllexport/dllimport) there). If OpenSwiftUI_SPI is ever built as a Windows DLL, it’s worth double-checking that these C symbols remain exported as intended.

Severity: low

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 23, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.72%. Comparing base (773cb26) to head (e5a39d5).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #870      +/-   ##
==========================================
- Coverage   25.73%   25.72%   -0.02%     
==========================================
  Files         677      677              
  Lines       47008    47008              
==========================================
- Hits        12097    12091       -6     
- Misses      34911    34917       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Kyle-Ye Kyle-Ye changed the title Align MovableLock SPI implementation Update MovableLock SPI implementation May 23, 2026
@Kyle-Ye
Copy link
Copy Markdown
Member Author

Kyle-Ye commented May 23, 2026

/uitest

@Kyle-Ye Kyle-Ye merged commit ba736ae into main May 23, 2026
11 of 12 checks passed
@Kyle-Ye Kyle-Ye deleted the optimize/movablelock branch May 23, 2026 04:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: maintenance Refactor, cleanup, dependency bump, NFC, or internal maintenance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant