Skip to content

LINT028 and LINT029: argument mutation is a finding - #3910

Draft
aleksisch wants to merge 7 commits into
masterfrom
aleksisch/lint-arg-mutation
Draft

LINT028 and LINT029: argument mutation is a finding#3910
aleksisch wants to merge 7 commits into
masterfrom
aleksisch/lint-arg-mutation

Conversation

@aleksisch

@aleksisch aleksisch commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

Two new lint rules make argument mutation a finding, and the tree is converted to match them where the conversion is done.

LINT028 catches a by-value var argument that the body both writes and reads. The argument is silently a local: its writes never reach the caller, and after the first write the name no longer holds what the caller passed. The fix is a const parameter plus a local copy. This rule is complete across the tree, and it found two live bugs. In examples/games/river_run, drop_texture and drop_fbo zeroed a by-value copy, so the caller's GL handle stayed stale and a reinit could double-delete it. In modules/dasTerminal, the OSC8 hyperlink dedup state traveled by value through the checkpoint chain, so every row restarted from stale state and re-emitted escape sequences. Both are now by-reference.

LINT029 catches a mutated by-ref argument - a var array or table, or an explicit &. Functions communicate through return values; a call site where every argument is read-only is one a reader can reason about locally. Four shapes stay exempt: a returned parameter (that is how a non-copyable value moves out), a struct or handle receiver in any position (the method idiom), block and lambda parameters (a callback's slot is its caller's contract), and every daslib/ folder (library code lives on the builder/state idiom). A deliberate out-parameter keeps its var under // nolint:LINT029.

This is a draft because the LINT029 conversion is partial. The lint tooling, dastest, the LSP subtools, daspkg, das-herd, doc-verify and dasLLAMA's performance tools are converted and their suites pass. About 1270 findings remain, mostly in dasLLAMA, the GPU modules and examples.

Where to look: daslib/lint.das for both rules and the exemption arms, daslib/lint_config.das for lint029_source_exempt, and the three fixtures under utils/lint/tests/.

Validation, claims, ledger

Validation

  • The full pre-PR chain was NOT run - this is a draft opened on request, not a merge candidate. The branch is 42 commits behind origin/master and has not been rebased.
  • Lint gate is knowingly red: about 1270 LINT029 findings remain in unconverted areas. Zero compile errors tree-wide (3918 files swept).
  • Suites run and green: tests/lint (82), dastest/tests (30), tests/json (396), tests/linq (2071), tests/lsp (2), das-herd watcher (123), tests/fio, tests/match, ast-fuzz (16), dasweb-verify (14), dasTerminal semantics.
  • tests/sql_conformance cannot run here: dasSQLITE's native module is not built in this worktree (error 20605). Same for dasLLAMA and dasVulkan runtime tests - those sources lint and compile clean but were not executed.
  • Docs: das2rst regenerated clean (no stubs, no Uncategorized), sphinx build succeeded; no new warning touches the lint pages.

Claims - stated, not tested

  • LINT029 does not break ordinary builds. Verified by compiling files that carry findings: the [lint_macro] only arms in modules that require daslib/lint, so findings surface through the standalone runner and the CI lint lane. A break would look like an unrelated program failing to compile with error 50503.
  • The dasTerminal and river_run fixes restore intended behavior rather than change it. Verified by reading each caller: both callers already assumed the callee had written through. A break would look like a hyperlink re-emitted per row, or a GL handle freed twice.

Not done

  • About 1270 LINT029 findings remain: dasLLAMA (core, harness, benchmarks), dasImgui, dasVulkan, dasSpirv, dasMetal, examples, tutorials and parts of tests.
  • No rebase onto current master, and no full preflight, AOT build or AOT test run.
  • Pre-existing findings in modules/dasSMT (STYLE014/015/039, PERF030, LINT016/019) are untouched - that module is not part of this arc.

🤖 Generated with Claude Code

@aleksisch
aleksisch force-pushed the aleksisch/lint-arg-mutation branch 16 times, most recently from 53506c1 to 3f12cf3 Compare September 1, 2026 19:38
@aleksisch
the image probe unmaps through the one release, and the gate that say…
936da43 
…s so

ImgProbe's finalizer called fmap_close directly. For a chunk-backed
probe - anything image_from_carrier builds in memory - image_map points
at an image-page-aligned offset INTO a das-heap array<uint8>, so that
munmap succeeded and punched a hole in the process heap. The array
stayed alive in g_image_chunks, nothing complained, and the next large
malloc walked into the unmapped pages: dastest died with SIGSEGV at a
page-aligned address while building typeinfo for the NEXT test file.
One file per process hid it from CI, which is how it survived.

image_backing_release is the one release that tells a chunk from a
mapping, and REVIEW_IMAGE.md already called releasing a backing anywhere
else a defect. Nothing checked it, so REVIEW.das checks it now:
fmap_close is legal inside that release, or inside a function that
opened the mapping it closes, and nowhere else. The whole dasLLAMA suite
runs to the end instead of crashing.
@aleksisch
argument mutation goes away across the tree
18e12d3 
Functions that built their result through a mutable argument now return
it. Out-parameter clusters that traveled together became a returned
struct or a tuple; single collectors return their array or table; the
recursive walks in the lint runner, the AST-fuzz generator and the LSP
merge each subtree into the parent, which retired two shared dedup
tables - sorting already puts a nested root's repeats adjacent.

Two live bugs surfaced. river_run's drop_texture and drop_fbo cleared a
by-value copy, so the caller's GL handle stayed stale and a reinit could
double-delete it; they return the cleared handle now. dasTerminal's OSC8
hyperlink dedup state traveled by value through the checkpoint chain, so
every row restarted from stale state and re-emitted escape sequences.
doc-verify called mark_elisions for effect after it became a returning
function, silently marking nothing at three sites.
@aleksisch
LINT029: argument mutation is a finding
6d99e06 
LINT029 catches a mutated by-ref argument - a `var` array or table, or
an explicit `&`. Functions communicate through return values; a call
site whose arguments are all read-only is one a reader can settle
locally. Exempt: a returned parameter (how a non-copyable value moves
out), block and lambda parameters (a callback's slot is its caller's
contract), and every `daslib/` folder, where the builder/state idiom is
the library's own.

Struct parameters are not exempt: wrapping two flagged containers in a
one-field struct made the finding disappear without changing what the
function does, which is the dodge the rule exists to catch. A struct
whose every field access yields a pointer or a handle stays exempt - the
var is what keeps the CONTAINED handle non-const.

A by-value argument is left alone. Writing one cannot reach the caller,
so the parameter IS the local - forcing a copy into a temporary buys a
name and nothing else. LINT023 keeps the half that finds a real defect:
a by-value argument written and never read back, where the write is
simply dead.

A file that must decline a rule says so at the top:
options _nolint = "LINT029,PERF030" silences those codes for that file
only, layered last so a whole-file shape the rule cannot see has a
hatch. Unknown codes are ignored, so a typo silences nothing.
@aleksisch
das-herd returns its results, and a file can decline a rule
7956fea 
The 152 by-ref out-parameters under utils/internal/das-herd are gone:
argv builders and parsers return their arrays, GitCommandScan and
WorktreeOperationPlan are RETURNED rather than filled, scalar out-pairs
became named tuples, and the selection state machine takes its state and
hands it back. Seven survive as nolint - the jobque capture-release
contract and imgui_boost's per-widget storage, both of which need the
caller's own handle.

That conversion made PERF030 fire 103 times on a self-returning move,
where the callee hands back the same value it took, so the target is
already moved-from when the result lands. The rule cannot see that
shape. Rather than repeat one nolint per line, a module can now decline
a rule outright with `options _lint_disable = "CODE,CODE"`: it layers
last, over CLI flags and repo config, and both the [lint_macro] path and
the standalone runner honor it.
@aleksisch
the LSP subtools return their diagnostics
408bead 
validate.das requires daslib/lint, so the rule armed on the validator's
own source and it stopped compiling - the language server was dead for
every file it was asked about. Its three collectors now return what they
found: compile_error_diagnostics, lint_diagnostics and
collect_lint_diagnostics hand back arrays the caller appends.

nav.das follows: decl_ranges returns the (full, selection) pair it used
to write through two references, and doc_child / ws_symbol /
ws_function_symbols return the at-most-one node they used to push.
@aleksisch
eight dasImgui and dasLLAMA files return their results
bcd68d8 
The first files of the dasImgui and dasLLAMA sweeps, each finished
whole before the next was started: plot samplers renamed for what they
produce (sine_samples, bar_samples, histogram_samples), the grammar
canary and text-edit fixtures returning their rows and spans, and the
Metal shape helpers handing back what they used to write through an
argument.
@aleksisch
dasImgui and dasLLAMA: 240 argument mutations become returns
b76d9e7 
[skip ci]

dasImgui goes 615 -> 454. The icons module is the headline: IconCtx was
never written, only the ImGui draw list behind its pointer field, and
daslang propagates const from a struct into a pointer field's pointee -
so `var` was mandatory for a struct nobody mutated. Lifting the draw
list out into a first parameter across the seven primitives, 104 glyphs
and the dispatch cleared all 127 findings there, same calls in the same
order. Text flow, find, the edit providers and the markdown model return
what they used to fill.

dasLLAMA goes 856 -> 778, twelve files to zero. Tokenizer derivation,
the exchange validators, kernel access and the chat renderers return
their results; the validator tree merges each subtree's errors instead
of threading one list, and applies its cap once at the public entries.

The nolints name what pins them: a per-layer requant loop at
dasllama_blocks.das:1542, the tokenizer's merge heaps whose REVIEW.md
forbids superlinear scaling, and consumers in dasllama-ladder that
REVIEW_EXCHANGE.md fixes as the rails' gate.
@aleksisch
aleksisch force-pushed the aleksisch/lint-arg-mutation branch from 3f12cf3 to b76d9e7 Compare September 1, 2026 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@aleksisch