Repository navigation
fix(recipes): retain includes and literals inside multiline attributes - #658
Conversation
There was a problem hiding this comment.
Verdict: CLEAN: No blocking defect found in the attribute input-discovery change.
Non-blocking
⚠ Cover the attribute-brace guard: tools/recipes.py:9633.
The fixture never exercises the new attr_depth == 0 guard at tools/recipes.py:1249; removing it leaves the fixture's scans unchanged.
Extend this test with a multiline attribute containing stringify!({ ... }) inside a #[cfg(test)] inline module, with its inner } aligned to the module declaration. Assert that inputs inside and after the attribute remain test-only. Removing the guard incorrectly moves those inputs into normal build inputs.
Checked
Duplicates: no functions, types, or constants added; existing include and literal scanners reused.
Complexity: _walk adds one brace-state condition.
Silent failure: unresolved includes still reach check/key errors; no new fallback introduced.
Tests: isolated in-memory walks retain both attribute inputs; restoring the early skip loses both. Removing the brace guard leaks test-only inputs in an adversarial fixture while leaving the existing fixture's scans unchanged.
Gates: no pins or thresholds moved; no cases removed.
Escape catalog: no C1–C4 patterns match. No inference hot path changed.
Notes
No Rust file changed; dryer and crapper did not run.
Validation used extracted walker code and in-memory fixtures. Full recipe-selection and recipes self-tests were not rerun.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
|
Addressed the non-blocking regression-coverage note in The existing module-selection fixture now puts an aligned standalone Verification: the new fixture passes with the production guard. Removing only |
There was a problem hiding this comment.
Since the last review: the attribute-brace regression request is fixed at tools/recipes.py:9688.
Verdict: CLEAN: The added regression detects the brace-state defect and preserves test-only input selection.
Checked
Duplicates: 0 new production functions, types or constants; the existing module-selection test is extended.
Complexity: module_gate_self_test gains fixture data and one assertion loop; production code is unchanged.
Silent failure: 0 new sites. Explicit set assertions reject missing inputs and inputs leaking into normal builds.
Tests: All 13 in-memory selection assertions pass. Removing the attr_depth == 0 guard at tools/recipes.py:1249 fails all three new assertions. Restoring the early attribute skip also fails the regression.
Gates: no pins or thresholds moved; no cases removed.
Escape catalog: no C1–C4 patterns match. No inference hot path changed.
Notes
No Rust file changed; dryer and crapper did not run.
Validation used production selection code with fixture I/O and justfile loading replaced in memory. The full recipes self-test suite was not rerun.
🔨 Shingler, bloomery's review bot: approves a clean pull request and requests changes on a must-fix (how it works)
|
@Burntgogi Thanks for the contribution. The review is done, so I'm merging this. |
Change
Refs #637; resolves only its multiline-attribute input-discovery item, not the other tracking items.
The Rust module walk scans includes and repository-path literals on inner attribute lines before skipping module/cfg interpretation. An attribute's inner brace cannot close the enclosing inline module. This uses the existing scanners and ownership partitions, with no second grammar, key relaxation or new cache.
The existing module-selection regression includes both an inner
include_str!and an inner repository-path literal. The contributor check documentation states this input contract.Verification
Base
69b66367; headabc846f.#[error(... include_str!("../../../outside.txt") ...)]: on unchanged main, changing the file changed program output fromoutside1tooutside2while the real key/ledger CLI still returnedskip. With this fix, the same edit returnsrun.2f1d9e3) integrated cleanly with recipes: key reached inputs and persist structural reach #587 (94ee6b2) in an isolated detached worktree. The same actual compiled consumer and fresh-process key/ledger scenario returnedrunwith the new reached-key implementation too.fmt,check-arch,check-comments, andcheck-leverspassed. The first concurrentcheck-recipesruns in two worktrees detected each other's real CPU-guard fixture processes; after restoring host isolation, the unchanged full check-recipes passed at2f1d9e34in 124 s. The 1,200-second bound and every check remain enabled.Review follow-up at
abc846fThe non-blocking attribute-brace coverage note is addressed without changing production code. The existing selection fixture now has a standalone aligned
}insidestringify!under a test-only inline module. Its inner include, inner path literal and following include remain test-only for the explicitly selected targets. Removing the guard via an in-memory AST mutant makes all three new cases fail by admitting ordinary build recipes.Actual Rust compilation accepts that same fixture both normally and with
--test; removing the test-only include files leaves the normal build green and makes the test build fail by filename. The final five-unit owning static run at committedabc846f8passed 5 / 5, wall 147 s (full check-recipes 127 s), with unchanged limits and no concurrent worktree check.