diff --git a/CHANGELOG.md b/CHANGELOG.md index d5001316b..3d7e1d080 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -132,6 +132,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 and C++ tests, a name that several files declare now links to the one the test's own includes declare, when there is one: protocolbuffers/protobuf's C# generator tests call the `TestSourceDir()` that `googletest.h` declares, not another test helper's they never include; its upb tests call `upb_Arena_New()` and the rest of upb's C API in upb's own files, not the copies bundled for PHP and Ruby, which had left those calls unlinked; and facebook/rocksdb's C API test reaches `rocksdb_open` and its siblings in `db/c.cc`. Re-index C and C++ projects after upgrading. - In C and C++ code outside tests too, a name that several files declare now links to the one the file's own includes declare, when there is one: protocolbuffers/protobuf's Python extension calls functions like `upb_Encode()` in upb's own files rather than the copies bundled for PHP, and facebook/rocksdb's stress tool calls the `StringToHex()` its own header declares rather than `LDBCommand::StringToHex`. Re-index C and C++ projects after upgrading. +- In C and C++, a call of a function a header declares now links to its definition even when the file that defines it reaches that header only through other headers: protocolbuffers/protobuf's Python extension calls upb's reflection functions, like `upb_FieldDef_Number()`, in upb's own files rather than the copies bundled for PHP. Re-index C and 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. diff --git a/__tests__/cpp-translation-unit.test.ts b/__tests__/cpp-translation-unit.test.ts index 7c1f24bc0..3ba7f2453 100644 --- a/__tests__/cpp-translation-unit.test.ts +++ b/__tests__/cpp-translation-unit.test.ts @@ -7,7 +7,10 @@ * helpers.h declares, not `ImmutableFieldGenerator::HasHasbit`, a method * beside the caller. A source implements the header beside it (the * Python extension's pyext/message.cc is no implementation of - * google/protobuf/message.h), a header's inline overload reaches the + * google/protobuf/message.h), even one it reaches only through other + * headers (upb's reflection/field_def.c includes + * reflection/internal/field_def.h, which reaches reflection/field_def.h + * through def.h), a header's inline overload reaches the * definition its own source holds, and a call keeps a function outside the * unit that takes its arguments over one inside that can't (glog's * three-argument `GetStackTrace(…)` is not utilities.h's `GetStackTrace()`). @@ -44,6 +47,7 @@ int upb_Encode(const void* msg, int options, char** buf, size_t* size) { 'php/ext/google/protobuf/php-upb.h': `#include int upb_Encode(const void* msg, int options, char** buf, size_t* size); +int upb_FieldDef_Number(const void* f); `, 'php/ext/google/protobuf/php-upb.c': `#include "php-upb.h" @@ -51,6 +55,30 @@ int upb_Encode(const void* msg, int options, char** buf, size_t* size) { *size = 0; return 0; } + +int upb_FieldDef_Number(const void* f) { return 0; } +`, + // upb's reflection/field_def.c implements reflection/field_def.h, which + // it reaches only through its internal header and def.h. + 'upb/reflection/field_def.h': `typedef struct upb_FieldDef upb_FieldDef; + +int upb_FieldDef_Number(const upb_FieldDef* f); +`, + 'upb/reflection/def.h': `#include "upb/reflection/field_def.h" +`, + 'upb/reflection/internal/field_def.h': `#include "upb/reflection/def.h" + +int _upb_FieldDef_Index(const upb_FieldDef* f); +`, + 'upb/reflection/field_def.c': `#include "upb/reflection/internal/field_def.h" + +int upb_FieldDef_Number(const upb_FieldDef* f) { return 1; } +`, + 'python/convert.c': `#include "upb/reflection/def.h" + +int PyUpb_FieldNumber(const upb_FieldDef* f) { + return upb_FieldDef_Number(f); +} `, 'python/message.c': `#include "upb/wire/encode.h" @@ -344,6 +372,12 @@ describe('a C/C++ reference links what its translation unit declares', () => { expect(calls.some((c) => c.includes('php-upb'))).toBe(false); }); + it('reads a source as implementing the header beside it that it reaches through others', () => { + const calls = callsFrom('python/convert.c'); + expect(calls).toContain('upb_FieldDef_Number (upb/reflection/field_def.c)'); + expect(calls.some((c) => c.includes('php-upb'))).toBe(false); + }); + it('calls the free function its includes declare, not a class\'s method beside it', () => { const calls = callsFrom('src/google/protobuf/compiler/java/full/enum_field.cc'); expect(calls).toContain('google::protobuf::compiler::java::HasHasbit (src/google/protobuf/compiler/java/helpers.h)'); diff --git a/src/resolution/cpp-includers.ts b/src/resolution/cpp-includers.ts index b44439ff8..a9ff3472e 100644 --- a/src/resolution/cpp-includers.ts +++ b/src/resolution/cpp-includers.ts @@ -33,6 +33,8 @@ interface Include { interface Memo { /** Every C / C++ `#include`, by the file name it spells (`a/b.h` → `b.h`). */ byName: Map | null; + /** The same, by the stem of that name (`a/b.h` → `b`). */ + byStem: Map | null; /** Import node id → the indexed file its `#include` names (null: none). */ targets: Map; /** File → the files that include it, directly or through other includes. */ @@ -47,12 +49,17 @@ export function clearCppIncluderMemos(context: ResolutionContext): void { function memoFor(context: ResolutionContext): Memo { let memo = MEMO.get(context); - if (!memo) MEMO.set(context, (memo = { byName: null, targets: new Map(), includers: new Map() })); + if (!memo) MEMO.set(context, (memo = { byName: null, byStem: null, targets: new Map(), includers: new Map() })); return memo; } const isCFamily = (language: string): boolean => language === 'c' || language === 'cpp'; const fileName = (filePath: string): string => filePath.slice(Math.max(filePath.lastIndexOf('/'), filePath.lastIndexOf('\\')) + 1); +const stemOf = (name: string): string => { + const dot = name.lastIndexOf('.'); + return dot > 0 ? name.slice(0, dot) : name; +}; +const dirOf = (filePath: string): string => filePath.slice(0, filePath.lastIndexOf('/') + 1); const toInclude = (n: Pick): Include => ({ id: n.id, spec: n.name, file: n.filePath, language: n.language, line: n.startLine, column: n.startColumn }); @@ -91,6 +98,37 @@ function includesByName(context: ResolutionContext, memo: Memo): Map { + if (memo.byStem) return memo.byStem; + const byStem = new Map(); + for (const [name, includes] of includesByName(context, memo)) { + const stem = stemOf(name); + const list = byStem.get(stem); + if (list) list.push(...includes); + else byStem.set(stem, [...includes]); + } + memo.byStem = byStem; + return byStem; +} + +/** + * The headers in `filePath`'s folder named like it (`field_def.c` → + * `field_def.h`) that it includes, directly or through other headers. upb's + * reflection/field_def.c includes only reflection/internal/field_def.h, which + * reaches the reflection/field_def.h it implements through def.h. + */ +export function cppIncludedHeadersBeside(filePath: string, context: ResolutionContext): string[] { + const memo = memoFor(context); + const dir = dirOf(filePath); + const found: string[] = []; + for (const include of includesByStem(context, memo).get(stemOf(fileName(filePath))) ?? []) { + const target = includedFile(include, context, memo); + if (!target || target === filePath || found.includes(target) || dirOf(target) !== dir) continue; + if (cppIncluders(target, context).has(filePath)) found.push(target); + } + return found; +} + /** The indexed file the `#include` an import node records names, or null. */ export function cppIncludedFile(include: Node, context: ResolutionContext): string | null { return include.kind === 'import' && isCFamily(include.language) ? includedFile(toInclude(include), context, memoFor(context)) : null; diff --git a/src/resolution/name-matcher.ts b/src/resolution/name-matcher.ts index 72b27f3f9..3534074b9 100644 --- a/src/resolution/name-matcher.ts +++ b/src/resolution/name-matcher.ts @@ -18,7 +18,7 @@ import { cppAliasedClass, cppAliasedTypeName, cppAliasedTypeWritten, cppDeclares import { clearCppNamespaceMemos, cppMacroNamespaceFrames, cppMacroNamespaceHeads, cppNameMacros, cppNamespaceAliases } from './cpp-namespaces'; import { clearCppRawStringMemos, cppLiteralsBlanked, cppNonCodeLines } from './cpp-raw-strings'; import { cppClassWritten, type CppNameSite } from './cpp-supertypes'; -import { cppIncludedFile, cppIncluders } from './cpp-includers'; +import { cppIncludedFile, cppIncludedHeadersBeside, cppIncluders } from './cpp-includers'; import { isCFeatureVariant, isOffByDefault } from './c-feature-variants'; import { isTestPath } from '../search/query-utils'; import { isMinifiedContent } from '../extraction/generated-detection'; @@ -1620,7 +1620,10 @@ const CPP_STEM_HEADERS = new WeakMap>() * definitions implement. One beside it, when it includes one, is the only * such header: protobuf's pyext/message.cc implements pyext/message.h, not * the google/protobuf/message.h it also includes; fmt's src/format.cc, with - * none beside it, implements include/fmt/format.h. + * none beside it, implements include/fmt/format.h. It may include that one + * through other headers only: upb's reflection/field_def.c includes + * reflection/internal/field_def.h, which reaches reflection/field_def.h + * through def.h (cppIncludedHeadersBeside). */ function cppSameStemHeaders(filePath: string, context: ResolutionContext): string[] { let memo = CPP_STEM_HEADERS.get(context); @@ -1635,7 +1638,8 @@ function cppSameStemHeaders(filePath: string, context: ResolutionContext): strin if (header && header !== filePath && fileStem(header) === stem) found.push(header); } const dir = filePath.slice(0, filePath.lastIndexOf('/') + 1); - const beside = found.filter((header) => header.slice(0, header.lastIndexOf('/') + 1) === dir); + let beside = found.filter((header) => header.slice(0, header.lastIndexOf('/') + 1) === dir); + if (beside.length === 0) beside = cppIncludedHeadersBeside(filePath, context); headers = beside.length > 0 ? beside : found; memo.set(filePath, headers); }