Skip to content

[WIP] Fix compile error with wchar_t in System.Globalization.Native - #84

Draft
VSadov with Copilot wants to merge 1 commit into
mainfrom
copilot/cherry-pick-upstream-commit
Draft

[WIP] Fix compile error with wchar_t in System.Globalization.Native#84
VSadov with Copilot wants to merge 1 commit into
mainfrom
copilot/cherry-pick-upstream-commit

Conversation

Copilot AI commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Thanks for asking me to work on this. I will get started on it and keep this PR's description up to date as I form a plan and make progress.

Problem

Building on a recent macOS / Xcode toolchain (clang 20+) fails when compiling System.Globalization.Native:

In file included from src/native/libs/System.Globalization.Native/pal_idna.c:7:
In file included from src/native/libs/System.Globalization.Native/pal_icushim_internal.h:47:
/opt/homebrew/opt/icu4c/include/unicode/ustring.h:1203:8: error: identifier 'wchar_t' conflicts with a C++ keyword [-Werror,-Wc++-keyword]
 1203 | U_CAPI wchar_t* U_EXPORT2

Repro: ./build.sh clr -c Release --cross -a x64 on macOS arm64.

Newer clang added the -Wc++-keyword diagnostic. Because this native lib is built with -Weverything -Werror, the Homebrew ICU header's use of wchar_t becomes a hard error.

Requested fix

Satori is a fork of dotnet/runtime, so please cherry-pick the corresponding upstream dotnet/runtime commit(s) rather than hand-writing the change, so future merges from upstream stay clean.

  1. Locate the upstream dotnet/runtime commit that added add_compile_options(-Wno-c++-keyword) to src/native/libs/System.Globalization.Native/CMakeLists.txt (in the "Workaround for warnings produced by ICU headers" block). In current dotnet/runtime main the file looks like:
    # Workaround for warnings produced by ICU headers
    add_compile_options(-Wno-reserved-id-macro)
    add_compile_options(-Wno-documentation)
    add_compile_options(-Wno-documentation-unknown-command)
    add_compile_options(-Wno-reserved-identifier)
    add_compile_options(-Wno-c++-keyword)

You can add dotnet/runtime as a remote (git remote add upstream https://github.com/dotnet/runtime.git, fetch, then git log -S"-Wno-c++-keyword" -- src/native/libs/System.Globalization.Native/CMakeLists.txt) and git cherry-pick -x <sha> the relevant commit(s), resolving conflicts minimally.

  1. If network access to upstream is unavailable, or the cherry-pick is not cleanly applicable, fall back to applying the equivalent minimal edit yourself: add add_compile_options(-Wno-c++-keyword) to the ICU header workaround block in src/native/libs/System.Globalization.Native/CMakeLists.txt (Satori currently lacks this line). Note -Wno-unknown-warning-option is already set nearby, so older clangs are unaffected.

  2. Also check whether upstream applied the same suppression anywhere else that Satori is missing (e.g. src/native/libs/CMakeLists.txt), and include it only if needed.

Keep the change as small as possible. Mention the cherry-picked upstream SHA(s) in the PR description, or state explicitly if you had to fall back to a manual edit.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: I am building the PR bits with a command: ./build.sh clr -c Release --cross -a x64

it has been a long time since I built this repo. There was couple OS updates since then.

In file included from /Users/vs/Satori/Satori/src/native/libs/System.Globalization.Native/pal_idna.c:7:
In file included from /Users/vs/Satori/Satori/src/native/libs/System.Globalization.Native/pal_icushim_internal.h:47:
/opt/homebrew/opt/icu4c/include/unicode/ustring.h:1203:8: error: identifier 'wchar_t' conflicts with a C++ keyword [-Werror,-Wc++-keyword]
1203 | U_CAPI wchar_t* U_EXPORT2
| ^
/opt/homebrew/opt/icu4c/include/unicode/ustring.h:1204:12: error: identifier 'wchar_t' conflicts with a C++ keyword [-Werror,-Wc++-keyword]
1204 | u_strToWCS(wchar_t *dest,
| ^
/opt/homebrew/opt/icu4c/include/unicode/ustring.h:1236:20: error: identifier 'wchar_t' conflicts with a C++ keyword [-Werror,-Wc++-keyword]
1236 | const wchar_t *src,
| ^
I am getting compile errors:

Assistant: This is unrelated to PR #83 — it's a newer-Xcode/clang issue. Clang 20+ added -Wc++-keyword, and since System.Globalization.Native builds with -Weverything -Werror, the Homebrew ICU header's use of wchar_t now fails.

Upstream dotnet/runtime already fixed this by suppressing that warning in the globalization CMakeLists (it currently only lives in the wasm branch of src/native/libs/CMakeLists.txt in your tree). Apply the same one-liner:

     # Workaround for warnings produced by ICU headers
     add_compile_options(-Wno-reserved-id-macro)
     add_compile_options(-Wno-documentation)
     add_compile_options(-Wno-documentation-unknown-command)
     add_compile_options(-Wno-reserved-identifier)
+    add_compile_options(-Wno-c++-keyword)
 
     # Workaround for https://unicode-org.atlassian.net/browse/ICU-20601
     add_compile_options(-Wno-extra-semi-stmt)
     add_compile_options(-Wno-unknown-warning-option)

-Wno-unknown-warning-option is already there, so older clangs that don't know -Wc++-keyword won't complain.

Alternatives if you'd rather not touch the build: downgrade/pin an older Homebrew icu4c (the wchar_t decls in ustring.h are what trip it), or select an older Xcode via xcode-select. The CMake change is the cleaner, upstream-aligned fix.

Want me to open a PR with this change?

User: lets open a PR. ideally we would cherry pick the commits, so we could easier merge from upstream

Assistant: I've queued up the coding agent task to cherry-pick the upstream -Wno-c++-keyword fix — just waiting on your confirmation of the dialog to kick it off.

@VSadov

VSadov commented Aug 29, 2026

Copy link
Copy Markdown
Owner

@copilot are there actual file changes?

Copilot stopped work on behalf of VSadov due to an error August 30, 2026 20:41
Copilot AI requested a review from VSadov August 30, 2026 20:41
Copilot stopped work on behalf of VSadov due to an error August 31, 2026 20:41
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