Repository navigation
docs: repoint the three source references the module split invalidated - #1197
Conversation
NVIDIA#1190 split several large files into module directories. Three places in the tree still point a reader at a path that no longer exists, and following any of them lands nowhere: * `cuda-oxide-codegen/tests/spine_kernel_ptx.rs` sends the reader to `mir-lower/src/convert/types.rs` for `is_kernel_func`, now `convert/types/func_abi.rs`; * `examples/ref_index_projections/README.md` and `examples/subslice_projection/README.md` both name `mir-importer/src/translator/rvalue.rs`, now the `translator/rvalue/` directory. Two of them needed more than a new path, because the sentence around the path had gone stale too: * the same doc comment cites `lowering.rs:132` for `propagate_kernel_attrs`, which is at line 336. A line number in prose rots on the next edit, so this names the function in its file instead of pinning a number. * `ref_index_projections` says the `Rvalue::Ref` arm "has five cases" in the present tense. It has two, and had five before the fix that README documents; the description is of the pre-fix code, so it now reads as past tense and points at where that arm and its address walk live today. `examples/atomics/README.md` also names `intrinsics/atomic.rs` and `ops/atomic.rs`, but those are crate-relative shorthand in a pipeline diagram and both resolve, so they are left alone. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
The NVIDIA#1190 module split deleted 16 monolithic files. PR NVIDIA#1197 repointed the three sites issue NVIDIA#1196 enumerated; this commit sweeps the rest, including bare-basename spellings the original sweep missed: - abi_hmm README table rows (convert/types.rs, ops/aggregate.rs) - book lowering-pipeline.md table rows (ops/memory.rs, ops/aggregate.rs) - mir-lower README (convert/generated_intrinsics.rs, now a directory) - translator/layout.rs + lowering_test/calls_and_values.rs comments (translator/rvalue.rs) - convert/ops/arithmetic.rs + ops/memory/tests/mod.rs comments (tests/lowering_test.rs, now a directory) - cargo-oxide README source tree (commands.rs, now a directory) - render/families/packed.rs comment (resolve.rs, now a directory) - translator/types.rs comment (place walker lives in rvalue/place_addr.rs) - intrinsics/indexing.rs doc (sreg dispatch lives in generated/sreg.rs) - cast_tests README pipeline diagram (rvalue.rs -> rvalue/expr.rs) Every new pointer was verified against the tree before writing it. Sweep now returns zero dangling references across full-path, crate-relative, and bare-basename spellings of all 16 deleted files. Signed-off-by: nihalpasham <nihalp@nvidia.com>
88572bd to
d34393d
Compare
|
Reviewed and finished this one off ourselves; we've been landing big refactors and the leftover work here traces straight back to them, so it felt wrong to bounce it back to you. Your three repoints: all verified correct
The gap: the sweep matched one spelling, the tree uses three Same split commit (40f03e1) deleted 16 files, and 14 more references to them survived in the other two spellings, two of them in the user-facing book. We stacked a commit on your branch repointing all 14 (your commit is untouched underneath), and re-swept full-path, crate-relative, and basename spellings of all 16 deleted files: zero dangles left. So the "Closes #1196" in the body is now genuinely true. Validation
We'll approve CI on the new head and land. One thought your PR itself proves: a shorthand-aware sweep is hard to do by hand, so if you still want to file that broken-path-check issue you offered, go for it. |
…t track NVIDIA#1190 split large files into module directories and left three prose references pointing at paths it had removed; NVIDIA#1197 repointed those and 14 more. Nothing catches that: a path in prose is compiled by nothing, `check-host-api-paths.sh` checks Rust API paths rather than files, and the book gate only builds. "In the repository" means tracked by git, not present on disk. Those are different questions and only the first is reproducible -- a path that exists as a local build artifact, or an untracked file someone happens to have, resolves for them and nobody else, so a filesystem test makes the guard's answer depend on whose checkout it ran in. Generated outputs are declared, never inferred. An earlier revision skipped any path whose parent directory held no tracked file, reasoning that such a directory must be an output location. That is backwards: deleting or misspelling a directory produces exactly the same signal as generating into one, so the broken reference this guard exists to catch was the case it let through. `GENERATED_OUTPUT_PATHS` is now the whole accommodation -- one entry, `crates/fuzzer/artifacts/summary.jsonl`, which `run_seed.py` writes -- and it is verified rather than trusted: an entry that becomes tracked, or that nothing refers to any more, is an error instead of a silent exemption. Roots are verified too. `docs/` was in the original list and holds no tracked file in this repository, so the pattern anchored there matched nothing and would have gone on matching nothing after a rename. The check that caught it runs before every sweep. Paths containing a traversal segment are reported rather than resolved, since they cannot name a tracked file relative to the repository root. The self-test runs the real scanner over four cases -- a missing file under a tracked directory, a missing file under a directory the repo does not have, a file that exists only in this checkout, and a tracked file -- and requires the first three to be reported and the fourth not. It takes the tracked example from the index rather than naming this script, which had made the self-test depend on the guard already being committed. Crate-relative and bare-basename spellings are still not checked. NVIDIA#1197's follow-up fixed 14 references written that way, so that is a real gap; telling `rvalue.rs` in prose from any other mention of it is guesswork, and a guard that guesses is worse than one that is narrow. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
…t track NVIDIA#1190 split large files into module directories and left three prose references pointing at paths it had removed; NVIDIA#1197 repointed those and 14 more. Nothing catches that: a path in prose is compiled by nothing, `check-host-api-paths.sh` checks Rust API paths rather than files, and the book gate only builds. "In the repository" means tracked by git, not present on disk. Those are different questions and only the first is reproducible -- a path that exists as a local build artifact, or an untracked file someone happens to have, resolves for them and nobody else, so a filesystem test makes the guard's answer depend on whose checkout it ran in. Generated outputs are declared, never inferred. An earlier revision skipped any path whose parent directory held no tracked file, reasoning that such a directory must be an output location. That is backwards: deleting or misspelling a directory produces exactly the same signal as generating into one, so the broken reference this guard exists to catch was the case it let through. `GENERATED_OUTPUT_PATHS` is now the whole accommodation -- one entry, `crates/fuzzer/artifacts/summary.jsonl`, which `run_seed.py` writes -- and it is verified rather than trusted: an entry that becomes tracked, or that nothing refers to any more, is an error instead of a silent exemption. Roots are verified too. `docs/` was in the original list and holds no tracked file in this repository, so the pattern anchored there matched nothing and would have gone on matching nothing after a rename. The check that caught it runs before every sweep. Paths containing a traversal segment are reported rather than resolved, since they cannot name a tracked file relative to the repository root. The self-test runs the real scanner over four cases -- a missing file under a tracked directory, a missing file under a directory the repo does not have, a file that exists only in this checkout, and a tracked file -- and requires the first three to be reported and the fourth not. It takes the tracked example from the index rather than naming this script, which had made the self-test depend on the guard already being committed. Crate-relative and bare-basename spellings are still not checked. NVIDIA#1197's follow-up fixed 14 references written that way, so that is a real gap; telling `rvalue.rs` in prose from any other mention of it is guesswork, and a guard that guesses is worse than one that is narrow. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
…t track NVIDIA#1190 split large files into module directories and left three prose references pointing at paths it had removed; NVIDIA#1197 repointed those and 14 more. Nothing catches that: a path in prose is compiled by nothing, `check-host-api-paths.sh` checks Rust API paths rather than files, and the book gate only builds. "In the repository" means tracked by git, not present on disk. Those are different questions and only the first is reproducible -- a path that exists as a local build artifact, or an untracked file someone happens to have, resolves for them and nobody else, so a filesystem test makes the guard's answer depend on whose checkout it ran in. Generated outputs are declared, never inferred. An earlier revision skipped any path whose parent directory held no tracked file, reasoning that such a directory must be an output location. That is backwards: deleting or misspelling a directory produces exactly the same signal as generating into one, so the broken reference this guard exists to catch was the case it let through. `GENERATED_OUTPUT_PATHS` is now the whole accommodation -- one entry, `crates/fuzzer/artifacts/summary.jsonl`, which `run_seed.py` writes -- and it is verified rather than trusted: an entry that becomes tracked, or that nothing refers to any more, is an error instead of a silent exemption. Roots are verified too. `docs/` was in the original list and holds no tracked file in this repository, so the pattern anchored there matched nothing and would have gone on matching nothing after a rename. The check that caught it runs before every sweep. A URL is not a repository path. The pattern would otherwise match the tail of any link ending in `.../crates/<something>.rs`, so a permalink into another repository, or into an old commit of this one, would be read as a claim about this tree. URLs are removed before matching, with one exception spelled as an exact literal rather than a pattern: `https://github.com/NVlabs/cuda-oxide/` `blob/main/` and its `tree/main/` form are this repository at its default branch, so what follows one of them is checked like any other path. Five links in the tree are written that way today. A traversal segment needs no rule of its own, and the one an earlier revision had is gone: `crates/../x.rs` is not a string git tracks, so the tracked test already reports it. Keeping the special case only changed the wording of a message, and no mutation of it could be observed -- which is the definition of a branch that is not carrying its weight. The self-test runs the real scanner over eight cases -- a file whose directory the repo tracks but which it does not, a file whose directory it does not track either, a file that exists only in this checkout, a tracked file, a link into another repository whose tail looks like a path here, both forms of a link into this repository at its default branch, and a traversal segment -- and fixes which must be reported and which must not. It takes the tracked example from the index rather than naming this script, which had made the self-test depend on the guard already being committed, and it writes the canonical URL out literally rather than building it from the constant, so that repointing the constant fails the test instead of moving it. Crate-relative and bare-basename spellings are still not checked. NVIDIA#1197's follow-up fixed 14 references written that way, so that is a real gap; telling `rvalue.rs` in prose from any other mention of it is guesswork, and a guard that guesses is worse than one that is narrow. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
…t track NVIDIA#1190 split large files into module directories and left three prose references pointing at paths it had removed; NVIDIA#1197 repointed those and 14 more. Nothing catches that: a path in prose is compiled by nothing, `check-host-api-paths.sh` checks Rust API paths rather than files, and the book gate only builds. "In the repository" means tracked by git, not present on disk. Those are different questions and only the first is reproducible -- a path that exists as a local build artifact, or an untracked file someone happens to have, resolves for them and nobody else, so a filesystem test makes the guard's answer depend on whose checkout it ran in. Generated outputs are declared, never inferred. An earlier revision skipped any path whose parent directory held no tracked file, reasoning that such a directory must be an output location. That is backwards: deleting or misspelling a directory produces exactly the same signal as generating into one, so the broken reference this guard exists to catch was the case it let through. `GENERATED_OUTPUT_PATHS` is now the whole accommodation -- one entry, `crates/fuzzer/artifacts/summary.jsonl`, which `run_seed.py` writes -- and it is verified rather than trusted: an entry that becomes tracked, or that nothing refers to any more, is an error instead of a silent exemption. Roots are verified too. `docs/` was in the original list and holds no tracked file in this repository, so the pattern anchored there matched nothing and would have gone on matching nothing after a rename. The check that caught it runs before every sweep. Paths containing a traversal segment are reported rather than resolved, since they cannot name a tracked file relative to the repository root. The self-test runs the real scanner over four cases -- a missing file under a tracked directory, a missing file under a directory the repo does not have, a file that exists only in this checkout, and a tracked file -- and requires the first three to be reported and the fourth not. It takes the tracked example from the index rather than naming this script, which had made the self-test depend on the guard already being committed. Crate-relative and bare-basename spellings are still not checked. NVIDIA#1197's follow-up fixed 14 references written that way, so that is a real gap; telling `rvalue.rs` in prose from any other mention of it is guesswork, and a guard that guesses is worse than one that is narrow. Signed-off-by: Vyncint Ng <115854244+vyncint@users.noreply.github.com>
Closes #1196.
#1190 split several large files into module directories. Three references still point at paths that no longer exist.
cuda-oxide-codegen/tests/spine_kernel_ptx.rsmir-lower/src/convert/types.rsconvert/types/func_abi.rsexamples/ref_index_projections/README.mdmir-importer/src/translator/rvalue.rstranslator/rvalue/expr.rs(+rvalue/place_addr.rsfor the address walk)examples/subslice_projection/README.mdmir-importer/src/translator/rvalue.rstranslator/rvalue/Two needed the sentence fixed, not just the path
Repointing alone would have left a reference that resolves and still misleads.
A line number that had also moved. The same doc comment cites
lowering.rs:132forpropagate_kernel_attrs; it is at line 336. Rather than swap one rotting number for another, this names the function in its file — the form that survives the next edit.A present-tense claim about code that changed.
ref_index_projections/README.mdsays theRvalue::Refarm "has five cases" and describes case 2's inner loop breaking on non-Fieldprojections. It has two labelled cases today; five is what it had before the fix that README documents. The passage is now past tense, and points at where that arm and its address walk live.Verified, not assumed
is_kernel_funcstill returnstrueiff the func op carries agpu_kernelStringAttr, andtranslate_place_addr_from_slot(named in the README) still exists, atrvalue/place_addr.rs:610.intrinsics/probes/removed.ll,intrinsics/overlay/test.toml), two paths thatcuda-intrinsics-genrender tests assert are absent, and themktempcanary incheck-reserved-prefixes.sh.Deliberately not changed
examples/atomics/README.mdnamesintrinsics/atomic.rsandops/atomic.rsin a pipeline diagram. Those are crate-relative shorthand and both resolve (crates/mir-importer/src/translator/terminator/intrinsics/atomic.rs,crates/dialect-nvvm/src/ops/atomic.rs), so rewriting them would add noise, not accuracy.I did not add a guard for this. A check that every path-shaped string in the docs exists would need an allowlist for the four synthetic fixtures above, and an allowlist is where that kind of guard goes soft. Happy to build one if you want it, but I would rather agree the shape with you than ship a check that can be green while wrong.
Test plan
cargo test -p cuda-oxide-codegen --test spine_kernel_ptx— 1 passed (the edited file is that test's module doc).cargo fmt --check— clean.scripts/check-spdx-headers.sh,scripts/check-book-api-names.sh— pass.