[PATCH] D85282: [Concepts] Dump template arguments for immediately declared constraint.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D85282#2198737 <https://reviews.llvm.org/D85282#2198737>, @nridge wrote:

> This patch has helped me understand the difference between the two `ConceptReference`s 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'





================
Comment at: clang/test/AST/ast-dump-concepts.cpp:18
   // CHECK:      TemplateTypeParmDecl {{.*}} referenced Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: |-ConceptSpecializationExpr {{.*}} <col:13, col:31> 'bool' Concept {{.*}} 'binary_concept'
-  // CHECK-NEXT: `-TemplateArgument {{.*}} type 'int'
----------------
nridge wrote:
> The ending column number here was important (it tested the fix to `ConceptSpecializationExpr::getEndLoc()` in D86413).
oh, I lost it during the rebase, added it back.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85282/new/

https://reviews.llvm.org/D85282