fix(c,cpp): a source implements the header beside it that it reaches through other headers (#2693)

upb's reflection sources include only their internal header:
upb/reflection/field_def.c includes upb/reflection/internal/field_def.h,
which reaches the upb/reflection/field_def.h it implements through def.h.
So the translation-unit rule of PR 2684 never read field_def.c as that
header's implementation, and 343 calls from protobuf's Python extension,
plus 88 from its Lua binding, still went to the copy of upb that the PHP
extension bundles in php-upb.c.

When a source includes no header named like it in its own folder, one
there that it includes through other headers counts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-20 01:00:25 +00:00
committed by GitHub
parent 1b12bf5b49
commit 6f8e6caad9
4 changed files with 82 additions and 5 deletions
+1
View File
@@ -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<std::shared_ptr<EventListener>*>(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<uint32_t>& 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.
+35 -1
View File
@@ -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 <stddef.h>
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)');
+39 -1
View File
@@ -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<string, Include[]> | null;
/** The same, by the stem of that name (`a/b.h` → `b`). */
byStem: Map<string, Include[]> | null;
/** Import node id → the indexed file its `#include` names (null: none). */
targets: Map<string, string | null>;
/** 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<Node, 'id' | 'name' | 'filePath' | 'language' | 'startLine' | 'startColumn'>): 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<string, Inc
return byName;
}
function includesByStem(context: ResolutionContext, memo: Memo): Map<string, Include[]> {
if (memo.byStem) return memo.byStem;
const byStem = new Map<string, Include[]>();
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;
+7 -3
View File
@@ -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<ResolutionContext, Map<string, string[]>>()
* 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);
}