Skip to content

lint: the compiles of one run share their modules - #4342

Merged
borisbat merged 5 commits into
masterfrom
bbatkin/lint-shared-modules
Oct 23, 2026
Merged

borisbat merged 5 commits into
masterfrom
bbatkin/lint-shared-modules

Conversation

@borisbat

Copy link
Copy Markdown
Collaborator

Rebuild required: CodeOfPolicies gains a trailing field and DAS_POLICIES_VERSION moves to 4 - a host built on earlier headers fails the ABI stamp at its first compile until it rebuilds.

Why. The lint compiled every file cold: no module cache under its policy set, and the shared modules neither promoted nor reused, since the runner set ignore_shared_modules beside no_optimizations. A dasLLAMA file paid its whole graph each time (18 s for the Vulkan classes file, 60 s for the resident llama test), and the module's lint walk ran past 94 minutes on one core.

What changes.

  • CodeOfPolicies.promote_shared_unoptimized lets a compile under no_optimizations promote its shared dependencies as an optimizing compile would, for a process that compiles many programs under one policy set and runs none of them. The field sits last, and DAS_POLICIES_VERSION moves to 4 because the bool lands in padding the ABI stamp's size cannot see; the simulate checklist gains that duty.
  • The lint sets it by default: every shared module compiles once per process and each later file reuses it. The files lint in require order (a module ahead of every file that requires it), read through dastest's das_requires and sorted by daslib/algorithm's topological_sort; a parallel run orders each worker's chunk the same way. --no-shared-modules compiles every file's modules in its own group, as before.
  • Every status line carries the count of modules the compile took promoted (PASS file (promoted 17)), builtin left out since it promotes under every policy.
  • --exclude <path> (repeatable) leaves a folder out of a run: the directory walk, the runner's folder rules and a positional root under it.
  • CI's chunked check_lint_tree leaves modules/dasLLAMA out; the new check_lint_dasllama target lints that module as one process, in the extended checks' core lane set.

Observable behavior.

  • six dasLLAMA files, same findings: 88.7 s cold -> 46.0 s shared; twelve files (the CUDA home among them): 175.8 s -> 61.1 s
  • the whole module, 493 files: a cold serial walk stopped at 94 min -> 2 min 16 s, peak working set 1.9 GB
  • one check_lint_tree chunk (33 files of the tree's 4145, uncached): 16 s -> 3 s, peak working set 247 MB -> 237 MB
  • the result cache works as before: a clean, unchanged file reads CACHED on the next run
  • the serializer version moves to 233, so a module cache written before this change is refused and rebuilt

Where to look. The promotion site in src/ast/ast_parse.cpp and the policy's paragraph in src/ast/ARCHITECTURE_SHARED.md; require_order in utils/lint/main.das, whose name mapping (a require names a file by the file's name) is the one place a wrong order could come from.

Validation, claims, ledger

Validation

  • The measurements above are one box (zen2, Windows); the lint's own tests (utils/lint/tests/lint_shared_modules.das) hold the mode's contract: the same findings with sharing and without, the cache hit on the next run, a requirer's promoted count past a file that requires nothing of the set, the module ahead of its users by name and by file path, a two-worker run, the excluded folder never linted. The result-cache tests are unchanged in substance; both files now share the _lint_child.das fixture.

  • The review audits ran under a five-agent cap: one review-md-auditor over the six checklists the diff touches instead of one per checklist. The TDD audit ran no mutation controls (the harness refused its capped wrapper) and probed the promoted count instead; its finding that builtin always counts is fixed here, with the test comparing against a file that requires nothing.

  • The external codex round ran on the first shape of the branch and found three defects, all fixed here: the require scan missed the guarded (?flag) and file-path require forms (the scan is now dastest's das_requires), the policy field sat mid-struct against the simulate checklist's offset rule, and the test did not observe the reuse. No second round on the fixed tip.

  • --jit-check-abi for a cross target: not run - no cross-target bundle is built on this box. The field is a trailing bool in a padding pocket, so the struct's size and every earlier member's offset are the same on both C++ ABIs; the version bump is what a stale host trips on.

  • The ABI-break sweep (skills/internal/abi_break_sweep.md): no symbol is removed, renamed or re-typed, so an external module compiles unchanged; a prebuilt host must rebuild, which the stamp enforces loudly.

  • The utils-tests lane reads red on this box for two reasons outside the diff: msbuild parses the MCP setup test's own error : line (the .mcp.json move in a temp dir) as a failure, and the detect-dupe suite prints FAIL for its deliberately broken fixture inside a passing run (98 of 98); every suite of the lane reports all tests passed.

  • The interpreter and JIT lanes pass every assertion (15235 of 15247 and 15137 of 15150, the rest skips) and read red only on preflight's local 60 s per-file budget: tests/watchdog/test_watchdog.das takes over 70 s alone on this box with every test passing (a ledgered item; CI carries no cap), and tests/jit_tests/jit_lib.das overran under the box load of a Sphinx build beside the lane. The AOT lane passes.

  • The docs lane's das2rst panic was this diff's: the new policy field needed its line in the handmade CodeOfPolicies page, which das2rst validates positionally. With it, das2rst and Sphinx build clean; the stale-digest and toctree findings were downstream of that first panic.

Claims - stated, not tested

  • The require order maps a require by its last path segment to the file of the set carrying that name; two files of one set with the same name order by the first seen, and a cycle that mapping could fake would stop the run on topological_sort's panic. No such pair exists in dasLLAMA, and the whole-module run and a tree-sweep chunk both order clean.
  • The --exclude check at the four folder-rule walkers (docs, basenames, test cells, arch citations) is exercised by the whole-tree form, not by a test: the test's excluded folder carries no documents.

Not done

  • Four older lint tests (lint025_rule_docs_only, lint026_arch_citations, lint027_rule_doc_lines, lint033_rule_doc_names) keep their own copies of the child-runner helpers the new _lint_child.das fixture holds; they fold onto it on their next touch.
  • The tree sweep's chunk size (33 files at 128 chunks) caps how much a chunk shares; a coarser cut would share more.
  • A per-module shared cache across processes and the JIT of the macro modules are the next levers on compile time.

…every file cold - no module cache under a compile-only policy set, the shared modules neither promoted nor reused since the runner set ignore_shared_modules beside no_optimizations - so a dasLLAMA file paid its whole graph (18 s for the Vulkan classes file, 60 s for the resident llama test) and the module walk ran near an hour on one core. CodeOfPolicies gains promote_shared_unoptimized: a compile under no_optimizations that sets it promotes its shared dependencies as an optimizing compile would, for a process that compiles many programs under one policy set and runs none of them (src/ast/ARCHITECTURE_SHARED.md). The lint sets it by default and drops ignore_shared_modules, so every shared module compiles once per process and each later file reuses it; the files lint in require order - a module ahead of every file that requires it, since a module promoted before its own compile would collide with it - the require paths read through the review gates' das_requires and module_group_module_name (a require names a file by the file's name), the order daslib/algorithm's topological_sort; a parallel run orders each worker's chunk the same way; every status line carries the count of modules the compile took promoted, builtin left out since it promotes under every policy; the result cache works as before, since its entries hash the same module sources. --no-shared-modules compiles every file's modules in its own group, as the runner did. On this box, the same findings either way: six dasLLAMA files 88.7 s cold against 46.0 s shared, twelve (the CUDA home among them) 175.8 against 61.1, the whole module (493 files, 33 Metal skips) 2 min 16 s against a cold serial walk stopped at 94 minutes, at a peak working set of 1.9 GB; one of the tree sweep's 128 chunks (33 files) 16 s against 3 s at a peak of 247 against 237 MB. CI's chunked check_lint_tree leaves modules/dasLLAMA out, whose every file compiles the engine's whole graph, and the new check_lint_dasllama target lints that module as one process, in the extended checks' core lane set. --exclude <path> (repeatable) leaves a folder out of a run - the directory walk, the runner's folder rules and a positional root under it alike - the lever a whole-tree sweep needs to carve a module tree out by hand. The lint's own tests cover the mode and the flags (lint_shared_modules.das: the same findings with sharing and without and the cache hit on the next run, the promoted count past a file that requires nothing of the set, the module ahead of its users by name and by file path, a worker run at two workers, the excluded folder never linted) over a fixture the cache-family tests share (_lint_child.das: the planted tree under test_temp_dir, the lint child, the status and line readers), where each test carried its own copy. The policy field sits last in CodeOfPolicies, since a cached JIT DLL reads the members before it by offset, and DAS_POLICIES_VERSION moves to 4, since the appended bool lands in a padding pocket the ABI stamp's size cannot see - a host built on the earlier headers fails the stamp at its first compile instead of handing the library its padding; the simulate checklist gains that duty. The README and skills/perf_lint.md carry the flags, the handmade CodeOfPolicies page the field's line; the serializer version is 233, since CodeOfPolicies rides the module cache.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 22, 2026 22:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes the shared C++ CodeOfPolicies ABI, bumps the AST serializer/module-cache version, and alters the core compileDaScript module-promotion path, which are high-impact areas warranting final human review.

