fix(grammar): stop the Swift try-bang entry shifting past its own type

[CaptainMittens]

One token in the same vendored file as #1977. This is the second of the two shifts in #1978, and the one that is undefined on Windows.

Stacked on #1977, because both changes rewrite the same recorded hash in scripts/vendored-checksums.txt. Two branches off main would collide there. Until #1977 merges this PR shows both commits; after it merges the diff narrows to the one token below.

The bug

internal/cbm/vendored/grammars/swift/scanner.c:131 builds the entry that suppresses the ! of a try!:

1UL << FAKE_TRY_BANG, // BANG,

FAKE_TRY_BANG is the last member of a 33-entry TokenType enum, so its value is 32.

unsigned long is 64 bits on Linux and macOS. On Windows it is 32 bits, so the shift count equals the width of the type, which is undefined. 1ULL is 64 bits on every target this project builds for.

Why nothing catches it

Three things line up, and the third is the one I would not have guessed:

1. The expression is a compile-time constant. The compiler folds it, so no sanitizer sees it at run time. UBSan cannot report what never executes.
2. Makefile.cbm:719 compiles every vendored grammar with -w. That switches off -Wshift-count-overflow, the one diagnostic that names this exact defect. The comment on line 718 gives the reason: upstream code has warnings.
3. The affected platform is not exotic. CLANG64 on windows-latest sits in CORE_WIN (.github/workflows/_test.yml:63), so it builds on every pull request and reports nothing.

What I did and did not verify

I have no Windows machine, so I did not observe a symptom in the product. What I did do is compile the same shape locally, using unsigned int because it is 32 bits everywhere:

uint64_t m = 1u << 32;

cc -Wall warns shift count >= width of type. Adding -w, as the grammar build does, silences it.

I expected the folded value to be 0 and said so in #1978 before testing. It is not. The variable is simply never assigned, and three runs of one binary printed 4338156640, 4364321888 and 4377183328; at -O2 it printed 8447164672. The correct form, (uint64_t)1 << 32, prints 4294967296 every time. So the practical effect on Windows is a suppressor entry holding whatever happens to be there, not a predictable zero.

No test, and why

A test here would pass before and after the change on every machine CI can run it on. unsigned long is already 64 bits on Linux and macOS, so the expression is correct there; the defect only exists on a target the test suite does not execute. Adding a test that cannot fail would suggest a guarantee that does not exist. The existing swift_force_unwrap_scanner_shift from #1977 already keeps a force-unwrap parsing at all.

Upstream

Not my own reading alone. alex-pinkus/tree-sitter-swift made the same one-token change in 6ab8d1d74ebd (2026-08-10). Our copy is pinned at 8abb3e8b3325 (2026-03-20), so that fix landed after our pin. I checked our copy byte-for-byte against upstream at that pin: it is a faithful vendor, not a local edit that drifted.

Re-vendoring picks up both fixes and five months of other work, but TOKEN_COUNT moves from 33 to 34 upstream, so the grammar itself changed. CONTRIBUTING.md:134 puts a vendored dependency change behind a design discussion, so I have not proposed it here.

Files

File	Change
internal/cbm/vendored/grammars/swift/scanner.c	1UL to 1ULL on line 131
scripts/vendored-checksums.txt	the one recorded hash for that file, as scripts/security-vendored.sh --update writes it

Layer 8 of the security gate compares vendored content against that manifest, so the edit and its recorded hash land together. scripts/security-vendored.sh exits 0 on this branch.

One thing worth your call

-w on the grammar build is what hid this. Turning on the single warning -Wshift-count-overflow for vendored grammars — one narrow diagnostic, not -Wall -Werror — would catch this class across all 104 vendored scanners without touching the rest of the noise. I have not made that change here. Say the word and I will open it as its own issue.

Fixes #1978

Refs #1892

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[DeusData]

Good catch, and the fix is right. One piece of paperwork is missing before it can land.

Please add a row to the "Local source patches" table in internal/cbm/vendored/grammars/MANIFEST.md. Any change to vendored sources has to be recorded there, because the vendoring table's claim is that our copy matches the pinned upstream commit — a local patch makes that claim false unless it is written down, and a future re-vendor would silently drop your fix.

You are in good company: three of the four existing rows are the same class of UBSan fix in a vendored scanner.

Suggested row, matching the existing shape:

| swift | `swift/scanner.c`, `OP_SYMBOL_SUPPRESSOR` + `eat_operators` | `1 <<` / `1UL <<` → `1ULL <<` | UBSan: `1 << suppressor` shifts an `int` by up to `TOKEN_COUNT` bits, undefined once the index reaches 31, while the mask it feeds is `uint64_t` |

Worth upstreaming too, if you have the appetite — alex-pinkus/tree-sitter-swift carries the same bug at the pinned commit, and an upstream fix means the next re-vendor drops the local patch instead of re-applying it.

On the change itself, two things I want to name because they are the reason this was easy to review:

