Skip to content

fix: Xcode 26.4 build for runtime, inspector, and metadata-generator#376

Open
NathanWalker wants to merge 1 commit into
mainfrom
fix/xcode-26.4
Open

fix: Xcode 26.4 build for runtime, inspector, and metadata-generator#376
NathanWalker wants to merge 1 commit into
mainfrom
fix/xcode-26.4

Conversation

@NathanWalker
Copy link
Copy Markdown
Contributor

@NathanWalker NathanWalker commented May 22, 2026

metadata-generator/CMakeLists.txt: downgrade two diagnostics from
errors to warnings. Both fire inside LLVM 17.0.6's own headers on
newer Apple toolchains and have no fix on our side:

  • -Wunnecessary-virtual-specifier on virtual void anchor() inside
    classes declared final (e.g. clang/AST/Decl.h:151).
  • -Wpreferred-type-bitfield-enum-conversion on bitfield-to-enum
    stores in clang AST headers.

The suppressions are guarded by check_cxx_compiler_flag so older
toolchains that don't recognize the flag names skip the
add_compile_options call cleanly.

V8 JSI runtime

NativeScript/v8runtime/V8Runtime.cpp:
evaluatePreparedJavaScript was passing nullptr as the
std::string sourceURL argument. Clang 17's -Wnonnull rejects
implicit conversion from nullptr to the std::string(const char*)
constructor. The function body is unreachable in practice (the sibling
prepareJavaScript already returns nullptr), but it still has to
compile. Switch to "" — the conventional "unknown source URL"
sentinel.

V8 inspector — UChar / char_traits

libc++ in MacOSX26.4.sdk deprecates std::char_traits<T> for any
T other than the standard char family (char, wchar_t,
char8_t, char16_t, char32_t). String16 is backed by
std::basic_string<UChar> with using UChar = uint16_t;, so every
inclusion of <string> after string-16.h instantiates
char_traits<unsigned short> and the whole inspector translation
unit fails under -Werror,-Wdeprecated-declarations.
Switch UChar to char16_t, which makes std::basic_string<UChar>
resolve to the fully supported std::u16string. The V8 inspector
public API surface still passes uint16_t* over its interfaces, so
string-util.h now uses reinterpret_cast at exactly those
boundaries — both directions are bit-identical 16-bit codepoints, so
the cast is sound.
utils.mm::ToStdString previously built a std::vector<uint16_t>
and then copy-constructed a std::u16string from that vector's
iterators. That iterator-range constructor pulled the deprecated
char_traits<unsigned short> back in through the back door. Rewrite
to write char16_t directly into the destination std::u16string,
dropping the intermediate vector.
JsV8InspectorClient.mm: add a file-local Make8BitStringView
helper that builds a StringView over a std::string's storage as
an 8-bit (LATIN1/ASCII) view. Use it at the three sites that
previously inflated each ASCII/UTF-8 JSON byte into a uint16_t,
allocated a std::vector<uint16_t>, and then handed it to the
16-bit StringView constructor. The new path is shorter, allocates
nothing, and no longer touches the deprecated 16-bit string-traits
chain at all.

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Fixed compilation issues on newer Apple toolchains by adjusting compiler warning configurations.
    • Improved internal string handling for more reliable inspector protocol communications.
  • Performance

    • Optimized message conversion in the debugger inspector, reducing intermediate data structure allocations.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 22, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1917e78c-ecf3-4c6a-bc15-8ff7648647a7

📥 Commits

Reviewing files that changed from the base of the PR and between ecd3598 and 25832d3.

📒 Files selected for processing (6)
  • NativeScript/inspector/JsV8InspectorClient.mm
  • NativeScript/inspector/src/inspector/string-16.h
  • NativeScript/inspector/src/inspector/string-util.h
  • NativeScript/inspector/utils.mm
  • NativeScript/v8runtime/V8Runtime.cpp
  • metadata-generator/CMakeLists.txt

📝 Walkthrough

Walkthrough

This PR modernizes the v8_inspector string representation from uint16_t to char16_t, optimizes inspector message dispatch through direct 8-bit StringView construction, and addresses LLVM 17.0.6 build compatibility on newer Apple toolchains.

Changes

String Type System and Inspector Message Handling

Layer / File(s) Summary
String type modernization and UTF-16 conversion
NativeScript/inspector/src/inspector/string-16.h, NativeScript/inspector/src/inspector/string-util.h, NativeScript/inspector/utils.mm
UChar redefined to char16_t to map std::basic_string<UChar> to std::u16string in libc++. StringUtil functions updated with reinterpret casts at the API boundary. ToStdString refactored to build std::u16string element-by-element from StringView instead of intermediate std::vector<uint16_t>.
Inspector message handling with 8-bit StringView construction
NativeScript/inspector/JsV8InspectorClient.mm
Make8BitStringView helper added to construct V8 StringView directly over 8-bit std::string buffers. Applied in dispatchMessage for incoming protocol JSON and domain handler payloads, and in inspectorSendEventCallback for frontend notifications.

Build Configuration and Parameter Corrections

Layer / File(s) Summary
LLVM compatibility and V8Runtime parameter fix
metadata-generator/CMakeLists.txt, NativeScript/v8runtime/V8Runtime.cpp
CMake script adds CheckCXXCompilerFlag probe to conditionally downgrade -Werror for LLVM-specific warnings (preferred-type-bitfield-enum-conversion and unnecessary-virtual-specifier). V8Runtime::evaluatePreparedJavaScript changed from evaluateJavaScript(nullptr, nullptr) to evaluateJavaScript(nullptr, "") to use empty string as unknown sourceURL value.

🎯 3 (Moderate) | ⏱️ ~20 minutes

🐰 With char16_t now leading the way,
Our strings flow swift without delay,
Eight-bit paths skip the round-trip dance,
While LLVM warnings take a stance—
Inspector messages optimized today!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically summarizes the main objective of the PR: fixing build compatibility for Xcode 26.4 across three key components (runtime, inspector, metadata-generator).
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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.

1 participant