Bump toolchains_llvm to 1.11.2, drop #791 archive_override

[zzcgumn]

Summary

• Bumps toolchains_llvm from 1.8.0 to 1.11.2 in MODULE.bazel.
• Drops the archive_override pinning an unreleased commit (merge of upstream #791, which dropped an unused -stdlib=libc++ flag that broke this project's Linux -Werror builds). That fix shipped in the 1.9.0 release — verified by diffing cc_toolchain_config.bzl at that tag against the pinned commit (identical) — so a plain bazel_dep pin replaces it.
• Confirmed llvm_versions = "21.1.8" (what this project pins for all three platforms) is still a catalogued archive at 1.11.2 (toolchain/distributions/github.jsonc).
• Confirmed the extra_link_flags/extra_link_libs mechanism this project relies on for the macOS 27 SDK / Linux native-linker overrides (see Release v3.1.1: fix macOS 27 SDK / Xcode 27 build break #417/Guard and document the macOS/Linux native-linker override and ThinLTO #418) is unchanged in cc_toolchain_config.bzl — still appended after the toolchain-default --ld-path, so "last flag wins" still holds.
• Deliberately not adopted here: 1.11.0 introduces a linker = {...} catalogue attribute (e.g. "darwin-aarch64": "auto") that's a more idiomatic way to express exactly the override this project hand-rolled with extra_link_flags — the release notes even call out "LLVM 23 on macOS still needs the native auto linker override for the known SDK/TAPI issue," which is our exact arm64e.x1 problem. Migrating to it is a worthwhile follow-up, but it's a mechanism change, not a side effect of a version bump, so I left it out of scope here.
• Updated ci_msan_test's guard: it now checks the bazel_dep version directly and that no override targets toolchains_llvm, using the same full-call-body regex approach as the apple_support guard in Bump apple_support to 2.10.1, drop Xcode 27 crosstool patches #421 (avoids the "module_name must be the next line" fragility flagged there).

Test plan

Run on this macOS 27 / Xcode 27 host — exactly the scenario the original native-linker fix targets:

• bazelisk test //python:ci_msan_test — red against the pre-bump 1.8.0 + override state (verified via git stash), green after
• bazelisk test //... — 104/104 pass
• bazelisk test --config=asan //library/tests/system/... — 22/22 pass
• bazelisk test --config=tsan and --config=ubsan //library/tests/system:worker_context_reuse_test — pass
• --config=msan is Linux-only and can't run on this host; ci_msan_test's config-wiring checks pass, and a --nobuild analysis probe of its toolchain fails identically before and after this bump (pre-existing macOS-host limitation, not a regression) — real msan verification happens on Linux CI for this PR

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

The new guard misses two valid Bazel override forms, allowing the protected dependency invariant to be bypassed.

1 open finding

• Reject all Bazel override forms for toolchains_llvm · New

What changed in this PR

Updates toolchains_llvm to a released version containing the required upstream fix and removes the temporary source override.

Changes:

• Bumps toolchains_llvm to 1.11.2 and refreshes the lockfile.
• Removes obsolete override documentation.
• Updates the MSAN configuration guard.

File	Description
MODULE.bazel	Updates the dependency and removes its archive override.
MODULE.bazel.lock	Records the updated dependency graph.
python/​tests/​ci_msan_test.py	Updates version and override validation.
docs/​BUILD_SYSTEM.md	Removes obsolete override guidance.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[zzcgumn]

Addressed in 0c9ec92: added `local_path_override` and `multiple_version_override` to the alternation in `_has_toolchains_llvm_override()`, plus a test for each. Confirmed both new tests fail against the pre-fix regex (standalone repro) before adding the fix.

This same gap exists in the identical `_has_apple_support_override()` helper added in #421 — fixing that one too to avoid the same finding there.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

The new version parser rejects valid reordered bazel_dep arguments.

0 open findings

1 resolved since last review

• Reject all Bazel override forms for toolchains_llvm

Previously missed (1)

In code that hasn't changed since last review

Parse bazel_dep fields independently of argument order

python/​tests/​ci_msan_test.py:30

This parser only recognizes name immediately followed by version. Reordering or multiline formatting the valid bazel_dep arguments makes this guard fail even though the dependency remains correctly pinned; the analogous helper in ci_windows_cppopts_test.py:169-176 parses the whole call body to avoid this. Parse each full bazel_dep(...) body and find both fields independently (and add a reordered-arguments case).

🧠 Review effort: Balanced

[zzcgumn]

Addressed in ccf2a1f: `_toolchains_llvm_bazel_dep_version()` now parses the whole `bazel_dep(...)` call body and finds `name`/`version` independently, matching `_rules_cc_bazel_dep_version()` in `ci_windows_cppopts_test.py`. Added a reordered-args test; confirmed it fails against the old pattern (standalone repro) before the fix.

24/24 `//python/...` tests pass.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

The new regex helpers can truncate valid Bazel calls and bypass or spuriously fail the regression guard.

1 open finding

• Regex truncates reordered bazel_dep calls at nested parentheses · New

🧠 Review effort: Balanced

[zzcgumn]

Addressed in 1a817e8. Both `_toolchains_llvm_bazel_dep_version()` and `_has_toolchains_llvm_override()` now use `_iter_call_bodies()`, a character-by-character scanner that tracks string-literal/comment state and paren depth, instead of `[^)]*)\)`. Added a test for each with a `)` inside a leading comment (e.g. `# see upstream fix (closes #791)`) before the real args; confirmed both fail against the old regex (standalone repro) before the fix.

24/24 `//python/...` tests pass.

Note: the same `[^)]*)\)` pattern — including in the `_rules_cc_bazel_dep_version`/`_rules_cc_override_version` helpers in `ci_windows_cppopts_test.py` that earlier rounds pointed to as the model to match — has this identical truncation issue. That file is outside this PR's diff, so I haven't touched it; flagging in case you want it fixed too, either here or separately.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

The new MODULE call scanner treats commented-out declarations as active configuration.

0 open findings

1 resolved since last review

• Regex truncates reordered bazel_dep calls at nested parentheses

Previously missed (1)

In code that hasn't changed since last review

Guard incorrectly matches commented-out MODULE declarations

python/​tests/​ci_msan_test.py:32

This pattern also matches call text inside comments, so the guard can inspect declarations that Bazel ignores. For example, a commented bazel_dep(... version = "9.9.9") before the real 1.8.0 dependency makes the minimum-version assertion pass, while a commented override makes _has_toolchains_llvm_override return true. Restrict matches to top-level, non-comment lines (and add a commented-call case) so this CI guard reflects the effective MODULE configuration.

🧠 Review effort: Balanced

[zzcgumn]

Addressed in 58d3db7. `_iter_call_bodies()` previously located each call via a plain regex search for `name(`, which matches that text inside a `#` comment just as readily as live code — confirmed both failure modes with a standalone repro: a commented-out `bazel_dep(... version = "9.9.9")` before the real dependency made the version guard read the dead value, and a commented-out override made `_has_toolchains_llvm_override` return a false positive.

Rewrote it as a single linear scan that tracks string/comment state continuously across the whole text, so a call name is only recognized outside both. Added a dedicated commented-out-call test for each helper.

24/24 `//python/...` tests pass. Will mirror this to #424 as well.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

Collection-based positional expressions can bypass the new override guard.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Record unresolved top-level list positional expressions

python/​tests/​ci_msan_test.py:280

A list expression can be the first positional module_name and resolve to the guarded module, but this branch descends without recording an unresolved positional value. For example, archive_override(["toolchains_llvm"][0], strip_prefix = "x") currently makes _has_toolchains_llvm_override() return False, bypassing the guard. Mark a top-level list expression unresolved just as the ( branch does.

This issue also appears on line 285 of the same file.

🧠 Review effort: Balanced

[zzcgumn]

Addressed the latest Copilot finding: list/dict positional expressions.

Same gap as the previous parenthesized-positional fix, for the "[" and "{" branches: a top-level list or dict expression opening the first positional argument (e.g. `["toolchains_llvm"][0]`) was descended into without ever recording `_POSITIONAL_KEY`, so `module_name` read as absent and `_has_toolchains_llvm_override()` missed the override. Reproduced directly against the Copilot-supplied example (returned `False`).

Fixed by applying the same "mark unresolved before descending" guard to both the `[` and `{` branches (the review noted the gap recurs at both), plus two regression tests — list and dict — and confirmed the sentinel actually fires in each case, and that a genuine literal positional value is unaffected (425e2fd).

24/24 `//python/...` tests pass.

🤖 Generated with Claude Code

[zzcgumn]

Proactively mirrored a fix from #424's identical `_iter_call_kwargs()`: the RHS lookahead after `ident =` only skipped whitespace, so a comment on its own line between `=` and the literal — valid Starlark formatting — made the RHS look non-literal. `module_name =\n # pin past a fix\n "apple_support"` was marked `_UNRESOLVED`, falsely tripping `_has_toolchains_llvm_override()` for an unrelated module's override, and the same shape on `bazel_dep`'s `version =` returned the sentinel instead of the actual version string.

Factored the comment-skipping loop already in `_is_complete_literal()` out into a shared `_skip_ws_and_comments()`, and used it for the RHS lookahead too (ed28e60). Mirrored two regression tests.

24/24 `//python/...` tests pass.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

The new parser can miss a valid Starlark override whose module name uses an escaped newline.

1 open finding

• Decode Starlark string continuations to prevent override guard bypass · New

🧠 Review effort: Balanced

[zzcgumn]

Proactively mirrored a fix from #424's identical `_iter_call_kwargs()`: the outer call-name scan only recognized a bare `name(` call, so a valid Starlark call through a parenthesized callable reference — `(archive_override)(module_name = "toolchains_llvm", ...)`, equivalent to the direct form since grouping parens around any expression are legal and the result can be called immediately — didn't match textually and let the override bypass `_has_toolchains_llvm_override()`.

Added `_match_parenthesized_call()` to recognize `"(" name ")("` as an alternate way to find the same call, wired into the outer scan alongside the existing direct-call match, plus two mirrored regression tests (29c5d11).

24/24 `//python/...` tests pass.

🤖 Generated with Claude Code

[zzcgumn]

Addressed the latest Copilot finding: string line continuations.

`decode_escape()` had no case for a backslash immediately followed by an actual LF/CRLF line break in the source (as opposed to the two-character `\n` escape for a newline value), so it fell through to the default "leave as literal backslash + next character" branch. Reproduced the example directly: `archive_override(module_name = "toolchains` + backslash + newline + `llvm", ...)` returned `False` before the fix (should be `True`, since Starlark drops the continuation entirely and the literal resolves to `toolchains_llvm`).

Added LF and CRLF line-continuation decoding, both collapsing to `""` (3dc70b2). Verified both forms resolve correctly, confirmed the distinct two-character `\n` escape (an actual newline value, not a continuation) still behaves differently and correctly, and the unaffected plain-literal case is unchanged. Two regression tests (LF, CRLF).

24/24 `//python/...` tests pass.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

The override scanner can miss a valid parenthesized override containing a comment.

0 open findings

1 resolved since last review

• Decode Starlark string continuations to prevent override guard bypass

Previously missed (1)

In code that hasn't changed since last review

Handle comments when scanning grouped callable overrides

python/​tests/​ci_msan_test.py:223

A valid grouped call can still bypass this guard when the grouping contains a comment, e.g. (archive_override # comment\n)(module_name = "toolchains_llvm", ...). Starlark ignores that comment, but _skip_ws stops at #, so this scanner returns False and permits the override. Skip comments around the grouped callable tokens as well, and add this case to the grouped-call regression test.

🧠 Review effort: Balanced

[zzcgumn]

Addressed the latest Copilot finding, plus proactively mirrored #424's raw-string fix.

Finding (comment in grouped call): `_match_parenthesized_call()` used `_skip_ws()` around the grouped callable name's tokens, so a comment inside the grouping — `(archive_override # note\n)(module_name = "toolchains_llvm", ...)`, which Starlark ignores identically to the plain `(name)(` form — made this scanner miss the call and let the override bypass the guard. Reproduced directly (returned `False`, should be `True`). Switched to `_skip_ws_and_comments()`.

Mirrored from #424: `module_name = r"rules_cc"` (a raw string literal) was read as a bare identifier `r` + a separate quoted string, falling through to the "unresolved" path and incorrectly failing closed for an unrelated module's override. Added `_is_raw_string_start()`/`_skip_raw_string()`/`_skip_any_string()` and wired them into the keyword-RHS and first-positional checks (dedcc23).

Verified both fixes directly (including that neither false-triggers for an unrelated module), plus the escaped-quote-inside-a-raw-string edge case resolves to its true value rather than matching. Six regression tests added (one for the comment fix, five for the raw-string fix, mirroring #424).

24/24 `//python/...` tests pass.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

The new scanner incorrectly rejects valid unrelated overrides using raw triple-quoted Starlark strings.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Handle raw triple-quoted strings in override scanner

python/​tests/​ci_msan_test.py:170

Starlark permits raw triple-quoted literals, but this scanner deliberately handles only single-quote delimiters. As a result, a valid unrelated override such as archive_override(module_name = r"""apple_support""", ...) is parsed as _UNRESOLVED, so _has_toolchains_llvm_override() returns true and fails this guard even though no LLVM override exists. Extend the raw-string scanner to recognize a three-quote delimiter (and add cases for both an unrelated override and the bazel_dep name).

🧠 Review effort: Balanced

[zzcgumn]

Addressed the latest Copilot finding (this is the same bug independently found on both PRs — the identical `_skip_raw_string()`): raw triple-quoted strings.

`module_name = r"""apple_support"""` stopped at the first lone quote inside the triple-quote run, leaving the value unresolved and making the fail-closed guard incorrectly return `True` for an override on a completely unrelated module. Extended `_skip_raw_string()` to match a triple-quote delimiter as the 3-character sequence, the same way `_skip_string()` already does for ordinary strings, keeping the raw `<quote>` escape rule (993d99c). Added both regression cases the finding asked for — an unrelated override and the `bazel_dep` name — plus one confirming `toolchains_llvm` via a raw triple-quote still matches.

24/24 `//python/...` tests pass.

🤖 Generated with Claude Code

[Copilot]

🔵 Needs a closer look

The override scanner misses valid nested callable grouping, allowing the prohibited override to bypass its guard.

0 open findings

Previously missed (1)

In code that hasn't changed since last review

Recursively unwrap grouped callables when detecting overrides

python/​tests/​ci_msan_test.py:353

This handles only one layer of callable grouping. Valid Starlark such as ((archive_override))(module_name = "toolchains_llvm", ...) therefore produces no match, so _has_toolchains_llvm_override() returns False and the new guard allows the override it is intended to reject. Recursively unwrap grouped callables (or use a Starlark parser) rather than special-casing exactly (name)(...).

🧠 Review effort: Balanced