fix(extract): give Go build-constrained twin files distinct QNs - #1946
fix(extract): give Go build-constrained twin files distinct QNs#1946ilyabrykau-orca wants to merge 1 commit into
Conversation
|
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. |
|
CI retrigger (no-op amend): |
d4c7f01 to
dcbb7fd
Compare
|
Field census landed (isolated
The call graph grew while twins were being disambiguated — recovered twin targets more than offset the twin-pair-ambiguity fail-closed cases, so the minimal-scope worry (losing edges from unconstrained callers) did not materialize at repo scale. |
DeusData
added
bug
|
Thank you for the measured build-constraint twin analysis and for keeping constraint-aware resolution outside this initial draft. Qualified-name changes affect extraction, joins, and compatibility across the graph, so we need more time to review the design and failure modes carefully. Please keep it in draft; we will come back with a grounded decision as soon as capacity allows. |
Go build-constrained twin files (//go:build lines, GOOS/GOARCH filename suffixes) legally define the same symbols in one package. The indexer ignored the constraints, so the graph upsert kept ONE node per QN by the smallest-file-path rule: a 2-line stub beat the 63-line real implementation and took all 25 inbound CALLS; a bpf2go GOARCH twin pair lost 39 of 40 nodes; parse coverage stayed clean so nothing flagged it. Fold the per-file build constraint tau into Go func/method QNs with the same #-suffix move Rust cfg twins got in DeusData#495: tau is the compacted //go:build expression when present, else the official GOOS/GOARCH filename suffix (cbm_go_build_tau in helpers.c, computed once per file in cbm_extract_file). The scope builder mirrors the def formula (go_tau_scope_qn) so body calls in constrained files stay attributed. Types and vars stay plain, keeping parent_class / DEFINES_METHOD joins intact. Two #-aware seams make the suffixed defs resolvable (both also close a latent gap for Rust cfg twins, which already carry such QNs): - cbm_registry_add indexes the simple name as the part before '#' (identifiers cannot contain one), so callers keep resolving by name. - The LSP registry's exact pkg.name lookup falls back to a SOLE #-suffixed variant (lookup_func_sole_tau_variant) - the dominant real-world shape is a constrained file with no in-tree twin, and its callers must keep their lsp_direct/lsp_strategy_cross_file edges. Two or more variants are genuinely ambiguous without the caller's build configuration and fail closed; tau-aware preference for the caller's own constraint is the declared follow-up. The pipeline's LSP-join leaf gate learns the same rule. Reproduce-first tests: extract_go_buildtag_tau_in_func_qns (//go:build compaction, GOOS_GOARCH and _test filename forms, types stay plain, unconstrained files carry no '#') and pipeline_go_buildtag_twins_both_survive (RED on main: count_nodes_named == 1, expected 2; plus the sole-variant caller edge staying alive). 854 tests green across extraction/pipeline/registry/ lsp probes/go_lsp/parallel suites. Part of DeusData#1911 (minimal PR: QN disambiguation; tau-aware same-module resolution is the follow-up) Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
|
Rebased onto current main. Conflicts were test insertion-anchor drift in tests/test_extraction.c and tests/test_pipeline.c (re-anchored, plus the count_nodes_named forward declaration my new test now needs above its insertion point); all source files auto-merged, including registry.c around the merged #1944 changes. Full scripts/test.sh green on this head (isolated CBM_RUNTIME_DIR — the earlier local run collided with the live 0.10.8 session daemon, same class as #1952's venue findings). |
dcbb7fd to
4bfb074
Compare
2 participants
Part of #1911 — draft implementation-proposal: this changes what gets extracted (Go func/method QNs in build-constrained files), so per CONTRIBUTING it's opened as a draft for design review; the design is the one proposed in the issue body (the #495 Rust-cfg
#-suffix pattern). Will mark ready on a maintainer design ack.What
Build-constrained twin files (
//go:buildlines, GOOS/GOARCH filename suffixes) legally define the same symbols; the upsert kept ONE node per QN by the smallest-file-path rule — a 2-line stub beat the 63-line implementation and took all 25 inbound CALLS; a bpf2go GOARCH twin pair lost 39/40 nodes; coverage stayed silent.How
//go:buildexpression, else the official GOOS/GOARCH filename suffix (cbm_go_build_tau, computed once per file). Folded into func/method QNs (MirrorConfig#unix), mirrored on the call-scope side (go_tau_scope_qn) so body-call attribution stays exact. Types/vars stay plain — parent_class/DEFINES_METHODjoins keep working.#-aware seams make suffixed defs resolvable (both also close a latent gap for Rust cfg twins, whose QNs already carry#):cbm_registry_addindexes the simple name as the part before#(identifiers cannot contain one);pkg.namelookup falls back to a sole#-suffixed variant (lookup_func_sole_tau_variant) — the dominant real-world shape is a constrained file with no in-tree twin, and its callers keep theirlsp_direct/lsp_strategy_cross_fileedges. Two+ variants are genuinely ambiguous without the caller's build configuration and fail closed. The pipeline's LSP-join leaf gate learns the same rule.Scope (deliberately the issue's "minimal PR")
QN disambiguation only. τ-aware resolution (preferring the caller's own constraint, and
module.name#τsame-module keys) is the declared follow-up — unconstrained callers of a genuine twin pair currently resolve as ambiguous rather than binding an arbitrary twin, which is strictly better than today's bind-everyone-to-the-stub.Tests (reproduce-first)
extract_go_buildtag_tau_in_func_qns://go:build linux && amd64→#linux&&amd64,_windows_amd64.go→#windows_amd64,_linux_test.go→#linux, types stay plain, unconstrained files carry no#. RED on main.pipeline_go_buildtag_twins_both_survive: RED on main (count_nodes_named == 1, expected 2); GREEN with both twins as nodes AND a sole-variant caller edge staying alive (UseFlush → FlushDisk#linux,lsp_strategy_cross_file@ 0.92).scripts/test.shvenue leg green;git clang-format --diffclean.Field census
Landed (same-day baseline, repo @ f555e5ce): 2177 τ-suffixed func/method nodes across 372 constrained files; swallowed-twin files 319 → 281;
Closemethods 15 → 27; CALLS total +62 — recovered twin targets outweigh the fail-closed twin-pair ambiguity. Full table in the census comment below.#1932 tracks the family.