fix(cpp): a member declared in an anonymous union, like protobuf's union { Impl_ _impl_; };, is found as a member of the class around it (#2731)

A class's data members are read from its own lines, skipping those of
the functions and classes nested in it. The index keeps an anonymous
union or struct as a nested `<anonymous>` node, so its lines were skipped
too, though C++ looks its members up as the enclosing class's
([class.union.anon]).

Every generated protobuf message keeps its fields in `union { Impl_
_impl_; };`, so a call through `_impl_` was guessed by the method's name:
- `this_._impl_.type_url_.Destroy()` reached `Arena::Destroy`;
- `_impl_._cached_size_.Get()` reached `FieldArenaRep::Get`;
- `_impl_._has_bits_.Clear()` reached the message's own `Clear`.

fmt's `value` keeps its argument in a union of `string`, `custom` and the
rest in the same way.

cppNestedSpans now leaves out an anonymous union or struct with no name
and no declarator (isCppAnonymousMembers):
- a bare `union {` or `struct {` head;
- `};` after the closing brace.
A union with a declarator, `union { … } u;`, keeps its members to itself,
and so does a partial specialization, which the index also keeps as
`<anonymous>` but whose head names its template.

A member-chain diagnostic on main 81fd0078 found the anonymous union
behind most of protobuf's untyped chains. Kit A/B against 81fd0078:

| repo | removed | added | retargeted | same target, now typed |
|---|---|---|---|---|
| protobuf | 2 | 54 | 349 | 231 |
| fmt | 0 | 0 | 1 | 1 |
| rocksdb | 0 | 0 | 2 | 0 |
| leveldb, redis | identical | | | |

- Retargets go from bare-name guesses to the member's class:
  ArenaStringPtr (Set, Get, IsDefault, Destroy), CachedSize::Get,
  HasBits::Clear, RepeatedPtrField, SourceContext and MessageLite.
- The 2 removals are `ptr.string_value->clear()` on a `std::string*`, which
  had reached `Map::clear`.
- fmt's and rocksdb's retargets pick the overload their argument fits now
  that a union member has a type: `map(value_.uint128_value)` takes
  `native_uint128`, and `Add(…, props.oldest_ancestor_time)` the `uint64_t`
  overload.
- About 45 changed sites read by hand were all right.

Mutations: dropping the nested-span exclusion, the declarator check or
the head check each fail the new tests. Dropping the name check survives,
because the head check covers named types; it stays as a cheap early exit.
A matching change to the caller's-declaration check left the kit A/B
byte-identical, so it was not kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Colby Mchenry
2026-10-21 06:55:34 +00:00
committed by GitHub
co-authored by Claude Opus 5.5
parent 81fd007850
commit 5994edb34e
3 changed files with 195 additions and 2 deletions
+23 -2
View File
@@ -13651,7 +13651,28 @@ function cppClassParenDepths(
return depths;
}
/** The lines of the functions and classes nested in the C++ class declaration `decl`, which ends on line `end`. */
/**
* Is the C or C++ declaration `n` an anonymous union or struct, with no name
* and no declarator, whose members are those of the class it is declared in?
* C++ looks them up as that class's own: protobuf's generated messages hold
* their fields in `union { Impl_ _impl_; };`, and fmt's `value` its argument
* in a union of `string`, `custom` and the rest. Not one with a declarator,
* `union { … } u;`, whose members are `u`'s, nor a specialization the index
* keeps as `<anonymous>`, whose head names its template.
*/
function isCppAnonymousMembers(n: Node, context: ResolutionContext): boolean {
if (n.name !== '<anonymous>' || (n.kind !== 'union' && n.kind !== 'struct')) return false;
const lines = context.getFileLines?.(n.filePath) ?? context.readFile(n.filePath)?.split(/\r?\n/);
const head = lines?.[n.startLine - 1]?.slice(n.startColumn ?? 0);
const foot = lines?.[(n.endLine ?? n.startLine) - 1];
return !!head && !!foot && /^\s*(?:union|struct)\s*\{/.test(head) && /\}\s*;\s*(?:\/\/.*)?$/.test(foot);
}
/**
* The lines of the functions and classes nested in the C++ class declaration
* `decl`, which ends on line `end` — not an anonymous union's or struct's,
* whose members are the class's own (isCppAnonymousMembers).
*/
function cppNestedSpans(decl: Node, end: number, context: ResolutionContext): Array<[number, number]> {
let memo = CPP_NESTED_SPANS.get(context);
if (!memo) CPP_NESTED_SPANS.set(context, (memo = new Map()));
@@ -13659,7 +13680,7 @@ function cppNestedSpans(decl: Node, end: number, context: ResolutionContext): Ar
if (!spans) {
spans = context.getNodesInFile(decl.filePath)
.filter((n) => n.id !== decl.id && n.startLine >= decl.startLine && (n.endLine ?? n.startLine) <= end &&
(isCppFunctionNode(n) || CPP_CLASS_KINDS.has(n.kind)))
(isCppFunctionNode(n) || (CPP_CLASS_KINDS.has(n.kind) && !isCppAnonymousMembers(n, context))))
.map((n): [number, number] => [n.startLine, n.endLine ?? n.startLine]);
memo.set(decl.id, spans);
}