Skip to content

fix(cpp): resolve includes against real files and unify name resolution across passes#657

Merged
vitali87 merged 4 commits into
mainfrom
fix/cpp-dangling-calls
Jul 8, 2026
Merged

fix(cpp): resolve includes against real files and unify name resolution across passes#657
vitali87 merged 4 commits into
mainfrom
fix/cpp-dangling-calls

Conversation

@vitali87

@vitali87 vitali87 commented Jul 8, 2026

Copy link
Copy Markdown
Owner

Part of #652 (first iteration of the dangling-edge campaign).

Result

souffle-lang/souffle emitted-edge audit: 18,175 dangling edges down to 4,261 (77% reduction). The IMPORTS family (3,908) and the builtin-operator CALLS family (~1,700) are eliminated entirely; the phantom-caller CALLS family drops from 11,353 to 3,683. The #650 invariant holds: zero orphan nodes, zero schema violations on the re-indexed graph.

Root causes fixed

All five are the same disease in different organs: independent components deriving names their own way, with every disagreement becoming an edge the database silently drops.

  1. C++ include resolution invented module qns. #include "Directive.h" was resolved as project-root + path with the extension stripped (also mangling .hpp stems via .replace(".h", "")), so same-stem header/source pairs produced self-imports and -I-style includes (ast/Directive.h) got phantom roots. Includes now resolve against the actual file tree (includer-relative, root-relative, then unique path-suffix match with longest-common-prefix tie-break) using the collision-disambiguation replay already proven in the libclang frontend (build_module_qn_map). Unresolvable quoted includes are external headers, not phantom project modules.
  2. External import targets had no nodes. The generic IMPORTS emission now creates ExternalModule nodes for external targets (mirroring feat(graph)!: dedicated ExternalModule label replaces Module.is_external #597), instead of emitting relationships against nothing.
  3. The import map could answer a class lookup with a module. C++ include entries map header stems, and a stem naming the class it declares (the dominant C++ file convention) shadowed the real class in resolve_class_name. Pass-3 resolution now requires the answer to be a registered entity (require_registered), falling through to the registry-backed steps; Pass-2 semantics unchanged.
  4. Pass 3 recomputed module qns without collision disambiguation. CallProcessor._module_qn used with_suffix(""), so every caller inside a same-stem header was attributed under the source file's qn. It now consults the definition pass's module_qn_to_file_path as single source of truth.
  5. C++17 namespace a::b { rendered two ways. The namespace walk kept the literal a::b segment while other paths split it, so one class got two nodes (the forward-declaration dedup missed across spellings) and resolution picked the variant without the methods. Canonicalized to dotted segments in extract_namespace_path and _cpp_get_name; modern and classic nesting now yield byte-identical qns (also aligning tree-sitter with the libclang frontend's nested cursors).
  6. Out-of-line method binding was nondeterministic and re-done per pass. The class lookup iterated an unordered set accepting any same-leaf class (souffle has ast::Type and ast::analysis::Type), and Pass 3 re-resolved independently, diverging. The definition pass now resolves once (namespace-scoped candidates first, deterministic sorted iteration), records each binding keyed by (module qn, definition line), and call attribution replays the recorded decision.
  7. Builtin operator table shadowed user overloads and emitted edges to nowhere. A user-defined operator+ never resolved because the synthetic builtin.cpp.operator_* table answered first, and those synthetic targets never had nodes. User overloads now win; primitive builtin operators emit no call edge (six fixture-count tests updated: they were counting edges the database always dropped).

Verification

Remaining (next iterations, tracked in #652)

3,683 CALLS + 157 REFERENCES phantom callers, 398 INHERITS cross-file base resolution, 21 INSTANTIATES, 2 OVERRIDES.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request resolves issue #652 by significantly improving C++ parsing accuracy, particularly for include resolution, C++17 nested namespaces, out-of-class method bindings, and operator calls. It ensures same-stem headers and source files are correctly disambiguated, nested namespaces are split properly, out-of-class methods are resolved scope-first and reused deterministically, and user-defined operator overloads are prioritized over builtins. The review feedback suggests replacing os.path.commonprefix with os.path.commonpath in _resolve_cpp_include_target to ensure path-safe, component-aware matching when resolving include targets.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread codebase_rag/parsers/import_processor.py
@greptile-apps

greptile-apps Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR makes C++ graph name resolution use the same registered entities across passes. The main changes are:

  • Resolves quoted C++ includes against real files before creating import targets.
  • Creates external module nodes for external import targets.
  • Reuses definition-pass module qns and out-of-class method bindings during call processing.
  • Normalizes C++17 nested namespaces to the same dotted qn form as classic nesting.
  • Lets registered user-defined operator overloads win and skips primitive builtin operator call edges.
  • Adds tests for include resolution, nested namespaces, same-leaf class binding, and operator calls.

Confidence Score: 5/5

Safe to merge with minimal risk.

No blocking or non-blocking issues were identified in the changed paths. The updated C++ resolution flow is covered by focused tests for the affected graph edge families.

No files require special attention.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex attempted to run the focused C++ regression pytest in /home/user/repo; the run exited with code 4 due to a missing loguru during test collection.
  • T-Rex generated a parser import smoke script to exercise parser dependencies and updated parser modules.
  • T-Rex attempted to run the parser import smoke script; the run exited with code 1 and was blocked by a missing tree_sitter.
  • T-Rex ran a code style check (ruff) for the cpp regression; the run exited with code 2 and was blocked by a missing ruff executable.

View all artifacts

T-Rex Ran code and verified through T-Rex

Important Files Changed

Filename Overview
codebase_rag/parsers/import_processor.py Resolves C++ quoted includes against real repo files and creates nodes for external import targets.
codebase_rag/parsers/function_ingest.py Makes C++ out-of-class method binding namespace-aware, deterministic, and replayable by call processing.
codebase_rag/parsers/call_processor.py Reuses definition-pass module qns and recorded C++ out-of-class method bindings during call attribution.
codebase_rag/parsers/call_resolver.py Requires registered class targets in pass-3 resolution and lets registered operator overloads win over primitive builtins.
codebase_rag/parsers/cpp/utils.py Splits C++17 nested namespace spellings into stable dotted namespace path segments.
codebase_rag/parsers/py/utils.py Adds optional registered-target requirement to shared class-name resolution to avoid module imports shadowing classes.
codebase_rag/tests/test_cpp_include_resolution.py Adds tests for same-stem header/source include resolution and out-of-line method caller attribution.
codebase_rag/tests/test_cpp_same_leaf_class_method_binding.py Adds tests for namespace-scoped same-leaf C++ class method binding and real caller nodes.
codebase_rag/tests/test_cpp_operator_call_resolution.py Adds tests for user-defined operator overload resolution and absence of builtin operator phantom targets.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Def as DefinitionProcessor
participant Imp as ImportProcessor
participant Reg as FunctionRegistry
participant Call as CallProcessor
participant Graph as Ingestor

Def->>Graph: create MODULE / CLASS / METHOD nodes
Def->>Def: disambiguate module qns and record C++ method bindings
Imp->>Imp: resolve quoted includes against real repo files
Imp->>Graph: create IMPORTS edges / ExternalModule nodes
Call->>Def: reuse module qn map and recorded method bindings
Call->>Reg: require registered class/operator targets
Call->>Graph: emit CALLS from registered caller/callee qns
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Def as DefinitionProcessor
participant Imp as ImportProcessor
participant Reg as FunctionRegistry
participant Call as CallProcessor
participant Graph as Ingestor

Def->>Graph: create MODULE / CLASS / METHOD nodes
Def->>Def: disambiguate module qns and record C++ method bindings
Imp->>Imp: resolve quoted includes against real repo files
Imp->>Graph: create IMPORTS edges / ExternalModule nodes
Call->>Def: reuse module qn map and recorded method bindings
Call->>Reg: require registered class/operator targets
Call->>Graph: emit CALLS from registered caller/callee qns
Loading

Reviews (3): Last reviewed commit: "chore: refresh uv.lock after rebase onto..." | Re-trigger Greptile

@vitali87

vitali87 commented Jul 8, 2026

Copy link
Copy Markdown
Owner Author

@greptile review

@vitali87
vitali87 force-pushed the fix/cpp-dangling-calls branch from b2daae3 to daebde8 Compare July 8, 2026 12:50
@codecov-commenter

codecov-commenter commented Jul 8, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 99.15612% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
codebase_rag/parsers/import_processor.py 93.10% 2 Missing ⚠️

📢 Thoughts on this report? Let us know!

@sonarqubecloud

sonarqubecloud Bot commented Jul 8, 2026

Copy link
Copy Markdown

@vitali87

vitali87 commented Jul 8, 2026

Copy link
Copy Markdown
Owner Author

@greptile review

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

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants