[PATCH] D147722: [Concepts] Fix Function Template Concepts comparisons

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:322
+      // they mean.
+      R = Response::UseNextDecl(PartialClassTemplSpec);
     } else if (const auto *ClassTemplSpec =
----------------
erichkeane wrote:
> rsmith wrote:
> > Can we produce a "done" response instead here? It doesn't seem right to carry on to the next level here without adding a level of template arguments -- if any further arguments were somehow collected, they would be collected at  the wrong template depth. But, as far as I can determine, we should never encounter a partial specialization declaration except in cases where there is nothing remaining to be substituted (all outer substitutions would be identity substitutions). I think the same thing applies when we reach a class template declaration (the first branch in `HandleRecordDecl`) -- we should just be able to stop in that case too, rather than collecting some identity template arguments and carrying on to an outer level.
> > 
> > If we can't produce a "done" response here for some reason, then we should collect a level of identity template arguments instead.
> For constraints, the arguments are relative to namespace scope as far as I can tell.  If we ended up 'stopping' we would miss references to the outer template, so something like:
> 
> ``` 
> template<typename T>
> struct Outer {
> 
>   template<typename U>
>   struct Inner {
>      void foo() requires (is_same<T, U>){}
>   };
> 
> };
> 
> ```
> 
> Right?  The substitution fo 'T' would be invalid at that point?
> 
> Also, what do you mean by 'identity' template arguments?  I'm not sure I follow.
Er, I meant something like (since it needs to be a partial spec): 
```
  1 template<typename T>
  2 concept C = true;
  3
  4 template<typename T>
  5 struct Outer {
  6   template<typename U, typename V>
  7   struct Inner {};
  8   template<typename U>
  9   struct Inner<U, U> {
 10     void foo()  requires C<U> && C<T>{}
 11     void foo()  requires true{}
 12   };
 13 };
```


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

https://reviews.llvm.org/D147722