Skip to content

fix(cpp): recover callables after preprocessor-gated signatures#949

Open
blankanswer wants to merge 2 commits into
DeusData:mainfrom
blankanswer:agent/fix-cpp-preproc-signature-gap
Open

fix(cpp): recover callables after preprocessor-gated signatures#949
blankanswer wants to merge 2 commits into
DeusData:mainfrom
blankanswer:agent/fix-cpp-preproc-signature-gap

Conversation

@blankanswer

Copy link
Copy Markdown

Summary

Fixes #946.

C++ files that gate alternate out-of-line method signatures with #ifdef / #else / #endif can confuse the raw tree-sitter parse and cause methods after the conditional block to disappear from the graph.

This PR extends the existing C/C++/CUDA preprocessor second pass so it can recover missing Function / Method definitions from the expanded AST, while keeping the normal raw-source extraction as the primary source of truth.

What Changed

  • Added a regression test for the SurfaceFlinger-style conditional signature shape.
  • Added a lightweight definition-walker entry point that does not emit a Module node or variables.
  • During the existing preprocessor second pass, merge only missing callable definitions from the expanded AST.
  • Deduplicate recovered callables by label + qualified_name.
  • Did not change vendored tree-sitter grammars.

Validation

  • make -f Makefile.cbm SANITIZE= build/c/test-runner build/c/codebase-memory-mcp
  • build/c/test-runner extraction — 211 passed
  • build/c/test-runner c_lsp — 750 passed
  • git diff --check
  • CLI smoke test with a temporary C++ repo:
    • index_repository
    • search_graph verified both commit and composite are indexed after the #ifdef/#else/#endif signature block.

Signed-off-by: Blank_Answer <97771966+blankanswer@users.noreply.github.com>
@blankanswer blankanswer requested a review from DeusData as a code owner July 8, 2026 04:38
Signed-off-by: Blank_Answer <97771966+blankanswer@users.noreply.github.com>
@DeusData DeusData added bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Jul 8, 2026
@DeusData DeusData mentioned this pull request Jul 8, 2026
2 tasks
@DeusData

DeusData commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Thanks for the focused C++ parser fix. Triage: parser recovery bug for #946, normal priority. Review focus is ownership of recovered callables, avoiding duplicate definitions across #ifdef branches, and keeping unrelated extraction behavior unchanged.

@DeusData DeusData added this to the 0.9.1-rc milestone Jul 8, 2026
@DeusData

DeusData commented Jul 9, 2026

Copy link
Copy Markdown
Owner

This is the fix we most wanted from the 0.9.1-rc bug queue — the coverage signal we shipped this week (#963/#968) flagged exactly this class on our own codebase (#961: cbm_path_within_root, handle_process_kill — both in-function #ifdef splits), and recovering missing callables from the already-existing preprocessor second pass is the right systemic answer: raw parse stays primary, expanded AST only fills verified gaps, deduped by label+QN. The defs-only walker entry point is a clean seam.

Two things before merge:

  1. Line coordinates of recovered defs. The merged callables carry EXPANDED-source line numbers, but get_code_snippet (and the UI's GitHub deep-links) read the ORIGINAL file at start_line..end_line — a drifted range shows the wrong code for exactly the methods this PR rescues. Please either remap to original coordinates where feasible, or mark recovered defs (e.g. a recovered_from_preprocessed property / reduced confidence) and document the drift — plus one test pinning whatever behavior you choose, so snippet consumers aren't silently wrong.

  2. Rebase onto current main — the end-of-extraction region of internal/cbm/cbm.c changed in feat(index): best-effort parse-coverage signal — flag not-fully-indexed files #968: the parse-coverage signal now computes error regions AFTER extraction and subtracts recovered definitions. Your merge hook lands in the same neighborhood (conflict likely), and there's a pleasant interaction to verify once rebased: callables you recover should automatically UN-flag the corresponding parse_partial ranges (a region fully covered by recovered defs is dropped). The regression test tests/test_parse_coverage.c shows the mechanics; a one-line assertion in your test that the fixture is no longer parse_partial-flagged would prove the two features compose.

Genuinely excited about this one — with the rebase + the line-coordinate decision it merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working parsing/quality Graph extraction bugs, false positives, missing edges priority/normal Standard review queue; useful PR with ordinary maintainer urgency.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug for parse c++

2 participants