[Clang][Sema] Use StructuralValues to model dependent NTTP arguments

[zyn0217]

This patch takes Richard's approach of no longer modeling dependent NTTP arguments with TemplateParamObjectDecls. Clang used to do so, which left behind a problem in that we might mess up dependent and non-dependent arguments that boil down to the same canonical type because there's a default argument on the NTTP.

The problem of "canonical expression" is still present because this patch doesn't touch the profiling part. Namely, #92292 seems different.

Fixes #84052

[llvmbot]

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

This patch takes Richard's approach of no longer modeling dependent NTTP arguments with TemplateParamObjectDecls. Clang used to do so, which left behind a problem in that we might mess up dependent and non-dependent arguments that boil down to the same canonical type because there's a default argument on the NTTP.

The problem of "canonical expression" is still present because this patch doesn't touch the profiling part. Namely, #92292 seems different.

Fixes #84052

---

Full diff: https://github.com/llvm/llvm-project/pull/93556.diff

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/AST/TemplateBase.cpp (+6-1)
• (modified) clang/test/SemaTemplate/temp_arg_nontype_cxx2c.cpp (+22-1)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 49ab222bec405..722efd08721c3 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -710,6 +710,7 @@ Bug Fixes to C++ Support
 - Correctly treat the compound statement of an ``if consteval`` as an immediate context. Fixes (#GH91509).
 - When partial ordering alias templates against template template parameters,
   allow pack expansions when the alias has a fixed-size parameter list. Fixes (#GH62529).
+- Clang no longer models dependent NTTP arguments as ``TemplateParamObjectDecl``s. Fixes (#GH84052).
 
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/clang/lib/AST/TemplateBase.cpp b/clang/lib/AST/TemplateBase.cpp
index 3310d7dc24c59..e6b23eddb5888 100644
--- a/clang/lib/AST/TemplateBase.cpp
+++ b/clang/lib/AST/TemplateBase.cpp
@@ -221,8 +221,13 @@ static const ValueDecl *getAsSimpleValueDeclRef(const ASTContext &Ctx,
 
   // We model class non-type template parameters as their template parameter
   // object declaration.
-  if (V.isStruct() || V.isUnion())
+  if (V.isStruct() || V.isUnion()) {
+    // Dependent types are not supposed to be described as
+    // TemplateParamObjectDecls.
+    if (T->isDependentType() || T->isInstantiationDependentType())
+      return nullptr;
     return Ctx.getTemplateParamObjectDecl(T, V);
+  }
 
   // Pointers and references with an empty path use the special 'Declaration'
   // representation.
diff --git a/clang/test/SemaTemplate/temp_arg_nontype_cxx2c.cpp b/clang/test/SemaTemplate/temp_arg_nontype_cxx2c.cpp
index 9fb6b440b6b2a..e74c031eba4c1 100644
--- a/clang/test/SemaTemplate/temp_arg_nontype_cxx2c.cpp
+++ b/clang/test/SemaTemplate/temp_arg_nontype_cxx2c.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++20 -Wconversion -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++2c -Wconversion -verify %s
 
 struct Test {
     int a = 0;
@@ -102,3 +102,24 @@ void bar() {
 }
 
 }
+
+namespace GH84052 {
+
+template <class... T>
+concept C = sizeof(T...[1]) == 1; // #C
+
+struct A {};
+
+template <class T, C<T> auto = A{}> struct Set {}; // #Set
+
+template <class T> void foo() {
+  Set<T> unrelated;
+}
+
+Set<bool> sb;
+Set<float> sf;
+// expected-error@-1 {{constraints not satisfied for class template 'Set'}}
+// expected-note@#Set {{because 'C<decltype(GH84052::A{}), float>' evaluated to false}}
+// expected-note@#C {{evaluated to false}}
+
+} // namespace GH84052

[erichkeane]

This change seems smaller than I expected here, so I guess I'm not entirely sure what is going on. Can you better explain how this ends up using structural values instead of the TemplateParamObjectDecl?

[erichkeane]

What do we end up modeling them as instead?

[zyn0217]

The only caller of getAsSimpleValueDeclRef is TemplateArgument's constructor (for an NTTP establishment), where we basically try the following things in order, IIUC:

1. integral type
2. nullptrs
3. declarations (this is where we called getAsSimpleValueDeclRef and built up a TemplateParamObjectDecl if the function does return a value)

otherwise, a structural type.

llvm-project/clang/lib/AST/TemplateBase.cpp

Lines 237 to 249 in 74ed79f

	TemplateArgument::TemplateArgument(const ASTContext &Ctx, QualType Type,
	const APValue &V, bool IsDefaulted) {
	if (Type->isIntegralOrEnumerationType() && V.isInt())
	initFromIntegral(Ctx, V.getInt(), Type, IsDefaulted);
	else if ((V.isLValue() && V.isNullPointer()) ||
	(V.isMemberPointer() && !V.getMemberPointerDecl()))
	initFromType(Type, /*isNullPtr=*/true, IsDefaulted);
	else if (const ValueDecl *VD = getAsSimpleValueDeclRef(Ctx, Type, V))
	// FIXME: The Declaration form should expose a const ValueDecl*.
	initFromDeclaration(const_cast<ValueDecl *>(VD), Type, IsDefaulted);
	else
	initFromStructural(Ctx, Type, V, IsDefaulted);
	}

[erichkeane]

Ah! Thanks! That explains it.

[zyn0217]

Thanks for the review. I'm also waiting for @mizvekov in case he has some opinions.

[mizvekov]

LGTM as well, thanks!