refactor: 2026 modernization pass — builtin macro migration, file splits, dead-code removal, test hygiene - #20
Merged
Merged
Conversation
…boundaries prover.lisp/tabling.lisp/data.lisp/collection.lisp/arithmetic.lisp/control.lisp were split into 8 new files along natural module seams (pure spec data vs. evaluator logic, static graph analysis vs. runtime dispatch, declaration registry vs. clause storage, sort/aggregate helpers vs. solutions machinery), each verified by exact top-level-form reconciliation against the originals and a full-text-multiset diff, so no form was lost, duplicated, or altered. prover.lisp's core CPS dispatch chain (the direct-dispatch lane documented in Serena memory architecture/rule_program_materialization_free_dispatch) was deliberately left untouched as one compilation unit; only the goal/module-resolution cluster it doesn't depend on was extracted ahead of it as src/goal-resolution.lisp. control.lisp also carries one define-iso-builtin migration (call_with_depth_limit/3) from the same session's macro-adoption pass, bundled here because it was applied sequentially on top of this file's split without an intermediate commit. cl-prolog.asd's :components list is reordered to respect the new load-order constraints this introduces (e.g. left-recursion-analysis.lisp and goal-resolution.lisp must precede prover.lisp).
Migrates 12 of the remaining hand-written argument-resolution preambles to the existing define-iso-builtin macro: atom_string, string_to_atom, number_string, term_string, sub_string, split_string (string.lisp), char_type, code_type (char-type.lisp, both using :raw to preserve the original's guard-then-resolve ordering), term_to_atom, read_term_from_atom (term-io.lisp), numlist, permutation (list-extra.lisp). Every :raw marking was checked individually against the original's resolution order so no argument that was previously resolved conditionally/lazily became eagerly resolved -- eager resolution has a real cost (a per-call environment index) that this project's prior performance-tuning pass specifically minimized. 21 other candidate sites were investigated and left as hand-written for concrete reasons recorded in this session's report: meta-goals that must stay unresolved for backtracking, branch-local preambles the macro cannot wrap, and one case (list.lisp length/2) where the hand-written code uses a shallow variable chase rather than the macro's deep resolution -- migrating would have silently changed behavior. Two sites (term-sorting.lisp sort/4, predsort/3) were attempted and reverted: they load before core.lisp, which defines the macro, an ASDF ordering constraint now recorded in Serena memory architecture/define_iso_builtin_load_order_constraint.
A dead-code sweep found this codebase already had almost none. Removed: - call-graph-p (callgraph/package.lisp export, callgraph/call-graph.lisp defstruct): auto-generated by defstruct with zero references anywhere outside its own export line, unlike every sibling callgraph export. Added (:predicate nil) so defstruct stops interning it at all, rather than just removing the export and leaving an unused internal function. - One compiler-proven-unreachable branch in text-conversion.lisp's %text-of (an empty-code-list case that %term-atom-p's earlier clause always intercepts first for NIL). - One unreachable branch in clause.lisp's %compile-rule-program (a not-consp check on a slot declared :type list, whose only other possible value is already caught by the preceding clause). A third candidate in term-compare.lisp's %compare-strings was investigated and deliberately NOT removed: the "unreachable" claim held for only one of its three call sites, a live NaN edge case in the other two could not be ruled out, and removing the arm would have violated %compare-terms's declaimed return type under (optimize (safety 1)). Recorded in Serena memory architecture/trichotomy_comparator_equality_arm_not_dead.
- README.md: 4 documentation links pointed at paths that don't exist under the current MkDocs site (installation/, quick-start/, api-reference/, architecture/ instead of /getting-started/, /guide/first-program/, /reference/api/, /reference/architecture/); the flake-input example still pinned v1.0.1 against a current v1.4.2 release. - docs/src/reference/architecture.md: the numbered load-order file inventory was stale twice over -- missing 8 pre-existing builtin modules, then made staler still by this session's own file-split work adding 8 more files to cl-prolog.asd. Regenerated to match the current :components list exactly (65 entries), and updated three prose bullets describing file responsibilities that the split changed. - docs/src/reference/callgraph.md: removed the now-nonexistent call-graph-p from the public API summary (see the paired dead-code removal commit). - docs/src/guide/builtin-goals.md: added the export-completeness caveat api.md already carries, for the predicate group where it was missing.
Confirmed via the GitHub compare API (v1.1.4...v1.2.0) that the only changes are cl-weave.asd/flake.nix/flake.lock -- no Lisp source changed, so cl-weave run and cl-weave:run-all are behaviorally identical. paredit-cli (v1.4.0) and cl-nix-forge (v0.5.0) were already at their latest tags. Note: cl-weave's newest tag has no attached GitHub Release, so `gh release list` alone reports v1.1.4 as latest; the tags API is what actually surfaces v1.2.0.
Converted ~40 longhand-repeated deftest groups to deftest-table/ deftest-queries/deftest-io-queries (the latter new, added to t/support/core.lisp alongside a :signals-condition table spec kind so class-checked assertions don't get silently downgraded to "any throw accepted"). Three groups were deliberately left un-collapsed because their cases are sequentially state-coupled (splitting them would make each case pass while testing nothing). Fixed a genuinely vacuous test: abolish-validates-predicate-indicators passed an unquoted loop variable to assert-query, which quotes its argument internally, so the test queried the literal atom GOAL every iteration rather than the intended terms -- and passed only because that atom reliably signals an existence-error. A full sweep of the remaining 325+ quoting-macro call sites found no other instance. Recorded in Serena memory testing/vacuous_assert_query_unquoted_goal. Removed 24 stray no-op top-level (progn ...) wrappers (merge artifacts, no semantic effect -- CL propagates top-level-ness through progn) and consolidated a duplicate signals-condition macro (33 call sites) into the existing signals-prolog-condition helper. Added coverage-closing tests for the largest gaps a fresh sb-cover run identified, concentrated in rulebase-compaction.lisp, builtins/format.lisp, term-compare.lisp, and text-conversion.lisp, verified with a non-vacuity control run (5 deliberately-wrong expectations produced exactly 5 failures, nothing else). Determined most of the remaining gap is structurally uncoverable by sb-cover -- declaration forms, defstruct slot defaults, and inline function bodies whose out-of-line definition is never entered even when called -- rather than a real test gap; recorded in Serena memory testing/sb_cover_structurally_uncoverable_expressions. Suite: 1732 -> 1765 passing, 0 failed/errored throughout. Coverage: 95.40% -> 95.92% expression, 92.40% -> 93.39% branch.
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
prover.lisp/tabling.lisp/data.lisp/collection.lisp/arithmetic.lisp/control.lispinto 8 new files along data/logic and responsibility boundaries, each verified by exact top-level-form reconciliation against the originals.define-iso-builtinmacro; documents and deliberately leaves 21 sites hand-written for concrete reasons (meta-goals, branch-local preambles, one ASDF load-order constraint).call-graph-p) and two compiler-proven-unreachable branches; investigates and deliberately keeps a third candidate that only looked dead from one of three call sites.cl-weaveto v1.2.0 (metadata-only change, confirmed via compare API).abolish-validates-predicate-indicators), removes 24 no-op wrapper forms, adds coverage-closing tests. Suite: 1732 → 1765 passing, coverage 95.40% → 95.92% expression / 92.40% → 93.39% branch.No public API additions or removals; all changes are internal refactors, dead-code removal, dependency bump, and doc/test fixes.
Test plan
nix flake check) green on this PR