Skip to content

feat(lint): report a discarded destructive sequence result - #118

Merged
takeokunn merged 1 commit into
mainfrom
feat/lint-destructive-sequence
Aug 4, 2026
Merged

feat(lint): report a discarded destructive sequence result#118
takeokunn merged 1 commit into
mainfrom
feat/lint-destructive-sequence

Conversation

@takeokunn

Copy link
Copy Markdown
Collaborator

CLHS 17.1 lets sort and its relatives destroy the argument and use it to construct the result, so the caller must use the return value. (sort xs #'<) as a bare statement leaves xs as garbage.

rule Category Severity Fixability scope
discarded-destructive-sequence-result DeadCode Warning ReportOnly COMMON_LISP_ONLY

Heads are the 20 body forms with an implicit prognprogn prog prog* let let* flet labels macrolet symbol-macrolet lambda when unless dolist dotimes block with-open-file with-slots defun defmethod defmacro — not the destructive calls. See cost, below.

The premise held for 6 of 22 functions, and the check is the point

SBCL 2.6.0 already warns on eleven of them, unconditionally:

nreverse  =>  STYLE-WARNING: The return value of NREVERSE should not be discarded.
nreconc, delete, delete-if, delete-if-not, delete-duplicates,
nunion, nintersection, nset-difference, nset-exclusive-or, merge   — same

failure-p stays NIL; it is warnings-p that goes T. Worth recording: an early probe muffled the warnings and reported NIL, appearing to confirm the premise. Muffling falsifies warnings-p.

sort/stable-sort are the real gap, and their silence is conditional on type inference:

(defun f (xs) (sort xs #'<) xs)                        ; warn=NIL  silent
(defun f (xs) (declare (list xs))   (sort xs #'<) xs)  ; warn=T    STYLE-WARNING
(defun f (xs) (declare (vector xs)) (sort xs #'<) xs)  ; warn=NIL  silent

SBCL warns only once it can prove listness and picks the STABLE-SORT-LIST transform. Undeclared parameters — the common case — get nothing.

That last line also removes five heads: discarding (sort v #'<) on a vector is silent and harmless, since sorting a vector is genuinely in place. Same for nstring-upcase. Reporting them would be pure noise.

Net: 6 heads survive — silent under SBCL and able to return an object different from the argument: sort, stable-sort, nconc, nbutlast, nsublis, nsubst.

Measured, not assumed — the post-discard state:

call argument afterwards
(sort xs #'<) on (5 4 3 2 1) (4 5) — a two-element interior tail
(sort xs #'<) on (3 1 2) (1 2 3) — accidentally correct
(nreverse xs) on (1 2 3 4 5) (1) — the first cons

A second proposed rule was a true duplicate

lint-sequence/src/destructive_literal/rule.rs:18 already ships with 23 heads (a superset), per-head destroyed-argument indices, and tests asserting exactly the motivating examples — plus does_not_flag_a_variable_sequence pinning (sort xs #'<) as clean. Not built.

Near-misses read and cleared: copy_before_destructive.rs:34, destructive_function_naming.rs:33, macro_body_destroys_argument_form/domain.rs:117, unnecessary_sort_before_extremum_extraction.rs:82 (whose module doc argues against parent-walks on sort — the objection this design had to answer), sort_not_guaranteed_stable.rs:91.

The cost trap, caught by measurement

The first design anchored on sort and walked up via root_view(). 3.9 seconds on a 200-function fixture with zero findingsroot_view() materializes every node, and the correct idiom (setf xs (sort xs #'<)) passes any cheap head-and-argument test, so every correct call paid a full materialization.

Inverting the anchor to the body form makes the parent-child relation local. Release, load average 3.32:

8× ratio ns
cost-control-wrong-order 46 744,907,050 … 34,465,625,007
discarded-destructive-…-result 10 229,175 … 2,368,243
cost-control-shipped-local 7 31,543 … 244,800

3,250× at n=250 → 14,553× at n=2000; the factor grows because the shipped shape is linear and the rejected one quadratic.

Stated weakness: on a file where every call is a finding the rule is itself quadratic (5.37 s), since is_unevaluated_at runs per reporting body form. Recorded in cost_tests.rs rather than hidden — it is invisible on correct code, and the 28 MB audit sweeps in 0.69 s.

Corpus audit: 1619 files, and the zero is explained

files scanned : 1619 (898 SBCL + 721 Quicklisp), 31 unparsed, 28,378,755 bytes
body forms dispatched                  : 56731
destructive calls present              :   295
  on a bare variable         (cond 1)  :   122
  in a discarded slot        (cond 2)  :     1
  read by a later form       (cond 3)  :     0   <- FINDINGS

Condition 2 does the cutting — only 1 of 122 destructive calls on a variable sits in a discarding position. That near-miss is sbcl/src/code/globals.lisp:70, (nconc list (list (list symbol initform))), which is correct code: list is a header cons the function reads via (cdr list), so nconc mutates in place and discarding is deliberate. Condition 3 declined it — the only evidence that condition earns its place.

No real bugs found, and the honest reading is that for 11 of 22 functions SBCL warns, so those defects don't survive in SBCL-compiled code. What the audit establishes is a false-positive rate of 0 over 295 candidates, verified by a self-test plus an end-to-end plant into alexandria's control-flow.lisp.

Positions

Discarded = non-final form at or after body-start. Used = last child, or any argument of a plain call — which is why setf/push/return-from are structurally unreportable. Ambiguous, never reported = tagbody, loop, cond/case clauses, unwind-protect cleanups, prog1/prog2.

Mutation: 16 guards, all killed, 4 rounds

Every mutation was first verified to actually change the file — one regex silently no-op'd and reported a false "survived".

  • 4 live guards had no test, including the is_unevaluated_at suppression that no unit test could reach, because the test helper filters data before the rule sees it. It needed an engine-level test.
  • 3 were genuinely dead and removed, with the reasoning relocated to where it is reachable: subtree_mentions's is_bare_symbol call (unreachable — atom_text carries the reader prefix), discarded_range's start < last (an empty range is already inert), and value_is_discarded, a second spelling of a predicate the rule inlined.

Counts

RULE_COUNT 357 → 358, warning_count 254 → 255, catalogue 341 → 342, preset-filtered warnings 238 → 239.

cargo build --workspace / fmt --check / clippy --all-targets --all-features -D warnings / test --workspace all exit 0.

CLHS 17.1 lets `sort` and its relatives destroy the argument and use it
to construct the result, so the caller must use the return value. The
classic bug is `(sort xs #'<)` as a bare statement, after which `xs` is
garbage.

The premise held for only six of the twenty-two functions, and the check
is why. SBCL 2.6.0 already signals a `STYLE-WARNING` for eleven of them
-- `nreverse`, `nreconc`, `delete`, `delete-if`, `delete-if-not`,
`delete-duplicates`, `nunion`, `nintersection`, `nset-difference`,
`nset-exclusive-or` and `merge` -- so those defects do not survive in
code compiled daily. `failure-p` stays `NIL`; it is `warnings-p` that
goes `T`. An early probe muffled the warnings and reported `NIL`, which
is worth recording: muffling falsifies `warnings-p`.

`sort` and `stable-sort` are the real gap, and their silence depends on
type inference. SBCL warns once it can prove listness and picks the
`STABLE-SORT-LIST` transform, so a `(declare (list xs))` parameter warns
while an undeclared one -- the common case -- does not, and a
`(declare (vector xs))` one is silent because sorting a vector really is
in place.

That removes five more heads: discarding `(sort v #'<)` on a vector, or
`(nstring-upcase s)`, is silent *and* harmless, so reporting them would
be noise. What is left is the six that are both silent under SBCL and
able to return an object different from their argument: `sort`,
`stable-sort`, `nconc`, `nbutlast`, `nsublis`, `nsubst`.

The rule anchors on the twenty body forms with an implicit `progn`, not
on `sort`. Anchoring on the call and walking up cost 3.9 seconds on a
200-function fixture with no findings, because `root_view()` materializes
every node and the *correct* idiom `(setf xs (sort xs #'<))` passes any
cheap head-and-argument test -- so every correct call paid a full
materialization. From the body form the parent-child relation is local:
3,250x faster at n=250 and 14,553x at n=2000, the factor growing because
this shape is linear and the other quadratic. On a file where every call
is a finding the rule is itself quadratic, which is recorded in
`cost_tests.rs` rather than hidden; it is invisible on correct code, and
the 28 MB audit sweeps in 0.69 seconds.

A second proposed rule was dropped as a true duplicate: `lint-sequence`'s
`destructive_literal` already covers 23 heads with per-head
destroyed-argument indices, and its tests assert exactly the examples
that motivated it.

Audited over 1619 files (SBCL 2.6.0 plus 721 Quicklisp files, 28 MB): 0
findings, and the funnel explains the zero rather than leaving it
suspicious. 56,731 body forms dispatched, 295 destructive calls, 122 on a
bare variable, 1 in a discarding position, 0 read by a later form. The
near-miss is `sbcl/src/code/globals.lisp:70`, where `list` is a header
cons and `nconc` mutates in place, so discarding is deliberate -- the
last condition declined it, which is the only evidence that condition
earns its place.

Mutation: 16 guards, all killed, over four rounds. Every mutation was
first verified to change the file -- one regex silently no-op'd and
reported a false survivor. Four live guards had no test, including the
`is_unevaluated_at` suppression that no unit test could reach, because
the test helper filters data before the rule sees it; it needed an
engine-level test. Three guards were genuinely dead and were removed with
the reasoning recorded where it is reachable.
@takeokunn
takeokunn force-pushed the feat/lint-destructive-sequence branch from 7f190d8 to d074bc0 Compare August 4, 2026 03:45
@takeokunn
takeokunn merged commit 2a8a668 into main Aug 4, 2026
9 of 10 checks passed
@takeokunn
takeokunn deleted the feat/lint-destructive-sequence branch August 4, 2026 03:57
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