Skip to content

flag_cluster: fix the three findings, and pin the mordant pack by rev - #37

Merged
alii merged 3 commits into
masterfrom
flag-cluster-and-pin
Aug 13, 2026
Merged

flag_cluster: fix the three findings, and pin the mordant pack by rev#37
alii merged 3 commits into
masterfrom
flag-cluster-and-pin

Conversation

@alii

@alii alii commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

language CI went red with no commit in this repo. Cargo.toml pinned the mordant pack by git URL with no rev, so it tracked mordant master — and mordant#4 put flag_cluster there.

Three findings, three different right answers. -D warnings aborts dylint at the first failing crate, so the first was masking the other two: neither had ever been seen until it was fixed.

The threshold is 3 bools, not 2

flag_cluster.rs reads config.flag_cluster_min_bools.max(2), and MordantConfig defaults it to 3; the .max(2) floor only applies to a workspace with no dylint.toml at all. language has one.

That is load-bearing: a two-of-four collapse on Compiler would have cleared the lint without fixing anything, and it is why FmtArgs is fixed by collapsing three flags and leaving one.

vm::http::HeadFlags — 8 representable, 8 observable, 6 distinct

Connection is a token list and may be repeated, so close, keep-alive is a real wire state — but (close, keep_alive) and (close, ¬keep_alive) behave identically everywhere, so only 3 of the 4 are distinct.

Modelled the observation, named: enum ConnTokens { Neither, Close, KeepAlive, Both }, with accumulation across repeated fields an explicit union rather than two independent |=. Intent-modelling gives 6 states but changes the record's arity, needs three new AbiSlots behind a new public Persistence type in h1.scrl, and reverses a decision the code states twice ("raw findings, not decisions") — a stdlib API change riding on a CI-unbreak.

The RFC pointer in the ticket was off by a subsection: §9.1 is Establishment and says nothing about tokens; the opinion is §9.3 Persistence, which h1.scrl already cited correctly at should_close.

bytecode::compiler::Compiler — 2^4 = 16 → 16, argued suppression

No state dropped, because none is illegal. Every write site walked: check_only is construction-only, collect_hover_facts is a post-construction latch, retain_namespaces is scoped by with_retained_namespaces, walking_module_statements is mem::replace-scoped. No pair is exclusive.

And the all-true corner has a witness the ticket lacked: IncrementalSession::new_from_sourcenew_compiler(None, true), sets collect_hover_facts, then register_prelude runs under with_retained_namespaces, inside which analyse_module sets walking_module_statements. All four at once. All-false is an ordinary compile.

The argument lives in the code above the attribute, not in a baseline entry, so MORDANT_BASELINE_WRITE=1 cannot eat it. Supporting evidence: this struct's mode bools that did carry an invariant are already enums (UnusedBindings, ModuleScope) — the file's owners drew this line already, and these four sit on the other side of it.

FmtArgs — 2^4 = 16 → 8, and the lint was pointing at a live defect

--stdin --stdout parsed, and cmd_fmt's if args.stdin branch returned before ever reading args.stdout — the flag was silently ignored. path was excluded from --stdin by a clap attribute, not by the type.

clap's conflicts_with already excluded 6 of 16 at parse time; of the remaining 10, (stdout, stdin) in both debug settings behaved identically to stdin alone. So 8 were meaningfully distinct — 4 output modes × 2 debug — and the type now represents exactly those.

FmtTarget is Stdin or Files { path, action }, so path lives only in the arm where it is legal. derive(Args) cannot produce this, so Args/FromArgMatches are hand-written — the Command they build is what --help and clap_mangen render, and every flag keeps its text.

Behaviour change worth your judgement: --stdin --stdout now exits 2 with the argument '--stdin' cannot be used with '--stdout' rather than being accepted and ignored. Previously accepted; never did anything.

The pin

rev = "eba57c33...", verified it took — the run's DYLINT_METADATA names that rev and the checkout appeared. Carried over from Scarlet's pin block, including that rm -rf target/dylint is what makes a bump take. Deliberately not carried: Scarlet's sentence about xtask/src/check/mordant.rs catching that staleness — language has no such gate, so it is a manual step here.

The pin is now something somebody must move. t195-on-master (264bc0d) is already waiting on the other side of it.

Also required, and measured rather than predicted

scarlet_core gains [lints.rust] for unexpected_cfgs: cargo dylint sets --cfg dylint_lib="mordant" and plain cargo does not, so the cfg_attr made clippy -- -D warnings fail. A [workspace.lints] table would scale better but touches every manifest.

Plants

Suppression removed → rc=1 naming Compiler at mod.rs:411. main.rs reverted → rc=1 naming FmtArgs at main.rs:90. Old struct literal → E0560, four fields. Old HeadFlags field spelling → E0609, "available fields are: conn, expect_100_continue".

Every dylint run with rm -rf target/dylint, a touch, and a .rc deleted immediately before — one earlier read on this branch showed stale rc=0 files from a previous run and was discarded (T-83).

Gates

fmt 0 · clippy -D warnings 0 · test --workspace 0 (43 binaries / 1303 passed) · gen-editor-syntax 0 · SCARLET_GC_STRESS=1 0 · cargo dylint --all 0.

No suppression in the tree lacks an argument. The only one is Compiler's, and its argument is the eleven lines above it.

alii added 3 commits August 12, 2026 21:07
mordant's flag_cluster: 3 bools is 8 representable states. All 8 are
observable — Connection is a token list and may also be repeated, so a peer
can genuinely send `close, keep-alive` — but only 6 are distinct, because
RFC 9112 §9.3 gives `close` precedence and every reader collapses the pair.

Two ways to spend that difference, and the choice is which side of the parse
boundary the contradiction dies on.

Modelling the parsed intent resolves it here and leaves 3 x 2 = 6 states. It
also changes H1HeadFlags' arity, needs three new AbiSlots behind a new public
`Persistence` type in h1.scrl, and reverses a decision this code states in two
places ("raw findings, not decisions") — a stdlib API change riding on a
lint fix.

Modelling the observation keeps the ABI, the behaviour and that decision, and
still carries the invariant the convention was carrying: the two bools are one
header's token set, and neither half is separately assignable. ConnTokens
names its four cases, `Both` included, and accumulation across repeated
Connection fields is an explicit union instead of two independent `|=`.
should_close keeps the §9.3 precedence, where it was already correct.

Note the RFC pointer that sent me here was off by a subsection: 9112 §9.1 is
"Establishment" and says nothing about tokens. §9.3 "Persistence" is the one
with the opinion, and h1.scrl already cited it correctly.

No behaviour change. 43 binaries / 1303 passed, identical before and after,
and again under SCARLET_GC_STRESS=1.

Plant: restoring the old spelling `flags.conn_close |= has_token(...)` fails
to build, twice —

  error[E0609]: no field `conn_close` on type `HeadFlags`
     --> crates/scarlet_vm/src/vm/http.rs:380:19
      = note: available fields are: `conn`, `expect_100_continue`

which is the proof a passing test could not give: the suite passed before the
change too. The old spelling is now unwritable, not merely unwritten.

Does not turn CI green on its own. flag_cluster has two further pre-existing
findings that were masked, because -D warnings aborts at the first failing
crate: Compiler (16 states, T-278) and FmtArgs (16 states, T-279). Measured
extent is exactly those three and no other mordant finding at the pinned rev.
[workspace.metadata.dylint] carried no rev, so the gate tracked mordant
master. flag_cluster landing there turned this repo's CI red with no commit
here at all — three findings appeared in a tree that had not moved. That is
the whole argument for the pin, and it is not hypothetical: scarlet's own
Cargo.toml carries a longer version of this comment because an unpinned pack
accumulated seven checkouts and two libmordant dylibs on one machine, so the
same documented command ran a 10-lint pack in one worktree and a 12-lint pack
in another, minutes apart.

rev = eba57c339ebdfc7f2ce9a814507e028db0d21867, current mordant master.
Verified taking effect: DYLINT_METADATA in the run names that rev.

The cost, stated plainly because somebody now has to pay it: this line is a
thing that must be moved. A fix on mordant master no longer reaches this repo
until someone bumps it. One is already waiting — t195-on-master (264bc0d)
stops the baseline recording findings a normal run never emits, which matters
here because mordant-baseline.toml is checked in and currently empty.

Changing this line rebuilds nothing. With a library already built, moving the
rev and re-running cargo dylint loads the dylib already in target/dylint, so
the bump leaves every run using the old pack. rm -rf target/dylint is what
makes it take. scarlet has an xtask gate that catches that staleness; this
repo does not, so it is a manual step here.

Related, and the reason the verification below is not just "cargo dylint":
a warm cache returns rc=0 on a genuinely red tree, so the run needs a touch on
a source file first.
…FmtArgs

-D warnings aborts dylint at the first failing crate, so HeadFlags was
masking both of these. Neither had ever been seen until it was fixed.

The threshold is 3 bools, not 2. flag_cluster.rs reads
config.flag_cluster_min_bools.max(2), and MordantConfig defaults it to 3;
the .max(2) floor only applies to a workspace with no dylint.toml at all.
language has one. That matters: a two-of-four collapse on Compiler would
have cleared the lint without fixing anything, and is why FmtArgs is fixed
by collapsing three flags and leaving one.

Compiler: 2^4 = 16 -> 16. No state dropped because none is illegal, and the
all-true corner has a witness the ticket did not have -
IncrementalSession::new_from_source calls new_compiler(None, true), sets
collect_hover_facts, then register_prelude runs under
with_retained_namespaces, inside which analyse_module sets
walking_module_statements. All four at once. All-false is an ordinary
compile. Every write site was walked: construction-only, a latch, and two
scoped by with_/mem::replace. No pair is exclusive.

Suppressed with the argument in the code above the attribute, not in a
baseline entry, so MORDANT_BASELINE_WRITE=1 cannot eat it. This struct's
mode bools that DID carry an invariant are already enums (UnusedBindings,
ModuleScope), so the file's owners drew this line already.

FmtArgs: 2^4 = 16 -> 8. clap's conflicts_with already excluded 6 at parse
time; of the remaining 10, (stdout, stdin) in both debug settings behaved
identically to stdin alone. So 8 were distinct: 4 output modes x 2 debug.

And the lint was pointing at a live defect. `--stdin --stdout` parsed, and
cmd_fmt's `if args.stdin` branch returned before ever reading args.stdout -
the flag was silently ignored. path was excluded from --stdin by a clap
attribute, not by the type.

FmtTarget is Stdin or Files { path, action }, so path lives only in the arm
where it is legal. derive(Args) cannot produce this, so Args and
FromArgMatches are hand-written; the Command they build is what --help and
clap_mangen render, and every flag keeps its text.

BEHAVIOUR CHANGE: `--stdin --stdout` now exits 2 with "the argument
'--stdin' cannot be used with '--stdout'" rather than silently ignoring the
flag. Previously accepted, never did anything.

scarlet_core gains [lints.rust] for unexpected_cfgs: cargo dylint sets
--cfg dylint_lib="mordant" and plain cargo does not, so the cfg_attr made
clippy -D warnings fail. Measured, not predicted. A workspace.lints table
would scale better but touches every manifest.

Plants: suppression removed -> rc=1 naming Compiler at mod.rs:411. main.rs
reverted -> rc=1 naming FmtArgs at main.rs:90. Old struct literal -> E0560,
four fields. Each dylint run with rm -rf target/dylint, a touch, and a .rc
deleted immediately before.

fmt 0, clippy -D warnings 0, test 0 (43 binaries, 1303 passed),
gen-editor-syntax 0, GC_STRESS 0, dylint 0. CI is green.
@alii
alii force-pushed the flag-cluster-and-pin branch from fbe7814 to 90be4ca Compare August 13, 2026 04:09
@alii
alii merged commit cc2eb27 into master Aug 13, 2026
4 checks passed
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