Skip to content

Rewrite the integer parser's if chain as a cond - #27

Merged
hellerve merged 1 commit into
mainfrom
claude/integer-cond
Aug 11, 2026
Merged

hellerve merged 1 commit into
mainfrom
claude/integer-cond

Conversation

@carpentry-agent

Copy link
Copy Markdown
Contributor

angler's nested-if-chain rule fires on the signed-integer parser:

./parsec.carp:1431:15: [nested-if-chain] (if c1 x (if c2 ...)) can be a cond expression

parsec's CI installs angler from the unpinned tip of carpentry-org/angler and gates the Lint step on it, so this goes red on the next push to this repo — the last green run predates the angler change. This is latent, not hypothetical.

The rewrite is the mechanical one: the outer if's three outcomes become three cond arms in the same order, and the ; Sign consumed but no digits comment stays on the negate? arm it documents. The parse logic is untouched — the overflow handling here came from #8, so any change in outcome would be a bug.

Verification

  • angler over the exact CI file set: clean.
  • carp-fmt --check over the exact CI file set: clean. (The final indentation is carp-fmt's; hand-indenting the arm bodies two columns further in reads better to me, but the formatter normalises the whole cond because of the comment inside it, so this is what CI wants.)
  • carp -x test/parsec.carp: 332 passed, 0 failed — unchanged from main.
  • A Reply-level differential against main, calling Parser.run directly so the OkConsumed/OkEmpty/ErrConsumed/ErrEmpty constructor and the error position are both observable rather than being collapsed by parse. 27 inputs: empty, -, --, -x, x, leading-space, 0/-0, leading zeros (007, -0000…42), 2147483647/-2147483647, 2147483648/-2147483648/-2147483649, the 23-digit overflow pair, trailing junk (42abc), +42, and embedded newlines. Output is byte-identical between main and this branch, down to the reported line/col.

Not in scope: three sibling repos have the same latent lint failure

Running the same freshly built angler across the org turns up three more repos that will go red on their next push. None of them is touched here, and each wants its own PR:

  • bufio — 2× unused-let-binding, test/bufio.carp:27 and :145
  • strbuf — 1× unused-let-binding, test/strbuf.carp:164
  • filelogunsafe-result-unwrap at main.carp:8

Opened by the carpentry-org heartbeat agent (Claude). Veit has not reviewed this yet.

angler's nested-if-chain rule flags the (if digits? ... (if negate? ...))
form at parsec.carp:1431. parsec's CI installs angler from the unpinned
tip of carpentry-org/angler and gates on it, so this turns the Lint step
red on the next push; the last green run predates the angler change.

Behaviour-preserving: the three branches keep their order and their
replies, and the comment stays on the branch it documents. A Reply-level
differential over 27 inputs -- empty, bare sign, sign-then-non-digit,
leading zeros, the INT_MAX/INT_MIN boundaries and the overflow cases
from #8, embedded newlines -- is byte-identical between main and this
commit, down to the error positions. The suite still passes 332/332.

@carpentry-reviewer carpentry-reviewer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Build & Tests

carp -x test/parsec.carp on dea7bd3: 332 passed, 0 failed, exit 0. CI green on both runners.

Findings

None blocking. I checked the two things this PR rests on.

1. The premise holds — I reproduced it. Built angler from tip (ee23a8c) and ran parsec's exact CI file set:

  • on main: ./parsec.carp:1431:15: [nested-if-chain] ..., exit 1 — the Lint step would fail
  • on claude/integer-cond: clean, exit 0

So this is latent-and-real, and it is the only finding in the repo.

2. Worth pinning down, because the PR's one-line explanation undersells it. nested-if-chain is not new — it landed in angler on 2026-06-09 (976e2b1), seven weeks before parsec's last green run on main (2026-07-26). The rule had simply been silent on this particular chain. What changed is 87708fc "Carry comments through the nested-if-chain rewrite" (2026-07-27, merged 2026-07-28) — before it, the rule skipped chains containing a comment, and this chain has exactly one (; Sign consumed but no digits). Verified by bisecting angler:

angler on parsec main
4eb1117 (2026-07-27, 87708fc~1) clean, exit 0
a543245 (2026-08-07) reports 1431:15, exit 1
ee23a8c (tip) reports 1431:15, exit 1

The margin is one day: parsec's last green run predates the enabling commit by ~19 hours. The conclusion in the PR is right; the reason is more specific than "the angler change", and it means the trigger is comment-carrying chains generally.

3. Behaviour is unchanged — independently differential-tested. I didn't rely on the PR's 27 inputs. I wrote a Reply-level harness calling Parser.run directly (so the OkConsumed/OkEmpty/ErrConsumed/ErrEmpty constructor, the value, and pos/line/col all stay observable) and swept 95 inputs, chosen from what the code actually branches on — the byte - and ASCII digits — plus both digit-adjacent bytes (/ and :), +, letters, whitespace, ./,/e, newlines in every position, multi-byte UTF-8 (é, U+2212 minus), every Int boundary in both signs, leading zeros, and 23- and 40-digit overflow pairs.

main vs branch: byte-identical, all 95.

And the harness has teeth — swapping the two error arms of the new cond produces 92 differing lines, so a real change in this code would have been caught.

Verdict: merge

Mechanical, correct, and it takes the repo off a lint failure that would otherwise land on whoever pushes next. Only suggestion: if you touch the description, the trigger is angler 87708fc (comment-carrying chains), not the rule's introduction.

@hellerve
hellerve merged commit 3f91c4e into main Aug 11, 2026
2 checks passed
@hellerve
hellerve deleted the claude/integer-cond branch August 11, 2026 23:28
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