diff --git a/CHANGELOG.md b/CHANGELOG.md index b3fac18095..b72616c1ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,7 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). - In C++, a name written in a googletest `TEST_F` or `TEST_P` body now finds the test fixture's nested types, those of its base classes included, before anything in the surrounding namespace, as C++ does for the member function such a body is: facebook/rocksdb's `new TestUserDefinedIndexFactory()` links to the class nested in the fixture's base. A class declared inside a test body, or inside any function, is now only found from that function, so the local classes many RocksDB tests each declare under one name, like `FakeWF` or `MyManager`, no longer stand in for one another, and a call like `fwf->SetIOError(true)` reaches the `FakeWF` its own test declares. Re-index C++ projects after upgrading. - In C++, a declaration continued on a line that starts with `*`, like facebook/rocksdb's `const auto& el =` over `*reinterpret_cast*>(jhandle);`, is now read, so calls such as `el->OnFlushCompleted(…)` link to `EventListener`'s methods; such a line used to be skipped as part of a comment. Code commented out in a `/* … */` block, and the text of a string literal, like protocolbuffers/protobuf's `[[deprecated("Please use SpaceUsedLong() instead")]]`, are no longer read as declarations, which had sent a call to the wrong method or to none. Re-index C++ projects after upgrading. - In C++, a call with a comparison or a shift among its arguments, like protocolbuffers/protobuf's `p->Emit({{"declare", has_bit_word_index < 0 ? … : ""}, …}, "…")` or `WriteVarint(num << 3, s)`, is now read with the right number of arguments, so it links to the overload that fits rather than one taking fewer. A parameter or variable whose type has template arguments is now read even after another such parameter on the same line, like facebook/rocksdb's `const autovector& cf_ids`, or with empty ones, like fmt's `parse_context<>& ctx`: calls on it link to that class's methods, and calls on a `std::vector` declared that way no longer link to a project class's method of the same name. Re-index C++ projects after upgrading. +- In C++, a call passing `nullptr` now links to the overload that takes a `std::nullptr_t`, as C++ picks it, rather than one taking a pointer, and a call no longer links to an overload declared `= delete`, which no call that compiles can reach: protocolbuffers/protobuf's `ptr.reset(nullptr)` on an `Arena::UniquePtr` reaches `reset(std::nullptr_t)`, and `field.Add("foo")` on a `RepeatedPtrField` reaches `Add(Element&&)` instead of the deleted `Add(const Element&)`. Re-index C++ projects after upgrading. - Framework support now switches on when an app's `package.json` sits three or more folders deep, as with an ASP.NET solution's Angular or React app in `src/Web/ClientApp/`, or a React app in a workspace folder like `web/ui/mantine-ui/`. Before, only the project root and the first two folder levels were read, so frameworks such as Angular, React Router, Vue Router, SvelteKit and TanStack Router could go undetected there: an Angular app's routes, its `routerLink` and `router.navigate(…)` navigation and its template's event bindings were missing, so a handler used only from a `(click)` binding had no callers, and a React app's `navigate('/login')` calls didn't reach their route. Re-index projects laid out this way after upgrading. - In a repository that mixes languages, a web framework's naming conventions now link only code written in that framework's own languages. Before, Express's also linked Go and Python code: in etcd a Go method's `*zap.Logger` result type linked to the method itself, and in Harbor a Python `self.jaeger.validate()` call linked to another class's `validate`. Links that cross languages on purpose, like React Native and Expo native modules, Swift and Objective-C, Drupal's `routing.yml`, SvelteKit's `$lib/…` imports in `.ts` files and a Razor page's `@model`, are unchanged. - In C and C++, a call written from the global scope, like `::leveldb::RepairDB(…)` or `::hpb::CreateMessage(…)`, now links to the function of exactly that name, and one through a namespace alias, like protobuf's `::_pbi::…` calls, to the function in the aliased namespace. Before, these calls were dropped unless the repository also held Swift and Objective-C files, and even then a name spelled out in full, like `::google::protobuf::internal::TSanWrite(…)`, linked to nothing, while `::operator delete(…)` linked to one class's own `operator delete`. A global call never links to a function or method of the same name inside a namespace or class, such as a wrapper `open()` that calls the C library's `::open(…)`, and `::std::…` and `::testing::…` calls stay unlinked unless that library's source is part of your project. Re-index C and C++ projects after upgrading. diff --git a/__tests__/cpp-nullptr-deleted-overloads.test.ts b/__tests__/cpp-nullptr-deleted-overloads.test.ts new file mode 100644 index 0000000000..7c30d5ad69 --- /dev/null +++ b/__tests__/cpp-nullptr-deleted-overloads.test.ts @@ -0,0 +1,131 @@ +/** + * Two ways a C++ call reached an overload no call that compiles reaches: + * - A `nullptr` argument fitted any pointer parameter as well as it could fit + * anything, and a `std::nullptr_t` parameter no better than a class, so + * protocolbuffers/protobuf's `ptr.reset(nullptr)` on an + * `Arena::UniquePtr` took `reset(T*)` over `reset(std::nullptr_t)`. + * `nullptr` is a `std::nullptr_t` exactly, and a pointer by conversion. + * - That `reset(T*)` is declared `= delete`. A deleted overload is now taken + * only when every overload is. + */ +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'; + +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-nullptr-')); + 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 the calls at `line` of the function or method `qualifiedName` reach, as `target:its first line @confidence`. */ +function callsAt(cg: CodeGraph, qualifiedName: string, line: number): string[] { + const from = [...cg.getNodesByKind('method'), ...cg.getNodesByKind('function')].find((n) => n.qualifiedName === qualifiedName)!; + return cg.getOutgoingEdgesFrom([from.id], ['calls']) + .filter((e) => 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++ overloads for nullptr, and deleted ones', () => { + it("takes protobuf's reset(std::nullptr_t) for reset(nullptr), not the deleted reset(T*)", async () => { + const cg = await indexed({ + 'arena.h': lines( + '#include ', + 'namespace google {', + 'namespace protobuf {', + 'class Arena {', + ' public:', + ' template ', + ' class UniquePtr {', + ' public:', + ' void reset() {}', + ' void reset(std::nullptr_t) {}', + ' void reset(T* PROTOBUF_NULLABLE) = delete;', + ' };', + '};', + '} // namespace protobuf', + '} // namespace google', + ), + 'arena_unittest.cc': lines( + '#include "arena.h"', + 'namespace google {', + 'namespace protobuf {', + 'void Reset() {', + ' Arena::UniquePtr ptr;', + ' ptr.reset(nullptr);', + ' ptr.reset();', + '}', + '} // namespace protobuf', + '} // namespace google', + ), + }); + try { + expect(callsAt(cg, 'google::protobuf::Reset', 6)).toEqual(['google::protobuf::Arena::UniquePtr::reset:10 @0.9']); + expect(callsAt(cg, 'google::protobuf::Reset', 7)).toEqual(['google::protobuf::Arena::UniquePtr::reset:9 @0.9']); + } finally { + cg.close(); + } + }); + + it('fits nullptr to std::nullptr_t before a pointer, and 0 to an int before both', async () => { + const cg = await indexed({ + 'canvas.h': lines( + '#include ', + 'class Widget {};', + 'class Canvas {', + ' public:', + ' void Set(Widget* w) {}', + ' void Set(std::nullptr_t) {}', + ' void Set(int v) {}', + ' void Run() {', + ' Set(nullptr);', + ' Set(0);', + ' }', + '};', + ), + }); + try { + expect(callsAt(cg, 'Canvas::Run', 9)).toEqual(['Canvas::Set:6 @0.9']); + expect(callsAt(cg, 'Canvas::Run', 10)).toEqual(['Canvas::Set:7 @0.9']); + } finally { + cg.close(); + } + }); + + it('takes no deleted overload while another fits, reading each declaration to its own end', async () => { + const cg = await indexed({ + 'meter.h': lines( + 'class Meter {', + ' public:', + ' void Scale(double factor) = delete;', + ' void Scale(int factor) {}', + ' void Scale(', + ' long factor) = delete;', + ' void Run() { Scale(2); }', + '};', + ), + }); + try { + expect(callsAt(cg, 'Meter::Run', 7)).toEqual(['Meter::Scale:4 @0.9']); + } finally { + cg.close(); + } + }); +}); diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index e2f0b73307..df43979eb3 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -10637,9 +10637,13 @@ function cppScopeMethod(scope: CppClassScope, name: string, receiver: 'bare' | ' * (cppOverloadFit), then, among overloads those fit alike, by the types the * arguments have where the call reads them (cppArgumentTypeFit). rocksdb's * `Insert(ch)` on a `char ch` is `Insert(char key, …)`, not `Insert(const - * std::string& key, …)`. + * std::string& key, …)`. A deleted overload (`void reset(T*) = delete;`) is + * none a call that compiles reaches, so it is only taken when every one is. */ -function cppOverloadFor(overloads: Node[], name: string, ref: UnresolvedRef, context: ResolutionContext): Node { +function cppOverloadFor(candidates: Node[], name: string, ref: UnresolvedRef, context: ResolutionContext): Node { + if (candidates.length === 1) return candidates[0]!; + const callable = candidates.filter((n) => !isCppDeletedFunction(n, name, context)); + const overloads = callable.length > 0 ? callable : candidates; if (overloads.length === 1) return overloads[0]!; const args = cppCallArguments(ref, name, context); if (!args) return overloads[0]!; @@ -10661,6 +10665,27 @@ function cppOverloadFor(overloads: Node[], name: string, ref: UnresolvedRef, con return best; } +const CPP_DELETED_FUNCTIONS = new WeakMap>(); + +/** + * Whether the C++ function or method `n`, named `name`, is declared deleted: + * from its name to the declaration's `;` or body, it says `= delete` + * (protobuf's `void reset(T* PROTOBUF_NULLABLE) = delete;`). + */ +function isCppDeletedFunction(n: Node, name: string, context: ResolutionContext): boolean { + let memo = CPP_DELETED_FUNCTIONS.get(context); + if (!memo) CPP_DELETED_FUNCTIONS.set(context, (memo = new Map())); + const hit = memo.get(n.id); + if (hit !== undefined) return hit; + const lines = context.getFileLines?.(n.filePath) ?? context.readFile(n.filePath)?.split(/\r?\n/) ?? []; + const text = lines.slice(n.startLine - 1, Math.min(lines.length, n.startLine + 11)).join('\n'); + const at = text.search(new RegExp(`\\b${name.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')}\\s*\\(`)); + const declaration = at < 0 ? '' : text.slice(at).split(/[;{]/)[0]!; + const deleted = /=\s*delete\b/.test(declaration); + memo.set(n.id, deleted); + return deleted; +} + /** What a C++ call's argument is: a literal's kind, or the type a name or expression has as written. */ type CppArgumentType = { kind: 'char' | 'number' | 'zero' | 'bool' | 'null' } | { kind: 'type'; text: string }; @@ -10701,7 +10726,7 @@ function cppArgumentType(arg: string, ref: UnresolvedRef, context: ResolutionCon } /** What a C++ type is, for an argument to fit a parameter of it. */ -type CppTypeCategory = 'string' | 'cstring' | 'char' | 'number' | 'bool' | 'pointer' | 'class'; +type CppTypeCategory = 'string' | 'cstring' | 'char' | 'number' | 'bool' | 'pointer' | 'null' | 'class'; const CPP_CHAR_TYPES: ReadonlySet = new Set(['char', 'wchar_t', 'char8_t', 'char16_t', 'char32_t']); const CPP_NUMBER_TYPES: ReadonlySet = new Set([ @@ -10710,12 +10735,16 @@ const CPP_NUMBER_TYPES: ReadonlySet = new Set([ ]); const CPP_STRING_TYPES: ReadonlySet = new Set(['string', 'wstring', 'string_view', 'wstring_view', 'basic_string', 'basic_string_view']); -/** The category of the written C++ type `type`: a pointer's, a string's, a character's, a number's, a class's. */ +/** + * The category of the written C++ type `type`: a pointer's, a string's, a + * character's, a number's, `std::nullptr_t`'s, a class's. + */ function cppTypeCategory(type: string): CppTypeCategory { const flat = stripCppTemplateArguments(type).replace(/\b(?:const|volatile|struct|class|typename|enum)\b/g, ' ').replace(/&+/g, ' ').trim(); const words = flat.replace(/\*/g, ' ').trim().split(/\s+/); const last = cppLastSegment(words[words.length - 1] ?? ''); if (flat.includes('*')) return CPP_CHAR_TYPES.has(last) && (flat.match(/\*/g) ?? []).length === 1 ? 'cstring' : 'pointer'; + if (last === 'nullptr_t') return 'null'; if (last === 'bool') return 'bool'; if (CPP_CHAR_TYPES.has(last) && words.every((w) => CPP_CHAR_TYPES.has(w) || w === 'signed' || w === 'unsigned')) return 'char'; if (CPP_NUMBER_TYPES.has(last) || words.every((w) => CPP_NUMBER_TYPES.has(w))) return 'number'; @@ -10744,13 +10773,17 @@ function cppArgumentTypeFit(n: Node, name: string, types: Array> = { - char: { char: 3, number: 1, bool: 0, string: -3, cstring: -3, pointer: -3, class: 0 }, - number: { number: 2, char: 1, bool: 0, string: -3, cstring: -3, pointer: -3, class: 0 }, - zero: { number: 2, char: 1, bool: 0, pointer: 1, cstring: 1, string: -3, class: 0 }, - bool: { bool: 3, number: 1, char: 0, string: -3, cstring: -3, pointer: -3, class: 0 }, - null: { pointer: 3, cstring: 3, class: 1, string: -3, char: -3, number: -3, bool: -1 }, + char: { char: 3, number: 1, bool: 0, string: -3, cstring: -3, pointer: -3, null: -3, class: 0 }, + number: { number: 2, char: 1, bool: 0, string: -3, cstring: -3, pointer: -3, null: -3, class: 0 }, + zero: { number: 2, char: 1, bool: 0, pointer: 1, cstring: 1, null: 1, string: -3, class: 0 }, + bool: { bool: 3, number: 1, char: 0, string: -3, cstring: -3, pointer: -3, null: -3, class: 0 }, + null: { null: 4, pointer: 3, cstring: 3, class: 1, string: -3, char: -3, number: -3, bool: -1 }, }; /** How an argument written as type `arg` fits a parameter of type `param` (of category `category`). */ @@ -11957,6 +11990,7 @@ export function clearNameMatcherMemos(context: ResolutionContext): void { CPP_USING_MEMBERS.delete(context); CPP_MEMBER_MACRO_ARGS.delete(context); CPP_OVERLOAD_PARAMETERS.delete(context); + CPP_DELETED_FUNCTIONS.delete(context); CPP_LOOP_ELEMENTS.delete(context); CPP_AUTO_LOCALS.delete(context); CPP_SCOPE_NAMES.delete(context);