Skip to content

[clang][P2996] Make parameter-name queries independent of instantiation state - #324

Merged
zebullax merged 1 commit into
bloomberg:p2996from
Cfretz244:reflect-param-name-instantiation-state
Jul 28, 2026
Merged

[clang][P2996] Make parameter-name queries independent of instantiation state#324
zebullax merged 1 commit into
bloomberg:p2996from
Cfretz244:reflect-param-name-instantiation-state

Conversation

@Cfretz244

Copy link
Copy Markdown

Fixes #322.

The P3096 consistency walk in getParameterName (report no identifier when redeclarations disagree on a parameter name) ran over the function's redeclaration chain — but for an instantiated member, instantiating the out-of-line definition REPLACES the parameters on the same FunctionDecl without adding a redeclaration, so the walk saw exactly one name: whichever redeclaration happened to be instantiated last. The same query on the same entity answered differently depending on instantiation state, and two TUs reflecting the same entity could disagree (field shape: Eigen's DenseBase::setConstant, declared with value, defined with val).

Fix: walk the template instantiation pattern's declaration chain, which carries the in-class declaration and the out-of-line definition regardless of instantiation state. Skipped when the pattern contains a parameter pack (its parameter list does not line up index-for-index with the instantiation's; pack-substituted parameters were already filtered earlier in the function). An inconsistently-named parameter now deterministically has no identifier; consistently-named parameters keep their name.

Adds libcxx/test/std/experimental/reflection/param-name-consistency-instantiation.pass.cpp: the pre-fix flip trigger (explicit instantiation of the definition before the query) plus a consistently-named control.

Validated on a Release+assertions AArch64/macOS build of this branch: the repro flips answers at the base commit and is deterministic with the fix; clang/test/Reflection parity with base; found and exercised by differential surface-comparison of two independently generated Python binding modules over Eigen.

…on state

The P3096 consistency walk in getParameterName runs over the function's
redeclaration chain and reports no identifier when redeclarations disagree.
For an instantiated member, however, instantiating the out-of-line
definition REPLACES the parameters on the same FunctionDecl -- no
redeclaration is added -- so the walk saw exactly one name: whichever
redeclaration happened to be instantiated last. The same query on the same
entity then answered differently depending on instantiation state, and two
translation units reflecting the same entity could disagree (field shape:
Eigen declares DenseBase::setConstant(const Scalar& value) and defines it
with 'val'; identifier_of flipped between the two depending on whether
binding lambdas had odr-used the member).

Walk the template instantiation pattern's declaration chain instead, which
carries the in-class declaration and the out-of-line definition regardless
of instantiation state. Skipped when the pattern contains a parameter pack
(its parameter list does not line up index-for-index with the
instantiation's; pack-substituted parameters are already filtered earlier).
An inconsistently-named parameter now deterministically has no identifier;
consistently-named parameters keep their name.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@zebullax zebullax left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix goes in the right direction, but it skips the pattern walk altogether when the function has a parameter pack anywhere, regardless of whether the parameter being queried is itself pack-derived.

template <class T> struct P {
    template <class... Args> void h(int value, Args... rest);
};
template <class T> template <class... Args>
void P<T>::h(int val, Args... rest) {}

template void P<int>::h<float>(int, float);

// The non-pack parameter "value"/"val" is inconsistently named,
// but has_identifier still returns true (reports "val"):
static_assert(!has_identifier(parameters_of(^^P<int>::h<float>)[0])); // FAILS

The pack guard (none_of(...isParameterPack())) should probably be narrowed.

@zebullax

Copy link
Copy Markdown

@Cfretz244 per above

@zebullax

Copy link
Copy Markdown

To make progress, i ll merge the above (incomplete) then push a delta.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants