Repository navigation
fix(discover): keep make/cargo dep-info .d files out of the D grammar - #2421
Open
Fieldnote-Echo wants to merge 8 commits into
Open
Fieldnote-Echo wants to merge 8 commits into
Fieldnote-Echo wants to merge 8 commits into
Conversation
.d mapped to D unconditionally, so make-style dependency files written by cargo, rustc, gcc/clang -MD and CMake were parsed with the D grammar and walked at superlinear cost. Add cbm_disambiguate_d, a 4 KB first-line sniff in the shape of the existing .frm/.cls/.m disambiguators, that returns CBM_LANG_COUNT only for a make rule whose targets look like paths and keeps D on any doubt. A 200-file cargo dep-info corpus goes from 108.9 s to 1.27 s to index; 0 of 12,062 real D files are misclassified. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
Run the full pipeline over a cargo dep-info file alone under its own directory, and a gcc -MD file beside real D source. The dep-info files get no File or Module node, their directory chain gets no Folder node, and the D file keeps its File, Module and Folder. With the .d probe disabled the test fails with three Folder nodes under the dep-info directory. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
is_dep_rule_line took any '.' or '/' before the rule colon as a path, so
valid D such as "@1.0:" (a float UDA) and "public/**/:" (a comment between
tokens) was dropped as dep-info. A target holding '@' or a comment opener
("/*", "/+", "//") is now D.
Over 12,062 real D files and 4,494 real files under deps/ dirs, no
classification changes. A dep-info file whose target path contains '@' now
stays D, as it does on main.
Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
is_dep_rule_line split targets only on space, tab and CR, so valid D such as "import\fstd.stdio : writeln;" kept the keyword and the dotted module name in one path-like target and was dropped as dep-info. Vertical tab, form feed and the UTF-8 separators U+2028/U+2029 now split targets too. Over 12,062 real D files and 4,494 real files under deps/ dirs, no classification changes. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
2 tasks done
main moved file classification into cbm_language_classify (c5abb64), which discovery and the test-impact inventory share, so the .d hook in detect_file_language conflicted. discover.c is main's. The probe is now a content rule in language.c: cbm_language_probe_bytes asks 4 KB for a .d name that maps to D, and cbm_language_classify runs lang_d_text on those bytes. cbm_disambiguate_d reads raw bytes through lang_read_head, as the other probes do. Over 22,720 local .d files (10,408 dep-info, 12,312 D), classification is unchanged from 006f2a9, and the path probe, discovery and the byte classifier agree on every file. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
lang_probe_bytes_follow_the_name, lang_classify_matches_every_content_rule and the unreadable-content test now include .d: a 4 KB probe, a cargo dep-info line that is not indexed and D source that stays D, through both cbm_language_classify and cbm_disambiguate_d. With lang_probe_d disabled, the first two fail, as does discover_d_dep_info_not_indexed_as_dlang. Signed-off-by: Nelson Spence <nelson@projectnavi.ai>
This branch has not been deployed
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.
Fixes #2414
What does this PR do?
.dalways maps to D (src/discover/language.c:411), so make-style dep-info files from cargo/rustc, gcc/clang-MDand CMake go through the D grammar and the unified walk, which yields nothing and grows superlinearly with file size. I add a 4 KB first-line probe for.d(src/discover/language.c:1374-1473) as one more content rule incbm_language_classify, next to.m/.cls/.frm/.res(:1653-1658,:1728-1733), so discovery and the test-impact inventory classify.dthe same way. A.dfile is dropped only when its first non-blank line is a make rule whose targets all look like paths. Anything uncertain stays D, including a target that holds@or a comment, and a user override of.dskips the probe. A dropped file leaves no File or Module node, and a directory with nothing else indexed gets no Folder node.Measured at fd1b883:
.rsAt fd1b883 the graph was byte-identical on dlang/phobos and dlang/dmd. The later commits only keep more inputs as D: at 006f2a9 no classification changes over the same 12,062 D files and 4,494 real files under
deps/dirs. After main moved classification intocbm_language_classify(c5abb64), I merged main and moved the probe there (c461ed0): over 22,720 local.dfiles (10,408 dep-info, 12,312 D), classification is unchanged from 006f2a9, and the path probe, discovery and the byte classifier agree on every file. One trade-off: a dep-info file whose target path contains@stays D, as on main.Tests:
discover_d_dep_info_not_indexed_as_dlangfails on main.lang_d_dep_info_unsupportedcovers cargo, gcc, CMake, Windows paths, escaped spaces and multiple targets.lang_d_source_stays_dlangcovers D that must stay D, including float attributes, comments between tokens, and form feed, vertical tab, U+2028 and U+2029 between tokens.pipeline_d_dep_info_leaves_no_nodeschecks the graph and fails with the probe disabled.lang_probe_bytes_follow_the_nameandlang_classify_matches_every_content_rulenow include.d, and both fail with the rule disabled.Checklist
git commit -s) — required, CI rejectsunsigned commits (DCO, see CONTRIBUTING.md)
make -f Makefile.cbm test)(at 66ff5b8: 8962 passed, 9 skipped, 0 failed)
make -f Makefile.cbm lint-ci)(for this PR's changes: CI's lint job passes on this head, and local cppcheck 2.22 / clang-format 23 add no findings over main)
Prepared with AI assistance (exploration, reproduction, verification, implementation); the spec and design decisions are mine, and I reviewed and ran everything above.