You found a bug that the normal test build actively hides. Recovering UBSan prints the diagnostic and continues, so the shift has been executing and being ignored for as long as it has existed. Identifying that the Windows CLANGARM64 leg runs UBSan in trap mode — where the same shift is an illegal instruction rather than a log line — is what turns "a warning nobody reads" into "a crash on one platform".

And you wrote the test's own limitation into the test. "This test cannot go red here… parsing this file at all is the check" is exactly the right comment to leave. A future reader who sees it pass on macOS will not mistake that for evidence. That honesty is worth more than the four-character fix.

Add the manifest row and this is ready.

[CaptainMittens]

Manifest row added, and the branch is rebased onto current main.

The row

I used your shape and extended the reason to name both sites, since this PR fixes two:

| swift      | `swift/scanner.c`, `OP_SYMBOL_SUPPRESSOR` + `eat_operators` | `1UL <<` / `1 <<` → `1ULL <<` | UBSan: `1 << suppressor` shifts an `int` by up to `TOKEN_COUNT` bits, undefined once the index reaches 31, while the mask it feeds is `uint64_t`. `1UL << FAKE_TRY_BANG` is the same defect on Windows, where `unsigned long` is 32 bits and `FAKE_TRY_BANG` is 32; the CLANGARM64 leg runs UBSan in trap mode, so there it is an illegal instruction rather than a log line |

The commit touches a second file, and that is deliberate rather than scope creep. MANIFEST.md is itself listed in scripts/vendored-checksums.txt, so writing the row breaks its own recorded checksum unless that line moves with it.

The rebase, and what the conflict was

The branch was 44 commits behind and conflicting. The conflict was one line, and it was that same self-reference: MANIFEST.md's checksum, bumped on main by another patch row and bumped here by mine.

I resolved it by recomputing the hash from the merged file rather than taking either side. Both recorded values described a MANIFEST.md that no longer exists once both rows are present, so either choice would have left the file failing its own check while looking resolved.

bash scripts/security-vendored.sh passes on the rebased branch, and the sanitized build is clean:
test-runner extraction pipeline grammar_regression → 591 passed, no failures and no UBSan runtime reports, built with -fsanitize=address,undefined.

Upstreaming

Yes, and I have not done it yet — I will open it against alex-pinkus/tree-sitter-swift and link it here rather than leave you to guess whether it happened. If it lands, the next re-vendor drops this local patch instead of re-applying it, which is the better end state for a row like this.

On the two things you named

The UBSan point is the one I want to be honest about: I did not identify the trap-mode difference from insight. I hit the recovering-UBSan diagnostic while running an unrelated test, went looking for which leg treats it as fatal, and found the CLANGARM64 one. The finding was real; the route to it was ordinary.

The test comment was deliberate. A test that cannot fail on the machine most people run it on is worth less than it looks, and writing that down felt cheaper than having someone later mistake a green macOS run for coverage.

[CaptainMittens]

The MANIFEST row is in, and I have to correct two things I said.

The upstream fix is already upstream — I cannot open it

I said I would open this against alex-pinkus/tree-sitter-swift and link it here. I went to do that and found both changes already there, in commits that landed after our pinned revision:

Our change	Upstream commit	Landed	Their issue
eat_operators, 1 << → 1ULL <<	fb63a7004f07	2026-04-06	#558
OP_SYMBOL_SUPPRESSOR, 1UL << → 1ULL <<	6ab8d1d74ebd	2026-08-10	—

fb63a7004f07 is byte-identical to 3f831936 here, at the same line. We pin 8abb3e8b3325 (2026-03-20), which predates both. So this is a stale pin, not an unfixed upstream bug, and there is nothing for me to send. That retracts the offer you recorded on #1978 — please do not wait on it.

Both fixes still matter here. main today still carries 1UL << at line 131 and 1 << at line 514.

The row I first wrote would have misled a re-vendor

Your reason for wanting the row is that a re-vendor would otherwise silently drop the fix. That reason is exactly right for the other four rows, where upstream never took the change. It inverts for this one. The table says:

Re-vendoring from upstream must re-apply these.

Anyone following that on the Swift row after a re-vendor past 2026-08-10 would hand-apply a change the newer file already has.

So the row differs from your suggested wording in two ways. The reason column now names both upstream commits, our pin, and the dates, and says to delete this row on a re-vendor rather than re-apply it. The table intro gains one clause allowing that case, because it is a property of the table rather than of one row:

Re-vendoring from upstream must re-apply these, unless the reason column names an upstream commit that already carries the change — then drop the row instead.

The location and patch columns are unchanged from your suggestion.

What is not here

Re-vendoring the Swift grammar is the real end state and it is not in this pull request. TOKEN_COUNT goes 33 to 34 between our pin and upstream main, so the generated parser.c moves with the scanner. That is a bigger change and yours to schedule.

bash scripts/security-vendored.sh passes — MANIFEST.md is itself listed in scripts/vendored-checksums.txt, so its own recorded checksum moved with the edit.

I amended the MANIFEST commit rather than stacking a correction on it, so the branch is still three commits. The two code commits are untouched.