From 401619c412b491664764051dd5ed9af1a0befd7e Mon Sep 17 00:00:00 2001 From: Aaron Queen Date: Wed, 14 Oct 2026 08:04:50 -0600 Subject: [PATCH 1/4] fix(resolve): seven more C++ edge cases in the Rust resolver A pointer alias (`using Ptr = Widget*; Ptr p(0);`) constructs nothing. A class nested in a function-local class is local to that function. A class an included .cc file declares is the includer's, and of two headers declaring one class an object creation takes the one it includes. A `using Base::foo;` in a nested class is not the outer class's. `NULL` is an integer zero when an overload is chosen. A header's unnamed namespace belongs to its includers, and the span check compares positions, not lines, so a comment before a directive no longer hides it. `const int FOO_BAR = 1;` and `n * FOO_BAR::VALUE` are left alone by annotation blanking, as extraction leaves them. --- CHANGELOG.md | 1 + README.md | 2 +- __tests__/cpp-resolver-edge-cases.test.ts | 218 ++++++++++++++++++ codegraph-kernel/src/resolve/cpp_aliases.rs | 3 + .../src/resolve/cpp_annotations.rs | 79 +++++-- .../src/resolve/cpp_construction.rs | 25 +- .../src/resolve/cpp_implicit_this.rs | 15 +- codegraph-kernel/src/resolve/cpp_overloads.rs | 3 +- .../src/resolve/cpp_translation_unit.rs | 70 ++++-- codegraph-kernel/src/resolve/cpp_types.rs | 23 +- codegraph-kernel/src/resolve/mod.rs | 4 +- 11 files changed, 380 insertions(+), 63 deletions(-) create mode 100644 __tests__/cpp-resolver-edge-cases.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c60ff68f2d..d2c257e5b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,6 +99,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes +- **Eight more C++ links land on the right target.** `Ptr p(0);` for `using Ptr = Widget*;` declares a pointer, so it no longer counts as constructing a `Widget`. A class nested in a function-local class is as local to that function as the class around it. A class an included `.cc` file declares is seen by the file that includes it, and of two headers declaring one class an object creation takes the one it includes. A `using Base::foo;` in a nested class no longer counts as the outer class's. `NULL` is an integer zero, not a `nullptr`, when an overload is chosen. A header's unnamed namespace belongs to the files that include the header, not to every file. Two annotation-macro cases read as extraction reads them: `const int FOO_BAR = 1;` keeps its name, and `n * FOO_BAR::VALUE` stays a product. - **Syncing a changed file in a large Python project is faster.** When a Python class has no method of the name being linked (a test class that passes `Handler.__init__` as a value, say), the resolver checks every class with that method for one that derives from it. For each class whose base is a package import (`import unittest`, then `unittest.TestCase`) it parsed the whole file again, and its cache holds only a few files, so two such references took about a second each. A plain one-line `import a.b` is now read from its line; other forms still use the parse. In CPython a one-file sync takes a median 5.5 s instead of 7.7 s on Node and 5.2 s instead of 8.1 s on Bun (4 alternating runs per build, ranges not overlapping; the phase that retries unresolved references drops from 4.6 s to 2.3 s). A full index is within the run-to-run variation, and the graph is the same on CPython and Django (no edge differs). - **The same sync resolves each Python class's bases once.** The scan above resolved the base classes of every candidate again for each reference that needed it. The bases of a class depend only on its own header and file, so they are now kept for the rest of the run. In CPython the phase that retries unresolved references drops from a median 2.29 s to 2.08 s on Node (-9%, 4 alternating runs per build, ranges not overlapping) and a one-file sync by about 3% (5.33 s to 5.15 s on the second sync); on Bun the sync time shows no clear change. A full index is within the run-to-run variation, and no edge, node or unresolved reference differs on CPython or Django. - **Questions to the MCP server are faster in a project with no `codegraph.json`.** While ranking results, CodeGraph checked for the project's `codegraph.json` once per result path, and with no such file each check raised and caught an error. It now asks the file system without raising, so `codegraph_explore` and the other query tools spend less CPU on most projects. Editing, adding or deleting `codegraph.json` is still picked up without a restart (#491). diff --git a/README.md b/README.md index 0f5a36266d..c03af1da1f 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,7 @@ Call resolution the fork adds: - C++ `->` calls on a `std::unique_ptr`, `std::shared_ptr` or `std::optional` reach the type it holds, looked up through the caller's enclosing classes and their bases, its namespaces, `using` directives and aliases. A held type the project does not declare leaves the call unresolved, as a plain pointer to one does; upstream guesses a callee from the receiver's name there. A partial class template specialization is indexed as an anonymous class, so a call to a member only it declares stays unresolved. - C++ calls on an `auto` local, on a chained call and on a data member of the caller's class (one a base class declares, or a gtest `TEST_F` / `TEST_P` fixture's) take the type its declaration writes, as upstream's do. Where that type cannot be read (a template parameter's, a `std::shared_ptr` of one, a trait of one, what a function template returns), the call stays unlinked, as does a local of a type the file does not declare; upstream guesses a callee from the receiver's name at 0.7. A chain of up to four calls is read from its declarations, where upstream reads its first inner call; a longer one is looked up by the last name, as before. - C++ namespace aliases and declarations in macro-opened namespaces are looked up only in the caller's include closure; upstream pools them across all files. -- C++ object creation sees a header under a test directory, such as nlohmann/json's old-release copy in `tests/abi/`, only from inside that directory's tree or through an `#include`; upstream sees every header. +- C++ object creation sees a header under a test directory, such as nlohmann/json's old-release copy in `tests/abi/`, only from inside that directory's tree or through an `#include`; upstream sees every header. A class declared in an included `.cc` file is the includer's too, and of two headers declaring one class the creation takes the one in its include closure. - C# namespace `using` directives and `using` aliases apply only inside their enclosing namespace, not to sibling namespaces in the same file (`using static` is still file-wide, as upstream). - A Java field declared with a qualified type (`outside.Repository`) keeps its qualifier, so calls through it never resolve to an unrelated project class with the same simple name. - Python `super().method()` resolves along the class's C3 method resolution order, starting after the class itself. Upstream resolves it like `self.method()` and drops the link only when it lands on the calling method. diff --git a/__tests__/cpp-resolver-edge-cases.test.ts b/__tests__/cpp-resolver-edge-cases.test.ts new file mode 100644 index 0000000000..ec1fc4ca64 --- /dev/null +++ b/__tests__/cpp-resolver-edge-cases.test.ts @@ -0,0 +1,218 @@ +/** + * Edge cases of the C++ resolver found reviewing the upstream reconcile (#497): + * + * - `Ptr p(0)` for `using Ptr = Widget*;` declares a pointer; it constructs no `Widget`. + * - A class nested in a function-local class is as local to the function as that class. + * - A class an included `.cc` file declares is the includer's. + * - A `using Base::foo;` in a nested class is not the outer class's. + * - A header's unnamed namespace is the includer's, not every file's. + * - Of two headers declaring one class, an object creation takes the one it includes. + * - `NULL` is an integer zero; a string literal converts to `const void*` ahead of a class. + * - Annotation blanking leaves `const int FOO_BAR = 1;` and `n * FOO_BAR::VALUE` alone. + */ +import { describe, it, expect, afterEach } from 'vitest'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; +import { CodeGraph } from '../src'; +import { blankCppDeclaratorAttributeMacros, blankCppPointerAnnotationMacros } from '../src/extraction/languages/c-cpp'; + +const roots: string[] = []; +afterEach(() => { + for (const root of roots.splice(0)) fs.rmSync(root, { recursive: true, force: true }); +}); + +async function indexed(files: Record): Promise { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'codegraph-cpp-edge-')); + roots.push(root); + for (const [rel, content] of Object.entries(files)) { + fs.mkdirSync(path.dirname(path.join(root, rel)), { recursive: true }); + fs.writeFileSync(path.join(root, rel), content); + } + return CodeGraph.init(root, { index: true }); +} + +const lines = (...text: string[]): string => [...text, ''].join('\n'); + +/** What a function's edges of `kind` reach at `line`, as `target:its first line @confidence`. */ +function reached(cg: CodeGraph, qualifiedName: string, kind: 'calls' | 'instantiates', line?: number): string[] { + const from = [...cg.getNodesByKind('method'), ...cg.getNodesByKind('function')].find((n) => n.qualifiedName === qualifiedName)!; + return cg.getOutgoingEdgesFrom([from.id], [kind]) + .filter((e) => line === undefined || e.line === line) + .map((e) => { + const t = cg.getNode(e.target)!; + return `${t.qualifiedName}:${t.startLine} @${(e.metadata as { confidence?: number } | undefined)?.confidence}`; + }) + .sort(); +} + +describe('C++ object creation', () => { + it('constructs no class for a pointer alias of it', async () => { + const cg = await indexed({ + 'widget.h': lines( + 'class Widget {', + ' public:', + ' Widget(int n) {}', + '};', + 'using Ptr = Widget*;', + 'void Run() {', + ' Ptr p(0);', + ' Widget w(1);', + '}', + ), + }); + try { + expect(reached(cg, 'Run', 'instantiates', 7)).toEqual([]); + expect(reached(cg, 'Run', 'instantiates', 8)).toEqual(['Widget:1 @0.9']); + } finally { + cg.close(); + } + }); + + it("does not see a class nested in a function's local class from another function", async () => { + const cg = await indexed({ + 'a_test.cc': lines( + 'namespace ns {', + 'struct Local { struct Inner { Inner(int n) {} }; };', + 'TEST_F(A, B) {', + ' struct Local { struct Inner { Inner(int n) {} }; };', + ' Local::Inner own(1);', + '}', + 'TEST_F(A, C) {', + ' Local::Inner other(1);', + '}', + '} // namespace ns', + ), + }); + try { + const made = cg.getNodesByKind('function').filter((n) => n.name === 'TEST_F').sort((a, b) => a.startLine - b.startLine) + .map((fn) => cg.getOutgoingEdgesFrom([fn.id], ['instantiates']).map((e) => `${fn.startLine} -> ${cg.getNode(e.target)?.startLine}`).sort()); + // (Each `Inner` is recorded by the name before its first `::`, `Local`.) + expect(made).toEqual([['3 -> 4'], ['7 -> 2']]); + } finally { + cg.close(); + } + }); + + it('constructs a class an included source file declares', async () => { + const cg = await indexed({ + 'engine.cc': lines('class Engine {', ' public:', ' Engine(int n) {}', '};'), + 'engine_test.cc': lines('#include "engine.cc"', 'void Run() {', ' Engine e(1);', '}'), + }); + try { + expect(reached(cg, 'Run', 'instantiates')).toEqual(['Engine:1 @0.9']); + } finally { + cg.close(); + } + }); + + it('takes, of two headers declaring one class, the one it includes', async () => { + const widget = (n: number) => lines('namespace ns {', 'class Widget {', ' public:', ` Widget(int n) {} // ${n}`, '};', '} // namespace ns'); + const cg = await indexed({ + 'one/widget.h': widget(1), + 'two/widget.h': widget(2), + 'one/use.cc': lines('#include "two/widget.h"', 'void Run() {', ' ns::Widget w(1);', '}'), + }); + try { + const run = [...cg.getNodesByKind('function')].find((n) => n.qualifiedName === 'Run')!; + const files = cg.getOutgoingEdgesFrom([run.id], ['instantiates']).map((e) => cg.getNode(e.target)?.filePath); + expect(files).toEqual(['two/widget.h']); + } finally { + cg.close(); + } + }); +}); + +describe('C++ calls', () => { + it("does not take a nested class's using-declaration for the outer class's", async () => { + const cg = await indexed({ + 'outer.h': lines( + 'template ', + 'class Base {', + ' public:', + ' void foo() {}', + '};', + 'void foo() {}', + 'template ', + 'class Outer : public Base {', + ' public:', + ' struct Nested : Base {', + ' using Base::foo;', + ' };', + ' void run() {', + ' foo();', + ' }', + '};', + ), + }); + try { + // (The free function, at the confidence of a name a dependent base might also give.) + expect(reached(cg, 'Outer::run', 'calls', 14).map((r) => r.split(' ')[0])).toEqual(['foo:6']); + } finally { + cg.close(); + } + }); + + it('takes NULL for an integer zero, not for a nullptr', async () => { + const cg = await indexed({ + 'reset.h': lines( + '#include ', + 'class Holder {', + ' public:', + ' void Reset(std::nullptr_t) {}', + ' void Reset(int n) {}', + ' void Run() { Reset(NULL); }', + '};', + ), + }); + try { + expect(reached(cg, 'Holder::Run', 'calls', 6)).toEqual(['Holder::Reset:5 @0.9']); + } finally { + cg.close(); + } + }); + + it("takes a string literal for a `const void*` ahead of a std::string it converts to", async () => { + const cg = await indexed({ + 'take.h': lines( + 'void Take(const std::string& text) {}', + 'void Take(const void* data) {}', + 'void Run() { Take("x"); }', + ), + }); + try { + expect(reached(cg, 'Run', 'calls', 3).map((r) => r.split(' ')[0])).toEqual(['Take:2']); + } finally { + cg.close(); + } + }); + + it("is not what a header's unnamed namespace declares, named by a file that does not include it", async () => { + const cg = await indexed({ + 'conformance/matchers.h': lines('namespace conformance {', 'namespace {', 'int RawPayload(int response) { return response; }', '} // namespace', '} // namespace conformance'), + 'conformance/matchers_test.cc': lines('namespace conformance {', 'int Describe() {', ' return RawPayload(1);', '}', '} // namespace conformance'), + 'conformance/uses_matchers.cc': lines('#include "matchers.h"', 'namespace conformance {', 'int Check() {', ' return RawPayload(1);', '}', '} // namespace conformance'), + }); + try { + const filesOf = (name: string) => { + const from = cg.getNodesByKind('function').find((n) => n.qualifiedName === name)!; + return cg.getOutgoingEdgesFrom([from.id], ['calls']).map((e) => cg.getNode(e.target)?.filePath); + }; + expect(filesOf('conformance::Describe')).toEqual([]); + expect(filesOf('conformance::Check')).toEqual(['conformance/matchers.h']); + } finally { + cg.close(); + } + }); +}); + +describe('C++ annotation blanking', () => { + it('leaves a constant named in capitals alone', () => { + expect(blankCppDeclaratorAttributeMacros('const int FOO_BAR = 1;')).toBe('const int FOO_BAR = 1;'); + expect(blankCppDeclaratorAttributeMacros('static constexpr unsigned MAX_LEN = 4;')).toBe('static constexpr unsigned MAX_LEN = 4;'); + }); + + it('leaves a qualified name after a product alone', () => { + expect(blankCppPointerAnnotationMacros('consume(n * FOO_BAR::VALUE);')).toBe('consume(n * FOO_BAR::VALUE);'); + }); +}); diff --git a/codegraph-kernel/src/resolve/cpp_aliases.rs b/codegraph-kernel/src/resolve/cpp_aliases.rs index 240ca649f8..e990c004aa 100644 --- a/codegraph-kernel/src/resolve/cpp_aliases.rs +++ b/codegraph-kernel/src/resolve/cpp_aliases.rs @@ -40,6 +40,9 @@ pub(super) fn type_segments(raw: &str) -> Option> { (!parts.is_empty() && parts.iter().all(|p| re!(r"^[A-Za-z_]\w*$").is_match(p))).then_some(parts) } +/// Whether `raw` is a pointer or a reference to something (`Widget*`, `const Widget&`). +pub(super) fn indirect_type(raw: &str) -> bool { pointer_type(raw) || reference_type(raw) } + fn pointer_type(raw: &str) -> bool { indirection_type(raw, '*') } fn reference_type(raw: &str) -> bool { indirection_type(raw, '&') } diff --git a/codegraph-kernel/src/resolve/cpp_annotations.rs b/codegraph-kernel/src/resolve/cpp_annotations.rs index c5f41db3f7..b0de7590d2 100644 --- a/codegraph-kernel/src/resolve/cpp_annotations.rs +++ b/codegraph-kernel/src/resolve/cpp_annotations.rs @@ -89,7 +89,46 @@ fn arguments_end(code: &[u8], open: usize) -> Option { } const ALTERNATIVE_OPERATORS: &[&str] = &["and", "and_eq", "bitand", "bitor", "compl", "not", "not_eq", "or", "or_eq", "xor", "xor_eq"]; -const NOT_A_TYPE: &[&str] = &["return", "throw", "co_return", "case", "goto", "else", "do", "new", "delete", "sizeof", "typedef", "using"]; +const TYPE_KEYWORDS: &[&str] = &["void", "bool", "char", "char8_t", "char16_t", "char32_t", "wchar_t", "short", "int", "long", "float", "double", "signed", "unsigned", "auto"]; +const OTHER_KEYWORDS: &[&str] = &[ + "if", "while", "for", "switch", "catch", "return", "sizeof", "alignof", "alignas", "decltype", "typeid", "noexcept", "throw", + "static_assert", "new", "delete", "case", "requires", "co_await", "co_yield", "co_return", "asm", "__asm__", "__attribute__", + "__attribute", "__declspec", "_Pragma", "else", "do", "constexpr", "consteval", "explicit", "typeof", "__typeof__", "__typeof", + "_Alignof", "__alignof__", "volatile", "__volatile__", "const", "static", "extern", "inline", "virtual", "constinit", "friend", + "mutable", "thread_local", "register", "typedef", "using", "template", "typename", "class", "struct", "union", "enum", + "namespace", "operator", "goto", "break", "continue", "default", "public", "private", "protected", "this", "true", "false", + "nullptr", "and", "or", "not", "xor", "bitand", "bitor", "compl", "and_eq", "or_eq", "xor_eq", "not_eq", +]; + +fn is_keyword(word: &str) -> bool { TYPE_KEYWORDS.contains(&word) || OTHER_KEYWORDS.contains(&word) } + +/// Whether the identifier ending at `code[last]` is a name a declaration declares: it has a +/// lowercase letter, is no keyword, and follows a type (a `*`, `&`, `>` that is no arrow, a type +/// keyword or another name; `const` and `volatile` only after a `*` or `&`). So in `const int +/// FOO_BAR = 1;` the name before `FOO_BAR` is `int`, a type, and `FOO_BAR` is the declared name. +fn follows_declared_name(code: &[u8], last: usize) -> bool { + if !is_word(code[last]) { return false; } + let mut start = last; + while start > 0 && is_word(code[start - 1]) { start -= 1; } + let name = std::str::from_utf8(&code[start..=last]).unwrap_or(""); + if name.as_bytes()[0].is_ascii_digit() || !name.bytes().any(|b| b.is_ascii_lowercase()) || is_keyword(name) { return false; } + let mut i = start; + while i > 0 && code[i - 1].is_ascii_whitespace() { i -= 1; } + let Some(c) = i.checked_sub(1).map(|p| code[p]) else { return false }; + if c == b'*' || c == b'&' { return true; } + if c == b'>' { return i < 2 || code[i - 2] != b'-'; } + if !is_word(c) { return false; } + let mut j = i; + while j > 0 && is_word(code[j - 1]) { j -= 1; } + let before = std::str::from_utf8(&code[j..i]).unwrap_or(""); + if before == "const" || before == "volatile" { + let mut k = j; + while k > 0 && code[k - 1].is_ascii_whitespace() { k -= 1; } + return k > 0 && matches!(code[k - 1], b'*' | b'&'); + } + if TYPE_KEYWORDS.contains(&before) { return true; } + !before.as_bytes()[0].is_ascii_digit() && !is_keyword(before) +} /// The spans of the macros right after a `*` or `&` and before the name it declares (or another /// `*`): `Arena* PROTOBUF_NULLABLE arena`, `const FieldDescriptor* PROTOBUF_NULLABLE* PROTOBUF_NONNULL @@ -101,10 +140,11 @@ fn pointer_annotation_spans(code: &[u8], text: &str, spans: &mut Vec<(usize, usi let mut at = skip_space(code, star + 1); let mut run = Vec::new(); while let Some(end) = macro_name_end(code, at) { - run.push((at, end)); let next = skip_space(code, end); - at = next; + // (A macro stands apart from what follows it: `n * FOO_BAR::VALUE` is a product.) if next == end && code.get(end) != Some(&b'*') { break; } + run.push((at, end)); + at = next; } if run.is_empty() { continue; } let after = at; @@ -132,7 +172,7 @@ fn pointer_annotation_spans(code: &[u8], text: &str, spans: &mut Vec<(usize, usi /// `versions_ GUARDED_BY(mutex_);`, `cord ABSL_ATTRIBUTE_LIFETIME_BOUND)`. The macro is followed by /// `;`, `=`, `,` or `)`, and stands after `]` or after a name with a lowercase letter that itself /// follows a type (a name, `*`, `&` or `>`), so a variable named in capitals stays. -fn declarator_attribute_spans(code: &[u8], text: &str, spans: &mut Vec<(usize, usize)>) { +fn declarator_attribute_spans(code: &[u8], spans: &mut Vec<(usize, usize)>) { let mut i = 0; while i < code.len() { let Some(mut end) = macro_name_end(code, i) else { i += 1; continue }; @@ -155,24 +195,7 @@ fn declarator_attribute_spans(code: &[u8], text: &str, spans: &mut Vec<(usize, u let space_end = k; while k > 0 && code[k - 1].is_ascii_whitespace() { k -= 1; } if k == space_end || k == 0 { continue; } - let declares = if code[k - 1] == b']' { true } else { - let name_end = k; - while k > 0 && is_word(code[k - 1]) { k -= 1; } - let name = &text[k..name_end]; - if name.is_empty() || !name.bytes().any(|b| b.is_ascii_lowercase()) || name.as_bytes()[0].is_ascii_digit() { false } else { - let mut t = k; - while t > 0 && code[t - 1].is_ascii_whitespace() { t -= 1; } - match t.checked_sub(1).map(|p| code[p]) { - Some(b'*' | b'&' | b'>') => true, - Some(c) if is_word(c) => { - let word_end = t; - while t > 0 && is_word(code[t - 1]) { t -= 1; } - !NOT_A_TYPE.contains(&&text[t..word_end]) - } - _ => false, - } - } - }; + let declares = code[k - 1] == b']' || follows_declared_name(code, k - 1); if declares { spans.extend(run); } } } @@ -183,7 +206,7 @@ pub(super) fn cpp_without_annotations(text: &str) -> Cow<'_, str> { let code = masked_code(text); let mut spans = Vec::new(); pointer_annotation_spans(&code, text, &mut spans); - declarator_attribute_spans(&code, text, &mut spans); + declarator_attribute_spans(&code, &mut spans); if spans.is_empty() { return Cow::Borrowed(text); } let mut out = text.as_bytes().to_vec(); for (from, to) in spans { @@ -211,4 +234,14 @@ mod tests { assert!(out.starts_with(&expected), "{out}"); assert!(out.contains("a * MAX_LEN + 1") && out.contains("Foo DEFAULT_OPTIONS;") && out.contains("x * FOO_BAR") && out.contains("Foo* PROTOBUF_NONNULL c;")); } + + #[test] + fn leaves_a_capitalized_constant_and_a_qualified_name_after_a_product() { + for text in ["const int FOO_BAR = 1;\n", "static constexpr unsigned MAX_LEN = 4;\n", "consume(n * FOO_BAR::VALUE);\n", "volatile long RETRY_LIMIT = 3;\n"] { + assert_eq!(cpp_without_annotations(text), text); + } + let text = "Printer* PROTOBUF_NONNULL printer_;\nconst Foo* PROTOBUF_NULLABLE ns::Bar::run();\nstd::vector items_ GUARDED_BY(mu_);\n"; + let out = cpp_without_annotations(text); + assert!(!out.contains("PROTOBUF_NONNULL") && !out.contains("PROTOBUF_NULLABLE") && !out.contains("GUARDED_BY"), "{out}"); + } } diff --git a/codegraph-kernel/src/resolve/cpp_construction.rs b/codegraph-kernel/src/resolve/cpp_construction.rs index 9c104daf8a..1e39ee8924 100644 --- a/codegraph-kernel/src/resolve/cpp_construction.rs +++ b/codegraph-kernel/src/resolve/cpp_construction.rs @@ -137,6 +137,8 @@ impl KernelResolver { for f in files.iter().filter(|f| *f == file || !source_file(f)) { if !self.cpp_unincluded_test_header(f, file)? { visible.insert(f.clone()); } } + // (A source file it includes is its own: a test that includes the `.cc` it tests.) + for f in self.namespace_visible_files(file, "cpp")? { if source_file(&f) { visible.insert(f); } } let visible = Rc::new(visible); self.cpp_visible_memo = Some((file.to_string(), visible.clone())); Ok(visible) @@ -161,12 +163,17 @@ impl KernelResolver { /// that function (the index names it after the function, and every gtest `TEST_F` body in a /// namespace is `ns::TEST_F`). fn cpp_construction_sees(&mut self, n: &KNode, r: &ResolveRefIn) -> Res { - if n.file_path != r.file_path && source_file(&n.file_path) { return Ok(false); } + if n.file_path != r.file_path && source_file(&n.file_path) && !self.namespace_visible_files(&r.file_path, &r.language)?.contains(&n.file_path) { return Ok(false); } if self.cpp_unincluded_test_header(&n.file_path, &r.file_path)? { return Ok(false); } - let Some((scope, _)) = n.qualified_name.rsplit_once("::") else { return Ok(true) }; - let around = self.nodes_by_qualified_name(scope)?.iter().find(|f| matches!(f.kind.as_str(), "function" | "method") - && f.file_path == n.file_path && f.start_line <= n.start_line && f.end_line >= n.start_line).cloned(); - Ok(around.is_none_or(|f| f.file_path == r.file_path && f.start_line <= r.line && r.line <= f.end_line)) + // A class declared in a function, or in a class declared there, is the function's. + let mut scope = n.qualified_name.as_str(); + while let Some((outer, _)) = scope.rsplit_once("::") { + scope = outer; + let around = self.nodes_by_qualified_name(scope)?.iter().find(|f| matches!(f.kind.as_str(), "function" | "method") + && f.file_path == n.file_path && f.start_line <= n.start_line && f.end_line >= n.start_line).cloned(); + if let Some(f) = around { return Ok(f.file_path == r.file_path && f.start_line <= r.line && r.line <= f.end_line); } + } + Ok(true) } /// The types named `name` in the scope `scope` that code at `r` sees, else in the classes a @@ -177,6 +184,12 @@ impl KernelResolver { for n in self.nodes_by_qualified_name(&qualified)?.iter().filter(|n| type_kind(n)) { if self.cpp_construction_sees(n, r)? { found.push(n.clone()); } } + // Of one class declared in several files, those the code includes, when there are any. + if found.len() > 1 && found.iter().any(|n| n.file_path != found[0].file_path) { + let included = self.namespace_visible_files(&r.file_path, &r.language)?; + let seen: Vec<_> = found.iter().filter(|n| n.file_path == r.file_path || included.contains(&n.file_path)).cloned().collect(); + if !seen.is_empty() { found = seen; } + } if !found.is_empty() || scope.is_empty() || depth >= 4 { return Ok(found); } let classes: Vec<_> = self.nodes_by_qualified_name(scope)?.iter().filter(|n| class_like(n)).cloned().collect(); for class in classes { @@ -226,6 +239,8 @@ impl KernelResolver { current = next; } if current.kind == "type_alias" { + // (A pointer or reference alias names no class to construct: `Ptr p(0);`.) + if self.cpp_alias_indirect(¤t) { return Ok(Lookup::NoClass); } if let Some(class) = self.cpp_expand_owner(¤t, r, 0, false, visible)? { current = class; } } return Ok(Lookup::Found(current, true)); diff --git a/codegraph-kernel/src/resolve/cpp_implicit_this.rs b/codegraph-kernel/src/resolve/cpp_implicit_this.rs index 4a60d75388..eda12119f7 100644 --- a/codegraph-kernel/src/resolve/cpp_implicit_this.rs +++ b/codegraph-kernel/src/resolve/cpp_implicit_this.rs @@ -404,7 +404,7 @@ impl KernelResolver { let from = (decl.start_line - 1).max(0) as usize; let to = (decl.end_line.max(decl.start_line) as usize).min(lines.len()); if from >= to { return false; } - let body = lines[from..to].join("\n"); + let body = cpp_own_scope(&lines[from..to].join("\n")); re!(r"\busing\s+((?:::\s*)?[A-Za-z_][\w:]*(?:\s*<[^;{}]*>)?)\s*::\s*([A-Za-z_]\w*)\s*;").captures_iter(&body).any(|m| { &m[2] == name && !m[1].trim_start().starts_with("namespace") && cpp_last_segment(strip_template_arguments(&m[1]).trim()) == base.name }) @@ -484,3 +484,16 @@ fn collect_template_header(source: &SourceFile, start_line: i64, start_column: i } } } + +/// The text of a class declaration at its own brace depth: what a nested class, a member +/// function body or an initializer braces holds is blanked, so a `using Base::foo;` of a +/// nested class is not the outer class's. +fn cpp_own_scope(text: &str) -> String { + let mut depth = 0usize; + text.chars().map(|c| match c { + '{' => { depth += 1; if depth > 1 { ' ' } else { c } } + '}' => { let was = depth; depth = depth.saturating_sub(1); if was > 1 { ' ' } else { c } } + '\n' => c, + _ => if depth > 1 { ' ' } else { c }, + }).collect() +} diff --git a/codegraph-kernel/src/resolve/cpp_overloads.rs b/codegraph-kernel/src/resolve/cpp_overloads.rs index f7c131491c..dd86759763 100644 --- a/codegraph-kernel/src/resolve/cpp_overloads.rs +++ b/codegraph-kernel/src/resolve/cpp_overloads.rs @@ -224,8 +224,9 @@ impl KernelResolver { Ok(match node.kind() { "char_literal" => Some(CppArgument::Char), "true" | "false" => Some(CppArgument::Bool), + // `NULL` is an integer zero (a null pointer constant); only `nullptr` is a nullptr_t. + "null" | "identifier" if text == "NULL" => Some(CppArgument::Zero), "null" | "nullptr" => Some(CppArgument::Null), - "identifier" if text == "NULL" => Some(CppArgument::Null), "number_literal" if re!(r"^0+[uUlL]*$").is_match(text) => Some(CppArgument::Zero), "number_literal" => Some(CppArgument::Number), "unary_expression" if re!(r"^[-+]\s*[0-9.]").is_match(text) => Some(CppArgument::Number), diff --git a/codegraph-kernel/src/resolve/cpp_translation_unit.rs b/codegraph-kernel/src/resolve/cpp_translation_unit.rs index addde4883f..0f0a52e7ad 100644 --- a/codegraph-kernel/src/resolve/cpp_translation_unit.rs +++ b/codegraph-kernel/src/resolve/cpp_translation_unit.rs @@ -22,25 +22,26 @@ fn cpp_class_kind(kind: &str) -> bool { matches!(kind, "class" | "struct" | "union") } -/// The line spans (1-based, inclusive) of the unnamed namespaces `text` writes, the outermost of -/// nested ones: what one declares has internal linkage, so no other translation unit names it. -/// Comments, preprocessor lines and string, character and raw string literals are skipped. -pub(super) fn unnamed_namespace_spans(text: &str) -> Vec<(i64, i64)> { +/// A position in a file: its line (1-based) and its column (UTF-16 units from the line's start, 0-based). +pub(super) type Position = (i64, i64); + +/// The spans of the unnamed namespaces `text` writes, the outermost of nested ones, from their +/// `{` to their `}`: what one declares has internal linkage, so no other translation unit names +/// it. Comments, preprocessor lines and string, character and raw string literals are skipped. +pub(super) fn unnamed_namespace_spans(text: &str) -> Vec<(Position, Position)> { let mut spans = Vec::new(); if !re!(r"\bnamespace\s*\{").is_match(text) { return spans; } let bytes = text.as_bytes(); - // Each open brace: the line it opens on, and whether it opens an unnamed namespace. - let mut open: Vec<(i64, bool)> = Vec::new(); - let mut line = 1i64; + // Each open brace: where it is, and whether it opens an unnamed namespace. + let mut open: Vec<(usize, bool)> = Vec::new(); + let mut offsets: Vec<(usize, usize)> = Vec::new(); let mut line_start = true; // The code since the last brace or `;`, without comments and literals. let mut code = String::new(); let mut i = 0; - let count_lines = |from: usize, to: usize| bytes[from..to.min(bytes.len())].iter().filter(|&&b| b == b'\n').count() as i64; while i < bytes.len() { let c = bytes[i]; if c == b'\n' { - line += 1; line_start = true; code.push(' '); i += 1; @@ -50,29 +51,27 @@ pub(super) fn unnamed_namespace_spans(text: &str) -> Vec<(i64, i64)> { // A preprocessor line, continued by a trailing backslash. let mut end = i; while end < bytes.len() && (bytes[end] != b'\n' || (end > 0 && bytes[end - 1] == b'\\') || (end > 1 && bytes[end - 1] == b'\r' && bytes[end - 2] == b'\\')) { - if bytes[end] == b'\n' { line += 1; } end += 1; } i = end; continue; } - if !c.is_ascii_whitespace() { line_start = false; } + // (A comment is a space: a directive may follow one that starts its line.) if c == b'/' && bytes.get(i + 1) == Some(&b'/') { i = text[i..].find('\n').map_or(bytes.len(), |n| i + n); continue; } if c == b'/' && bytes.get(i + 1) == Some(&b'*') { - let stop = text[i + 2..].find("*/").map_or(bytes.len(), |n| i + 2 + n + 2); - line += count_lines(i, stop); - i = stop; + i = text[i + 2..].find("*/").map_or(bytes.len(), |n| i + 2 + n + 2); continue; } + if !c.is_ascii_whitespace() { line_start = false; } if c == b'"' && { let from = (i.saturating_sub(4)..i).find(|&k| text.is_char_boundary(k)).unwrap_or(i); re!(r"(?:^|[^\w$])(?:u8|[uUL])?R$").is_match(&text[from..i]) } { // `R"delim( … )delim"`. - let stop = match text[i..].find('(') { + i = match text[i..].find('(') { Some(p) => { let paren = i + p; let closing = format!("){}\"", &text[i + 1..paren]); @@ -80,8 +79,6 @@ pub(super) fn unnamed_namespace_spans(text: &str) -> Vec<(i64, i64)> { } None => bytes.len(), }; - line += count_lines(i, stop); - i = stop; continue; } if c == b'"' || c == b'\'' { @@ -94,12 +91,12 @@ pub(super) fn unnamed_namespace_spans(text: &str) -> Vec<(i64, i64)> { } match c { b'{' => { - open.push((line, re!(r"(?:^|[^\w$])namespace\s*$").is_match(&code))); + open.push((i, re!(r"(?:^|[^\w$])namespace\s*$").is_match(&code))); code.clear(); } b'}' => { if let Some((start, unnamed)) = open.pop() { - if unnamed && !open.iter().any(|(_, u)| *u) { spans.push((start, line)); } + if unnamed && !open.iter().any(|(_, u)| *u) { offsets.push((start, i)); } } code.clear(); } @@ -112,6 +109,14 @@ pub(super) fn unnamed_namespace_spans(text: &str) -> Vec<(i64, i64)> { } i += 1; } + if offsets.is_empty() { return spans; } + let mut line_starts = vec![0usize]; + line_starts.extend(text.match_indices('\n').map(|(at, _)| at + 1)); + let position = |offset: usize| -> Position { + let line = line_starts.partition_point(|&s| s <= offset) - 1; + (line as i64 + 1, text[line_starts[line]..offset].encode_utf16().count() as i64) + }; + spans.extend(offsets.into_iter().map(|(from, to)| (position(from), position(to)))); spans } @@ -142,15 +147,25 @@ impl KernelResolver { return Ok(true); } } - if self.cpp_seen_from(candidate, &r.file_path)? { return Ok(false); } + // A source file's unnamed namespace is its own and its includers'; a header's is the + // includers' of that header, so another file's `Helper` is not the includer's. + if c_source_ext_re().is_match(&candidate.file_path) && self.cpp_seen_from(candidate, &r.file_path)? { return Ok(false); } let Some(source) = self.read_file(&candidate.file_path) else { return Ok(false) }; - Ok(source.cpp_unnamed_namespaces().iter().any(|&(start, end)| candidate.start_line >= start && candidate.start_line <= end)) + let at = (candidate.start_line, candidate.start_column); + if !source.cpp_unnamed_namespaces().iter().any(|&(open, close)| at > open && at < close) { return Ok(false); } + Ok(!self.cpp_file_reaches(candidate, &r.file_path)?) } /// Whether the declaration `decl` is seen from `file`: it is no source file's, or `file` is its /// own, includes its file, or is a header its file includes below it. fn cpp_seen_from(&mut self, decl: &KNode, file: &str) -> Res { - if !c_source_ext_re().is_match(&decl.file_path) || decl.file_path == file { return Ok(true); } + if !c_source_ext_re().is_match(&decl.file_path) { return Ok(true); } + self.cpp_file_reaches(decl, file) + } + + /// Whether `file` is the file of `decl`, includes it, or is a header it includes below it. + fn cpp_file_reaches(&mut self, decl: &KNode, file: &str) -> Res { + if decl.file_path == file { return Ok(true); } if self.namespace_visible_files(file, &decl.language)?.contains(&decl.file_path) { return Ok(true); } self.cpp_includes_after(&decl.file_path, decl.start_line, file, &decl.language) } @@ -197,7 +212,14 @@ mod tests { #[test] fn unnamed_namespaces_past_comments_and_literals() { let text = "namespace a {\nnamespace {\nint f();\nnamespace {\n}\n}\n// namespace {\n/* namespace { */\n#define X namespace {\nconst char* s = \"namespace {\";\nconst char* r = R\"(a \"namespace {\" b)\";\n}\n"; - assert_eq!(unnamed_namespace_spans(text), vec![(2, 6)]); - assert_eq!(unnamed_namespace_spans("#define B \\\n namespace {\nint g();\n"), Vec::<(i64, i64)>::new()); + assert_eq!(unnamed_namespace_spans(text), vec![((2, 10), (6, 0))]); + assert_eq!(unnamed_namespace_spans("#define B \\\n namespace {\nint g();\n"), vec![]); + } + + #[test] + fn a_directive_follows_a_comment_and_a_span_ends_at_its_brace() { + assert_eq!(unnamed_namespace_spans("/* note */ #define X namespace {\nclass A {};\n"), vec![]); + assert_eq!(unnamed_namespace_spans("namespace { class A {}; } class B {};\n"), vec![((1, 10), (1, 24))]); + assert_eq!(unnamed_namespace_spans("// é\nnamespace { int é; }\n"), vec![((2, 10), (2, 19))]); } } diff --git a/codegraph-kernel/src/resolve/cpp_types.rs b/codegraph-kernel/src/resolve/cpp_types.rs index a10dccbf5f..84e96415e0 100644 --- a/codegraph-kernel/src/resolve/cpp_types.rs +++ b/codegraph-kernel/src/resolve/cpp_types.rs @@ -191,18 +191,29 @@ impl KernelResolver { Ok(false) } - pub(super) fn cpp_expand_owner(&mut self, owner: &Arc, r: &ResolveRefIn, depth: u32, constructor: bool, included: &HashSet) -> Res>> { - if owner.kind != "type_alias" { return Ok((owner.kind != "enum").then(|| owner.clone())); } - let Some(source) = self.read_file(&owner.file_path) else { return Ok(None) }; + /// Whether the alias `owner` names a pointer or a reference (`using Ptr = Widget*;`). + pub(super) fn cpp_alias_indirect(&mut self, owner: &KNode) -> bool { + self.cpp_alias_rhs(owner).is_some_and(|rhs| super::cpp_aliases::indirect_type(&rhs)) + } + + /// What the alias `owner` is declared as: the text after its `=`. + fn cpp_alias_rhs(&mut self, owner: &KNode) -> Option { + let source = self.read_file(&owner.file_path)?; let mut lines = source.get((owner.start_line - 1).max(0) as usize..owner.end_line.max(0) as usize).unwrap_or(&[]).to_vec(); - if lines.is_empty() { return Ok(None); } + if lines.is_empty() { return None; } let last = lines.len() - 1; let end = super::names::js_unit_to_byte(&lines[last], owner.end_column.max(0) as usize); lines[last].truncate(end); lines[0] = lines[0].get(super::names::js_unit_to_byte(&lines[0], owner.start_column.max(0) as usize)..).unwrap_or("").to_string(); let text = lines.join("\n"); - let Some((_, rhs)) = text.split_once('=') else { return Ok(None) }; - let rhs = rhs.trim().trim_end_matches(';').trim(); + let (_, rhs) = text.split_once('=')?; + Some(rhs.trim().trim_end_matches(';').trim().to_string()) + } + + pub(super) fn cpp_expand_owner(&mut self, owner: &Arc, r: &ResolveRefIn, depth: u32, constructor: bool, included: &HashSet) -> Res>> { + if owner.kind != "type_alias" { return Ok((owner.kind != "enum").then(|| owner.clone())); } + let Some(rhs) = self.cpp_alias_rhs(owner) else { return Ok(None) }; + let rhs = rhs.as_str(); if constructor && rhs.contains(['*', '&']) { return Ok(None); } let rhs = re!(r"^(?:const|volatile)\s+").replace(rhs, ""); let rhs = rhs.trim_end_matches(['&', '*']).trim(); diff --git a/codegraph-kernel/src/resolve/mod.rs b/codegraph-kernel/src/resolve/mod.rs index a3467a75cc..42b9c478fa 100644 --- a/codegraph-kernel/src/resolve/mod.rs +++ b/codegraph-kernel/src/resolve/mod.rs @@ -343,7 +343,7 @@ pub(super) struct SourceFile { php_file_scope: OnceCell, rust_code_lines: OnceCell>, cpp_code_lines: OnceCell>, - cpp_unnamed_namespaces: OnceCell>, + cpp_unnamed_namespaces: OnceCell>, python_file: OnceCell, python_field_file: OnceCell, cpp_field_tree: OnceCell>>, @@ -419,7 +419,7 @@ impl SourceFile { } /// The line spans of the file's unnamed namespaces (cpp_translation_unit). - pub(super) fn cpp_unnamed_namespaces(&self) -> &[(i64, i64)] { + pub(super) fn cpp_unnamed_namespaces(&self) -> &[(cpp_translation_unit::Position, cpp_translation_unit::Position)] { self.cpp_unnamed_namespaces.get_or_init(|| cpp_translation_unit::unnamed_namespace_spans(self.text())) } From e8a90998cf09d7a1e40e0be7247b0b896223b22a Mon Sep 17 00:00:00 2001 From: Aaron Queen Date: Wed, 14 Oct 2026 08:05:05 -0600 Subject: [PATCH 2/4] docs: do not count the C++ fixes in the changelog headline --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2c257e5b7..364f7afd4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -99,7 +99,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Fixes -- **Eight more C++ links land on the right target.** `Ptr p(0);` for `using Ptr = Widget*;` declares a pointer, so it no longer counts as constructing a `Widget`. A class nested in a function-local class is as local to that function as the class around it. A class an included `.cc` file declares is seen by the file that includes it, and of two headers declaring one class an object creation takes the one it includes. A `using Base::foo;` in a nested class no longer counts as the outer class's. `NULL` is an integer zero, not a `nullptr`, when an overload is chosen. A header's unnamed namespace belongs to the files that include the header, not to every file. Two annotation-macro cases read as extraction reads them: `const int FOO_BAR = 1;` keeps its name, and `n * FOO_BAR::VALUE` stays a product. +- **More C++ links land on the right target.** `Ptr p(0);` for `using Ptr = Widget*;` declares a pointer, so it no longer counts as constructing a `Widget`. A class nested in a function-local class is as local to that function as the class around it. A class an included `.cc` file declares is seen by the file that includes it, and of two headers declaring one class an object creation takes the one it includes. A `using Base::foo;` in a nested class no longer counts as the outer class's. `NULL` is an integer zero, not a `nullptr`, when an overload is chosen. A header's unnamed namespace belongs to the files that include the header, not to every file. Two annotation-macro cases read as extraction reads them: `const int FOO_BAR = 1;` keeps its name, and `n * FOO_BAR::VALUE` stays a product. - **Syncing a changed file in a large Python project is faster.** When a Python class has no method of the name being linked (a test class that passes `Handler.__init__` as a value, say), the resolver checks every class with that method for one that derives from it. For each class whose base is a package import (`import unittest`, then `unittest.TestCase`) it parsed the whole file again, and its cache holds only a few files, so two such references took about a second each. A plain one-line `import a.b` is now read from its line; other forms still use the parse. In CPython a one-file sync takes a median 5.5 s instead of 7.7 s on Node and 5.2 s instead of 8.1 s on Bun (4 alternating runs per build, ranges not overlapping; the phase that retries unresolved references drops from 4.6 s to 2.3 s). A full index is within the run-to-run variation, and the graph is the same on CPython and Django (no edge differs). - **The same sync resolves each Python class's bases once.** The scan above resolved the base classes of every candidate again for each reference that needed it. The bases of a class depend only on its own header and file, so they are now kept for the rest of the run. In CPython the phase that retries unresolved references drops from a median 2.29 s to 2.08 s on Node (-9%, 4 alternating runs per build, ranges not overlapping) and a one-file sync by about 3% (5.33 s to 5.15 s on the second sync); on Bun the sync time shows no clear change. A full index is within the run-to-run variation, and no edge, node or unresolved reference differs on CPython or Django. - **Questions to the MCP server are faster in a project with no `codegraph.json`.** While ranking results, CodeGraph checked for the project's `codegraph.json` once per result path, and with no such file each check raised and caught an error. It now asks the file system without raising, so `codegraph_explore` and the other query tools spend less CPU on most projects. Editing, adding or deleting `codegraph.json` is still picked up without a restart (#491). From 402c4da9bf7c57f1cf4535834b420b4cfa9d6920 Mon Sep 17 00:00:00 2001 From: Aaron Queen Date: Wed, 14 Oct 2026 09:01:57 -0600 Subject: [PATCH 3/4] fix(resolve): follow alias chains, mask comments in aliases, read a using on a shared line, accept const after a type A chain of pointer aliases (using Ptr2 = Ptr) constructs nothing. An alias's text is read with comments masked. A class's own using-declaration is found when its first line also opens a namespace. A macro after a name that follows a const-qualified type (int const value_ GUARDED_BY(mu_);) is blanked, in the Rust port and in extraction. --- __tests__/cpp-resolver-edge-cases.test.ts | 54 +++++++++++++++++++ .../src/resolve/cpp_annotations.rs | 26 +++++++-- .../src/resolve/cpp_construction.rs | 4 +- .../src/resolve/cpp_implicit_this.rs | 5 +- codegraph-kernel/src/resolve/cpp_types.rs | 28 ++++++++-- src/extraction/languages/c-cpp.ts | 14 +++-- 6 files changed, 118 insertions(+), 13 deletions(-) diff --git a/__tests__/cpp-resolver-edge-cases.test.ts b/__tests__/cpp-resolver-edge-cases.test.ts index ec1fc4ca64..4f8fcb744d 100644 --- a/__tests__/cpp-resolver-edge-cases.test.ts +++ b/__tests__/cpp-resolver-edge-cases.test.ts @@ -69,6 +69,30 @@ describe('C++ object creation', () => { } }); + it('constructs no class for a chain of pointer aliases, and the class for an alias with a comment', async () => { + const cg = await indexed({ + 'widget.h': lines( + 'class Widget {', + ' public:', + ' Widget(int n) {}', + '};', + 'using Ptr = Widget*;', + 'using Ptr2 = Ptr;', + 'using Plain = Widget /* an object * not a pointer */;', + 'void Run() {', + ' Ptr2 q(0);', + ' Plain w(1);', + '}', + ), + }); + try { + expect(reached(cg, 'Run', 'instantiates', 9)).toEqual([]); + expect(reached(cg, 'Run', 'instantiates', 10)).toEqual(['Widget:1 @0.9']); + } finally { + cg.close(); + } + }); + it("does not see a class nested in a function's local class from another function", async () => { const cg = await indexed({ 'a_test.cc': lines( @@ -153,6 +177,31 @@ describe('C++ calls', () => { } }); + it("reads a class's using-declaration when its first line also opens a namespace", async () => { + const cg = await indexed({ + 'outer.h': lines( + 'template ', + 'class Base {', + ' public:', + ' void foo() {}', + '};', + 'void foo() {}', + 'namespace ns { template class Outer : public Base {', + ' public:', + ' using Base::foo;', + ' void run() {', + ' foo();', + ' }', + '}; }', + ), + }); + try { + expect(reached(cg, 'ns::Outer::run', 'calls', 11).map((r) => r.split(' ')[0])).toEqual(['Base::foo:4']); + } finally { + cg.close(); + } + }); + it('takes NULL for an integer zero, not for a nullptr', async () => { const cg = await indexed({ 'reset.h': lines( @@ -212,6 +261,11 @@ describe('C++ annotation blanking', () => { expect(blankCppDeclaratorAttributeMacros('static constexpr unsigned MAX_LEN = 4;')).toBe('static constexpr unsigned MAX_LEN = 4;'); }); + it('blanks a macro after a name that follows a const-qualified type', () => { + expect(blankCppDeclaratorAttributeMacros('int const value_ GUARDED_BY(mu_);')).toBe('int const value_ ' + ' '.repeat('GUARDED_BY(mu_)'.length) + ';'); + expect(blankCppDeclaratorAttributeMacros('Foo const volatile value_ GUARDED_BY(mu_);')).toBe('Foo const volatile value_ ' + ' '.repeat('GUARDED_BY(mu_)'.length) + ';'); + }); + it('leaves a qualified name after a product alone', () => { expect(blankCppPointerAnnotationMacros('consume(n * FOO_BAR::VALUE);')).toBe('consume(n * FOO_BAR::VALUE);'); }); diff --git a/codegraph-kernel/src/resolve/cpp_annotations.rs b/codegraph-kernel/src/resolve/cpp_annotations.rs index b0de7590d2..66a2b424fa 100644 --- a/codegraph-kernel/src/resolve/cpp_annotations.rs +++ b/codegraph-kernel/src/resolve/cpp_annotations.rs @@ -120,11 +120,23 @@ fn follows_declared_name(code: &[u8], last: usize) -> bool { if !is_word(c) { return false; } let mut j = i; while j > 0 && is_word(code[j - 1]) { j -= 1; } - let before = std::str::from_utf8(&code[j..i]).unwrap_or(""); - if before == "const" || before == "volatile" { + let mut before = std::str::from_utf8(&code[j..i]).unwrap_or(""); + // Qualifiers after the type read through: `int const volatile value_`, `Foo* const value_`. + while before == "const" || before == "volatile" { let mut k = j; while k > 0 && code[k - 1].is_ascii_whitespace() { k -= 1; } - return k > 0 && matches!(code[k - 1], b'*' | b'&'); + if k == 0 { return false; } + match code[k - 1] { + b'*' | b'&' => return true, + b'>' => return k < 2 || code[k - 2] != b'-', + c if is_word(c) => { + i = k; + j = k; + while j > 0 && is_word(code[j - 1]) { j -= 1; } + before = std::str::from_utf8(&code[j..i]).unwrap_or(""); + } + _ => return false, + } } if TYPE_KEYWORDS.contains(&before) { return true; } !before.as_bytes()[0].is_ascii_digit() && !is_keyword(before) @@ -244,4 +256,12 @@ mod tests { let out = cpp_without_annotations(text); assert!(!out.contains("PROTOBUF_NONNULL") && !out.contains("PROTOBUF_NULLABLE") && !out.contains("GUARDED_BY"), "{out}"); } + + #[test] + fn a_const_qualified_type_precedes_a_declared_name() { + let blanked = |text: &str| cpp_without_annotations(text).into_owned(); + assert_eq!(blanked("int const value_ GUARDED_BY(mu_);\n"), format!("int const value_ {};\n", " ".repeat("GUARDED_BY(mu_)".len()))); + assert_eq!(blanked("Foo const volatile value_ GUARDED_BY(mu_);\n"), format!("Foo const volatile value_ {};\n", " ".repeat("GUARDED_BY(mu_)".len()))); + assert_eq!(blanked("const int FOO_BAR = 1;\n"), "const int FOO_BAR = 1;\n"); + } } diff --git a/codegraph-kernel/src/resolve/cpp_construction.rs b/codegraph-kernel/src/resolve/cpp_construction.rs index 1e39ee8924..cb22b3c2c1 100644 --- a/codegraph-kernel/src/resolve/cpp_construction.rs +++ b/codegraph-kernel/src/resolve/cpp_construction.rs @@ -80,7 +80,7 @@ fn type_written_at(text: &str) -> Option { /// One of several declarations of a class: the one in `file` itself, then in its header, then the /// nearest by path. -fn pick(classes: Vec>, file: &str) -> Option> { +pub(super) fn pick(classes: Vec>, file: &str) -> Option> { if classes.len() <= 1 { return classes.into_iter().next(); } if let Some(own) = classes.iter().find(|n| n.file_path == file) { return Some(own.clone()); } let stem = |p: &str| p.rsplit_once('.').filter(|(head, _)| !head.ends_with('/')).map_or(p.to_string(), |(head, _)| head.to_string()); @@ -240,7 +240,7 @@ impl KernelResolver { } if current.kind == "type_alias" { // (A pointer or reference alias names no class to construct: `Ptr p(0);`.) - if self.cpp_alias_indirect(¤t) { return Ok(Lookup::NoClass); } + if self.cpp_alias_indirect(¤t, r) { return Ok(Lookup::NoClass); } if let Some(class) = self.cpp_expand_owner(¤t, r, 0, false, visible)? { current = class; } } return Ok(Lookup::Found(current, true)); diff --git a/codegraph-kernel/src/resolve/cpp_implicit_this.rs b/codegraph-kernel/src/resolve/cpp_implicit_this.rs index eda12119f7..337faf5947 100644 --- a/codegraph-kernel/src/resolve/cpp_implicit_this.rs +++ b/codegraph-kernel/src/resolve/cpp_implicit_this.rs @@ -404,7 +404,10 @@ impl KernelResolver { let from = (decl.start_line - 1).max(0) as usize; let to = (decl.end_line.max(decl.start_line) as usize).min(lines.len()); if from >= to { return false; } - let body = cpp_own_scope(&lines[from..to].join("\n")); + let mut own: Vec<&str> = lines[from..to].iter().map(String::as_str).collect(); + // (The class's own first line starts at the class: `namespace ns { class Outer : Base {`.) + own[0] = own[0].get(super::names::js_unit_to_byte(own[0], decl.start_column.max(0) as usize)..).unwrap_or(""); + let body = cpp_own_scope(&own.join("\n")); re!(r"\busing\s+((?:::\s*)?[A-Za-z_][\w:]*(?:\s*<[^;{}]*>)?)\s*::\s*([A-Za-z_]\w*)\s*;").captures_iter(&body).any(|m| { &m[2] == name && !m[1].trim_start().starts_with("namespace") && cpp_last_segment(strip_template_arguments(&m[1]).trim()) == base.name }) diff --git a/codegraph-kernel/src/resolve/cpp_types.rs b/codegraph-kernel/src/resolve/cpp_types.rs index 84e96415e0..984476068a 100644 --- a/codegraph-kernel/src/resolve/cpp_types.rs +++ b/codegraph-kernel/src/resolve/cpp_types.rs @@ -191,15 +191,35 @@ impl KernelResolver { Ok(false) } - /// Whether the alias `owner` names a pointer or a reference (`using Ptr = Widget*;`). - pub(super) fn cpp_alias_indirect(&mut self, owner: &KNode) -> bool { - self.cpp_alias_rhs(owner).is_some_and(|rhs| super::cpp_aliases::indirect_type(&rhs)) + /// Whether the alias `owner` names a pointer or a reference (`using Ptr = Widget*;`), itself or + /// through the aliases it names (`using Ptr2 = Ptr;`). + pub(super) fn cpp_alias_indirect(&mut self, owner: &KNode, r: &ResolveRefIn) -> bool { + let mut current = owner.clone(); + for _ in 0..8 { + let Some(rhs) = self.cpp_alias_rhs(¤t) else { return false }; + if super::cpp_aliases::indirect_type(&rhs) { return true; } + let named = re!(r"^(?:const|volatile)\s+").replace(&rhs, "").to_string(); + if !re!(r"^[A-Za-z_]\w*(?:::[A-Za-z_]\w*)*$").is_match(&named) { return false; } + // The alias it names, looked up from where this one is declared outward. + let mut scopes: Vec = Vec::new(); + let mut scope = current.qualified_name.as_str(); + while let Some(cut) = scope.rfind("::") { scope = &scope[..cut]; scopes.push(scope.to_string()); } + scopes.push(String::new()); + let mut next = None; + for scope in &scopes { + let Ok(found) = self.cpp_scope_types(scope, &named, r, 0) else { return false }; + if let Some(hit) = super::cpp_construction::pick(found, &r.file_path) { next = Some(hit); break; } + } + match next { Some(n) if n.kind == "type_alias" => current = (*n).clone(), _ => return false } + } + false } /// What the alias `owner` is declared as: the text after its `=`. fn cpp_alias_rhs(&mut self, owner: &KNode) -> Option { let source = self.read_file(&owner.file_path)?; - let mut lines = source.get((owner.start_line - 1).max(0) as usize..owner.end_line.max(0) as usize).unwrap_or(&[]).to_vec(); + // (From the code with comments masked: `using Plain = Widget /* an object * */;`.) + let mut lines = source.cpp_code_lines().get((owner.start_line - 1).max(0) as usize..owner.end_line.max(0) as usize).unwrap_or(&[]).to_vec(); if lines.is_empty() { return None; } let last = lines.len() - 1; let end = super::names::js_unit_to_byte(&lines[last], owner.end_column.max(0) as usize); diff --git a/src/extraction/languages/c-cpp.ts b/src/extraction/languages/c-cpp.ts index 8b33fc464c..45d6018152 100644 --- a/src/extraction/languages/c-cpp.ts +++ b/src/extraction/languages/c-cpp.ts @@ -791,11 +791,19 @@ function followsDeclaredName(code: string, last: number): boolean { if (!c || !/\w/.test(c)) return false; let j = i; while (j > 0 && /\w/.test(code[j - 1] as string)) j--; - const before = code.slice(j, i + 1); - if (before === 'const' || before === 'volatile') { + let before = code.slice(j, i + 1); + // Qualifiers after the type read through: `int const volatile value_`, `Foo* const value_`. + while (before === 'const' || before === 'volatile') { let k = j - 1; while (k >= 0 && /\s/.test(code[k] as string)) k--; - return code[k] === '*' || code[k] === '&'; + const p = code[k]; + if (p === '*' || p === '&') return true; + if (p === '>') return code[k - 1] !== '-'; + if (!p || !/\w/.test(p)) return false; + i = k; + j = k; + while (j > 0 && /\w/.test(code[j - 1] as string)) j--; + before = code.slice(j, i + 1); } if (CPP_TYPE_KEYWORDS.has(before)) return true; return /^[A-Za-z_]\w*$/.test(before) && !CPP_KEYWORDS.has(before); From d56b22aef1a8b1fe9e4995d5460147d18b82db5d Mon Sep 17 00:00:00 2001 From: Aaron Queen Date: Wed, 14 Oct 2026 09:21:39 -0600 Subject: [PATCH 4/4] fix(resolve): look up a global-qualified alias target from the global scope using Ptr3 = ::Ptr; was rejected before lookup, so the chain of pointer aliases was not followed and Ptr3 p(0) constructed the pointee. --- __tests__/cpp-resolver-edge-cases.test.ts | 9 ++++++--- codegraph-kernel/src/resolve/cpp_types.rs | 15 ++++++++++----- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/__tests__/cpp-resolver-edge-cases.test.ts b/__tests__/cpp-resolver-edge-cases.test.ts index 4f8fcb744d..cc2d5b1408 100644 --- a/__tests__/cpp-resolver-edge-cases.test.ts +++ b/__tests__/cpp-resolver-edge-cases.test.ts @@ -69,7 +69,7 @@ describe('C++ object creation', () => { } }); - it('constructs no class for a chain of pointer aliases, and the class for an alias with a comment', async () => { + it('constructs no class for a chain of pointer aliases (a global-qualified one too), and the class for an alias with a comment', async () => { const cg = await indexed({ 'widget.h': lines( 'class Widget {', @@ -78,16 +78,19 @@ describe('C++ object creation', () => { '};', 'using Ptr = Widget*;', 'using Ptr2 = Ptr;', + 'using Global = ::Ptr;', 'using Plain = Widget /* an object * not a pointer */;', 'void Run() {', ' Ptr2 q(0);', ' Plain w(1);', + ' Global g(0);', '}', ), }); try { - expect(reached(cg, 'Run', 'instantiates', 9)).toEqual([]); - expect(reached(cg, 'Run', 'instantiates', 10)).toEqual(['Widget:1 @0.9']); + expect(reached(cg, 'Run', 'instantiates', 10)).toEqual([]); + expect(reached(cg, 'Run', 'instantiates', 11)).toEqual(['Widget:1 @0.9']); + expect(reached(cg, 'Run', 'instantiates', 12)).toEqual([]); } finally { cg.close(); } diff --git a/codegraph-kernel/src/resolve/cpp_types.rs b/codegraph-kernel/src/resolve/cpp_types.rs index 984476068a..30e6f856fa 100644 --- a/codegraph-kernel/src/resolve/cpp_types.rs +++ b/codegraph-kernel/src/resolve/cpp_types.rs @@ -199,15 +199,20 @@ impl KernelResolver { let Some(rhs) = self.cpp_alias_rhs(¤t) else { return false }; if super::cpp_aliases::indirect_type(&rhs) { return true; } let named = re!(r"^(?:const|volatile)\s+").replace(&rhs, "").to_string(); - if !re!(r"^[A-Za-z_]\w*(?:::[A-Za-z_]\w*)*$").is_match(&named) { return false; } - // The alias it names, looked up from where this one is declared outward. + if !re!(r"^(?:::)?[A-Za-z_]\w*(?:::[A-Za-z_]\w*)*$").is_match(&named) { return false; } + // The alias it names, looked up from where this one is declared outward (a name + // written with a leading `::` only from the global scope). + let global = named.starts_with("::"); + let named = named.trim_start_matches("::"); let mut scopes: Vec = Vec::new(); - let mut scope = current.qualified_name.as_str(); - while let Some(cut) = scope.rfind("::") { scope = &scope[..cut]; scopes.push(scope.to_string()); } + if !global { + let mut scope = current.qualified_name.as_str(); + while let Some(cut) = scope.rfind("::") { scope = &scope[..cut]; scopes.push(scope.to_string()); } + } scopes.push(String::new()); let mut next = None; for scope in &scopes { - let Ok(found) = self.cpp_scope_types(scope, &named, r, 0) else { return false }; + let Ok(found) = self.cpp_scope_types(scope, named, r, 0) else { return false }; if let Some(hit) = super::cpp_construction::pick(found, &r.file_path) { next = Some(hit); break; } } match next { Some(n) if n.kind == "type_alias" => current = (*n).clone(), _ => return false }