No open issues 🎉

What changed in this PR

This PR makes the lint runner share compiled shared modules across all files it processes in one run, instead of recompiling each file's full dependency graph cold. It introduces a new CodeOfPolicies.promote_shared_unoptimized policy that lets a no_optimizations compile promote its shared dependencies (as an optimizing compile does) for processes that compile many programs but run none. The lint enables this by default, lints files in require order (module before its requirers), reports a per-file promoted-module count, adds a repeatable --exclude flag, and splits modules/dasLLAMA into its own check_lint_dasllama CI target. Reported speedups are large (e.g. a 493-file dasLLAMA walk from ~94 min to ~2 min).

Changes:

  • New promote_shared_unoptimized policy field (ABI: DAS_POLICIES_VERSION→4, serializer→233) and a single promotion-gate change in ast_parse.cpp.
  • Lint runner shares modules by default, lints in topological require order, prints (promoted N), and gains --exclude and --no-shared-modules; shared test helpers extracted to _lint_child.das.
  • New check_lint_dasllama CMake/CI target; check_lint_tree excludes modules/dasLLAMA; docs/architecture/REVIEW updates.
File Description
include/​daScript/​simulate/​code_of_policies.h Adds trailing promote_shared_unoptimized bool; bumps DAS_POLICIES_VERSION to 4.
src/​ast/​ast_parse.cpp Gates module promotion on the new policy even under no_optimizations.
src/​builtin/​module_builtin_rtti.cpp Binds the new policy field for RTTI/scripts.
src/​builtin/​module_builtin_ast_serialize.cpp Serializes the new policy field in the module-cache key.
include/​daScript/​ast/​ast_serializer.h Bumps serializer version to 233 to invalidate stale caches.
include/​daScript/​simulate/​REVIEW.md Adds the rule requiring a DAS_POLICIES_VERSION bump for same-size layout/meaning changes.
src/​ast/​ARCHITECTURE_SHARED.md Documents the promote_shared_unoptimized override.
utils/​lint/​main.das Implements shared-modules mode, require ordering, promoted count, --exclude, and --no-shared-modules; refactors compile-failure classification.
utils/​lint/​tests/​_lint_child.das New shared fixture module for the lint child-runner helpers.
utils/​lint/​tests/​lint_shared_modules.das New tests for sharing, ordering, counts, --exclude, and parallel workers.
utils/​lint/​tests/​lint_result_cache.das Reuses the shared fixture; drops duplicated helpers.
utils/​lint/​README.md, skills/​perf_lint.md, doc/​.../​CodeOfPolicies.rst Document sharing mode, --exclude, and the new policy field.
utils/​CMakeLists.txt, CMakeLists.txt, ci/​CMakeLists.txt Add check_lint_dasllama, exclude dasLLAMA from check_lint_tree, wire dependencies and CI core set.

Review effort: Balanced

… its fixture. The test requires a fixture module declared shared under a %/ guard and references it only under static_if over builtin_module_exists; a lint run that already promoted the fixture answers that guard true, the reference counts, STYLE030 stays silent and LINT019 reports the nolint stale, which the CI tree chunk showed once the compiles of one run shared their modules. The nolint lists LINT019 for that mode, as the finding text names, and the lint README states the rule for a file whose code keys on builtin_module_exists of a shared module. Reproduced and verified on the five files in both modes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 22, 2026 22:35

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes a serialized C++ ABI struct, bumps the policy and serializer versions, and alters core shared-module promotion plus lint ordering, so cross-platform ABI and cache-invalidation effects warrant final human review.

No open issues 🎉

Review effort: Balanced

…it alone. The post-compile macros run over the process's global module list with no visibility guard, so once one compile of a run promoted daslib/ast_verify (the ast-fuzz self-test fixtures require it), its verify ran on every later program of the process - the CI tree chunk hit the self-test fixture that strips its own locations on purpose, which the verify then reported. The verify's post-compile pass now runs only where the program's own library holds the module, a require or --ast-verify's extra dependency; the fuzz tests still see every report under the flag, and a new cell lints the verify fixture ahead of the breaking one in one shared run. The lint README states the mechanism.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 22, 2026 23:06

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes a core C++ ABI struct (CodeOfPolicies) with version/serializer bumps and alters the compiler's shared-module promotion plus a process-wide ast_verify behavior, which are high-impact areas warranting human verification.

No open issues 🎉

Review effort: Balanced

…der the shared run lints files in is read off the sources' require lines, which spell requires the compiler never takes - a guarded one, a commented one - so daslib read as a cycle and topological_sort's panic took the whole run down (the bundle smoke's lint of the installed daslib). The walk is the runner's own Kahn's now: a node is free once nothing it waits on is left, and a cycle releases the lowest index left, since a true cycle compiles nowhere. The optional-require test suppresses STYLE030 for itself - every guarded require there is the test - in place of the per-line nolints the two modes read apart, and the comments the line-length gate and STYLE014 reported on the verify guard and the fuzz cell are trimmed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 00:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes core compiler shared-module promotion semantics and introduces a C++ ABI/policy change (version bumps to the policy stamp and AST serializer), which warrants human verification across the build matrix.

No open issues 🎉

1 previously missed issue

In code that hasn't changed since last review

Low severity Comment refers to a non-existent --shared-modules flag

utils/​lint/​main.das:36

This comment references a --shared-modules flag that does not exist. The runner's flag is --no-shared-modules (sharing is the default-on mode), and the field is populated whenever lint_shared_modules is true. Consider rewording to avoid pointing readers at a non-existent flag.

Review effort: Balanced

…s an engine's whole module graph cold - the dasLLAMA facade takes 56 s on a 3990X - against a 120 s timeout, and a timeout read as a plain compile failure, which the tool then reported as a binary gap, "build the missing modules and re-run", on a macOS runner that had every module. The probe timeout is 300 s, and a timeout names itself with the seconds.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 23, 2026 01:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔵 Needs a closer look

It changes a public ABI struct, the module-cache serializer version, and process-wide module-promotion and macro semantics, and the one test for the new ast_verify guard does not actually exercise it, so these cross-cutting changes need human verification.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Medium severity Test order never reaches the new verify guard

utils/​internal/​ast-fuzz/​test_ast_fuzz.das:123

This test does not actually exercise the new program_asked_for_verify guard, so it would pass even if the guard were removed. The lint runner sorts its input files alphabetically (collect_input_files ends with files |> sort), and require_order keeps files with no inter-dependency in that sorted order. Since at_unset.das sorts before verify_breaker.das, at_unset.das is linted first — before verify_breaker.das (the only file that requires daslib/ast_verify) promotes ast_verify into the process. The lint runner itself does not require ast_verify, so nothing promotes it before at_unset.das is compiled; the promoted-verify-reaching-a-non-requiring-program path is therefore never hit. To make the test meaningful, the ast_verify-requiring fixture must be linted first — e.g. rename verify_breaker.das to a name that sorts before at_unset.das (and update the reference here).

Low severity Custom topological sort duplicates existing algorithm

utils/​lint/​main.das:1390

release_order re-implements a topological sort that already exists as topological_sort in daslib/algorithm, which operates on the exact same node shape (a struct with id and before : table<int> — identical to RequireNode). The only behavioral difference is cycle handling: topological_sort panics on a cycle, whereas release_order tolerates one by releasing the lowest remaining index. Given the repository's one-implementation convention, consider either reusing topological_sort (and handling the false-cycle concern a different way) or documenting why a separate implementation is required. Note also that the PR description states the files are "sorted by daslib/algorithm's topological_sort", which does not match this custom implementation — one of the two should be reconciled.

Review effort: Balanced

@borisbat
borisbat merged commit da33cfc into master Oct 23, 2026
65 checks passed
@borisbat
borisbat deleted the bbatkin/lint-shared-modules branch October 23, 2026 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants