[Concepts] Dump template arguments for immediately declared constraint.

[hokein]

Summary

The template arguments were dumped as part of the TemplateTypeParmDecl, which
was incorrect.

[nridge]

This patch has helped me understand the difference between the two ConceptReferences in the AST (the TypeConstraint and the ConceptSpecializationExpr).

Given that

template <binary_concept<int> R>

is a shorthand for

template <typename R> requires binary_concept<R, int>

the TypeConstraint is the explicitly written binary_concept<int> whose getTemplateArgsAsWritten() has one element: <int>.

Meanwhile the ConceptSpecializationExpr is the implicit binary_concept<R, int> whose getTemplateArgsAsWritten() has two elements: <R, int>.

Is there a reason to prefer dumping one over the other (or even both)?

[nridge]

clang/test/AST/ast-dump-concepts.cpp

18

The ending column number here was important (it tested the fix to ConceptSpecializationExpr::getEndLoc() in D86413).

[hokein]

address review comment.

[hokein]

In D85282#2198737, @nridge wrote:

This patch has helped me understand the difference between the two ConceptReferences in the AST (the TypeConstraint and the ConceptSpecializationExpr).

Given that

template <binary_concept<int> R>

is a shorthand for

template <typename R> requires binary_concept<R, int>

the TypeConstraint is the explicitly written binary_concept<int> whose getTemplateArgsAsWritten() has one element: <int>.

Meanwhile the ConceptSpecializationExpr is the implicit binary_concept<R, int> whose getTemplateArgsAsWritten() has two elements: <R, int>.

Yes, true.

Is there a reason to prefer dumping one over the other (or even both)?

This is a good question. Currently we don't dump TypeConstrain instead we dump the responding immediately declared constraint (which is a ConceptSpecializationExpr).

I think it is ok to just dump the immediately declared constraint as TypeConstrain is just a wrapper of it.

template <binary_concept<int> R>

before this patch:
TemplateTypeParmDecl R
  |- ConceptSpecializationExpr
  `- TemplateArgument {{.*}} type 'int'

after this patch:
TemplateTypeParmDecl R
  |- ConceptSpecializationExpr
     |- TemplateArgument {{.*}} type 'R'
     `- TemplateArgument {{.*}} type 'int'

maybe the following is better:
TemplateTypeParmDecl R
  |- TypeContraint
     `- ConceptSpecializationExpr
         |- TemplateArgument {{.*}} type 'R'
         `- TemplateArgument {{.*}} type 'int'

The behavior (before this patch) is to dump the written template arguments in TypeConstraint as part of the TemplateTypeParmDecl, which doesn't seem to reasonable.
It also confuses the default template argument case.

template<typename T = int>

// is dump as 
TemplateTypeParmDecl  T
   `-TemplateArgument type 'int'

clang/test/AST/ast-dump-concepts.cpp

18

oh, I lost it during the rebase, added it back.

[nridge]

Ok, sounds reasonable to me. Thanks!

clang/include/clang/AST/ASTNodeTraverser.h

public:
   void VisitVarTemplatePartialSpecializationDecl(
       const VarTemplatePartialSpecializationDecl *D) {
     dumpTemplateParameters(D->getTemplateParameters());
     VisitVarTemplateSpecializationDecl(D);
   }
 
   void VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
     if (const auto *TC = D->getTypeConstraint())
-      if (TC->hasExplicitTemplateArgs())
-        for (const auto &ArgLoc : TC->getTemplateArgsAsWritten()->arguments())
-          dumpTemplateArgumentLoc(ArgLoc);
+      Visit(TC->getImmediatelyDeclaredConstraint());
     if (D->hasDefaultArgument())
       Visit(D->getDefaultArgument(), SourceRange(),
             D->getDefaultArgStorage().getInheritedFrom(),
             D->defaultArgumentWasInherited() ? "inherited from" : "previous");
   }
 
   void VisitNonTypeTemplateParmDecl(const NonTypeTemplateParmDecl *D) {
     if (const auto *E = D->getPlaceholderTypeConstraint())
if (D->hasDefaultArgument())
           D->defaultArgumentWasInherited() ? "inherited from" : "previous");
   }
 
   void VisitConceptDecl(const ConceptDecl *D) {
     dumpTemplateParameters(D->getTemplateParameters());
     Visit(D->getConstraintExpr());
   }
 
+  void VisitConceptSpecializationExpr(const ConceptSpecializationExpr *CSE) {
+    if (CSE->hasExplicitTemplateArgs())
+      for (const auto &ArgLoc : CSE->getTemplateArgsAsWritten()->arguments())
+        dumpTemplateArgumentLoc(ArgLoc);
+  }
+
   void VisitUsingShadowDecl(const UsingShadowDecl *D) {
     if (auto *TD = dyn_cast<TypeDecl>(D->getUnderlyingDecl()))
       Visit(TD->getTypeForDecl());
   }
 
   void VisitFriendDecl(const FriendDecl *D) {
     if (!D->getFriendType())
       Visit(D->getFriendDecl());

clang/lib/AST/TextNodeDumper.cpp

void TextNodeDumper::VisitTemplateTypeParmDecl(const TemplateTypeParmDecl *D) {
   if (const auto *TC = D->getTypeConstraint()) {
     OS << " ";
     dumpBareDeclRef(TC->getNamedConcept());
     if (TC->getNamedConcept() != TC->getFoundDecl()) {
       OS << " (";
       dumpBareDeclRef(TC->getFoundDecl());
       OS << ")";
     }
-    AddChild([=] { Visit(TC->getImmediatelyDeclaredConstraint()); });
   } else if (D->wasDeclaredWithTypename())
     OS << " typename";
   else
     OS << " class";
   OS << " depth " << D->getDepth() << " index " << D->getIndex();
   if (D->isParameterPack())
     OS << " ...";
   dumpName(D);

clang/test/AST/ast-dump-concepts.cpp

 concept unary_concept = true;
 
 template <typename T, typename U>
 concept binary_concept = true;
 
 template <typename T>
 struct Foo {
   // CHECK:      TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}} <col:13, col:31> 'bool' Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT: `-ConceptSpecializationExpr {{.*}} <col:13, col:31> 'bool' Concept {{.*}} 'binary_concept'
+  // CHECK-NEXT:   |-TemplateArgument {{.*}} type 'R'
+  // CHECK-NEXT:   | `-TemplateTypeParmType {{.*}} 'R'
+  // CHECK-NEXT:   |   `-TemplateTypeParm {{.*}} 'R'
+  // CHECK-NEXT:   `-TemplateArgument {{.*}} type 'int'
+  // CHECK-NEXT:     `-BuiltinType {{.*}} 'int'
   template <binary_concept<int> R>
   Foo(R);
 
   // CHECK:      TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 'unary_concept'
   // CHECK-NEXT: `-ConceptSpecializationExpr {{.*}} <col:13> 'bool'
   template <unary_concept R>
   Foo(R);
 
-  // CHECK:      FunctionTemplateDecl {{.*}} <line:29:3, line:30:39> {{.*}} Foo<T>
+  // CHECK:      FunctionTemplateDecl {{.*}} <line:[[@LINE+1]]:3, line:[[@LINE+2]]:39> {{.*}} Foo<T>
   template <typename R>
   Foo(R, int) requires unary_concept<R>;
 
-  // CHECK:      FunctionTemplateDecl {{.*}} <line:33:3, line:35:3> {{.*}} Foo<T>
+  // CHECK:      FunctionTemplateDecl {{.*}} <line:[[@LINE+1]]:3, line:[[@LINE+3]]:3> {{.*}} Foo<T>
   template <typename R>
   Foo(R, char) requires unary_concept<R> {
   }
 };