Skip to content

Accept named constant entities as non-type template arguments - #1074

Open
conrade-ctc wants to merge 6 commits into
compiler-research:mainfrom
conrade-ctc:upstream-nttp-named-args
Open

Accept named constant entities as non-type template arguments#1074
conrade-ctc wants to merge 6 commits into
compiler-research:mainfrom
conrade-ctc:upstream-nttp-named-args

Conversation

@conrade-ctc

@conrade-ctc conrade-ctc commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Instantiating a template through cppyy's string subscript with the name of a constexpr variable or enum constant fails on the CppInterOp-based stack:

cppyy.cppdef("template <int N> class C {}; constexpr int IntArg = 42;")
cppyy.gbl.C["IntArg"]   # error: template argument for non-type template
                        # parameter must be an expression

cling-based cppyy accepted this (the argument was pasted textually, so name lookup made it an expression). On this stack the name reaches InstantiateTemplate as a TemplateArgInfo whose m_Type is the entity's declared type and whose m_IntegralValue is a non-numeric string — which the current code feeds straight to llvm::APSInt (an assert/UB on non-digit input).

This PR:

  • Factors the TemplateArgInfoTemplateArgument conversion shared by InstantiateTemplate and BestOverloadFunctionMatch into one helper.
  • Keeps numeric m_IntegralValue on the existing APSInt path.
  • Treats a non-numeric m_IntegralValue as the (possibly qualified) name of a constant entity: it is resolved (a GetNamed walk that, unlike GetScopeFromCompleteName, also finds variables and enum constants) and wrapped in a DeclRefExpr, so Sema converts it exactly like a written argument — constant evaluation for integral parameters, array-to-pointer decay for const char* parameters, prvalue handling for enum constants.
  • Treats a non-numeric m_IntegralValue naming a class/alias template as a template-template argument (TemplateName).
  • Fails cleanly (null result) for unknown names or non-constant entities, with Sema's proper diagnostics.

Companion PR: compiler-research/cppyy-backend#219 makes AppendTypesSlow produce these name-valued TemplateArgInfos (today it resolves such names to their type via Cpp::GetType, which is how the bad argument arises). Related discussion on the string-argument surface: compiler-research/cppyy-backend#137 (comment)

Tests: new ScopeReflection_InstantiateTemplateNamedNTTPArg covers a constexpr variable, a namespace-qualified name, an enum constant, char-array decay for a const char* parameter, and unknown-name failure; ScopeReflection_InstantiateTemplateTemplateArg covers plain/qualified/alias template-template arguments and null-info failure. check-cppinterop green.

Out of scope (intentionally): arbitrary constant expressions in strings ("IntArg + 1", "&obj", "'x'") — only (possibly qualified) names of constant entities, templates, and integer literals are resolved.

🤖 Done with the help of Claude Code (Fable 5, human in the loop)

Adds Cpp::SupportsNamedTemplateArguments() so callers can probe for this feature.

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.79%. Comparing base (9bbdbb2) to head (15fbfa4).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1074      +/-   ##
==========================================
+ Coverage   87.74%   87.79%   +0.05%     
==========================================
  Files          23       23              
  Lines        6429     6458      +29     
==========================================
+ Hits         5641     5670      +29     
  Misses        788      788              
Files with missing lines Coverage Δ
lib/CppInterOp/CppInterOp.cpp 90.62% <100.00%> (+0.07%) ⬆️
Files with missing lines Coverage Δ
lib/CppInterOp/CppInterOp.cpp 90.62% <100.00%> (+0.07%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread lib/CppInterOp/CppInterOp.cpp
Comment thread lib/CppInterOp/CppInterOp.cpp
Comment thread unittests/CppInterOp/ScopeReflectionTest.cpp
@conrade-ctc

Copy link
Copy Markdown
Collaborator Author

A follow-up report on the same stack showed template-template arguments were still broken: a string naming a class template raised "Cannot find Templated Arg" (a template has no QualType, so the old type-only path had nothing to carry). The new commit extends the named-entity path: a non-numeric value naming a class/alias template now becomes a Template-kind TemplateArgument (TemplateName), converted by Sema like a written argument.

Probing the argument matrix around this also surfaced two hardening fixes, included in the commit:

  • IsEnumType dereferenced a null QualType (reachable from cppyy when a TemplateProxy object is used as a template argument — a segfault in practice); it now returns false.
  • A TemplateArgInfo with null m_Type and no value now fails cleanly instead of building a TemplateArgument from a null type.

New unit tests: ScopeReflection_InstantiateTemplateTemplateArg (plain, namespace-qualified, and alias templates; unknown-name and null-info failures) plus negative-literal coverage on the NTTP test and a null check in EnumReflection_IsEnumType.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread lib/CppInterOp/CppInterOp.cpp

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread lib/CppInterOp/CppInterOp.cpp Outdated

@vgvassilev vgvassilev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@github-actions

Copy link
Copy Markdown
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@conrade-ctc
conrade-ctc force-pushed the upstream-nttp-named-args branch from 7850faa to 981d406 Compare August 21, 2026 21:04
Comment on lines +2478 to +2486
llvm::StringRef Value(Info.m_IntegralValue);
llvm::StringRef Digits = Value;
(void)(Digits.consume_front("-") || Digits.consume_front("+"));
if (!ArgTy.isNull() && !Digits.empty() &&
Digits.find_first_not_of("0123456789") == llvm::StringRef::npos) {
auto Res = llvm::APSInt(Value);
Res = Res.extOrTrunc(S.getASTContext().getIntWidth(ArgTy));
return TemplateArgument(S.getASTContext(), Res, ArgTy);
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On a second thought -- can we check if clang doesn't already have such conversion logic -- iirc there was something like that because we need to convert literals in the language.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It does, and we already rely on it: every argument built here goes through CheckTemplateArgumentList, which applies the converted-constant-expression rules (CheckTemplateArgument -> BuildConvertedConstantExpression). The helper only builds the reference. It now uses Sema::BuildDeclarationNameExpr for that, so clang also picks the value kind and type - the enum-constant special case is gone. The numeric branch keeps the pre-existing extOrTrunc: the API accepts out-of-range literals that a strict conversion would reject, and Sema still re-checks the truncated value.

return TemplateArgument(S.getASTContext(), Res, ArgTy);
}

Decl* Named = GetNamedFromCompleteName(Value.str());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe we do not need to do parsing here. We should have the access to the declaration through ArgTy.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ArgTy is the template parameter's type (int in the tests), so it cannot identify the entity - many declarations share one type, and a QualType never reaches a ValueDecl. For template-template arguments it is null by design. The name is the only handle the API carries, so I think the lookup should stay. The helper mirrors GetScopeFromCompleteName but also finds non-scope decls.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread unittests/CppInterOp/FunctionReflectionTest.cpp Outdated
Emery Conrad added 6 commits September 2, 2026 08:04
A non-numeric TemplateArgInfo value now names a constexpr variable or
enum constant: it is resolved and passed to Sema as a DeclRefExpr, so
conversion (constant evaluation, array-to-pointer decay) matches a
written argument. Shared by InstantiateTemplate and
BestOverloadFunctionMatch, which previously fed the string to APSInt.

Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
A non-numeric value naming a class/alias template becomes a
Template-kind argument (TemplateName). Fuzzing the arg matrix also
found: IsEnumType crashed on a null type, and a TemplateArgInfo with
null m_Type and no value reached TemplateArgument -- both now fail
cleanly.

Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
BestOverloadFunctionMatch leaked Exprs on the failed-template-arg early
return; own it with unique_ptr. Add the direct includes include-cleaner
asked for: CppInterOpTypes.h (TemplateArgInfo) in the lib, Unwrap.h
(HandleTypesTest precedent) and DeclTemplate.h in ScopeReflectionTest.

Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
The tidy bot flags unique_ptr<WrapperExpr[]> as a C-style array
(cppcoreguidelines-avoid-c-arrays); a sized vector keeps the same
placement-new pattern and early-return safety.

Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
Replace the manual DeclRefExpr construction with BuildDeclarationNameExpr.
Clang selects the value kind and type as it does for written code.
Sema still converts the argument in CheckTemplateArgumentList.

Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
Downstream code can query the named-argument support at run time.

Co-developed-with-the-help-of: Claude Code (Fable 5, human in the loop)
@conrade-ctc
conrade-ctc force-pushed the upstream-nttp-named-args branch from 01c9e38 to 15fbfa4 Compare September 2, 2026 13:31
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