[PATCH] D18495: Check default arguments of template template parameters for compatibility.

Faisal Vali via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 27 09:14:08 PDT 2016


faisalv marked 2 inline comments as done.

================
Comment at: lib/Sema/SemaTemplate.cpp:793-800
@@ -792,10 +792,2 @@
   if (!Default.isInvalid()) {
-    // Check only that we have a template template argument. We don't want to
-    // try to check well-formedness now, because our template template parameter
-    // might have dependent types in its template parameters, which we wouldn't
-    // be able to match now.
-    //
-    // If none of the template template parameter's template arguments mention
-    // other template parameters, we could actually perform more checking here.
-    // However, it isn't worth doing.
     TemplateArgumentLoc DefaultArg = translateTemplateArgument(*this, Default);
----------------
rsmith wrote:
> You don't appear to have added any test coverage for these cases. Are there existing tests for this? Something like:
> 
>   template<typename T, template<T> typename A>
>   struct X {
>     template<template<int> typename = A> struct Y {};
>   };
> 
> Similar cases exist where the template template parameter has a parameter pack and is used as a default template argument for a template template parameter that takes a non-pack.
Thanks for the example - I was struggling with conjuring an example that would represent the concerns expressed in those comments.  Example added to test suite.


================
Comment at: test/CXX/temp/temp.arg/temp.arg.template/p3-0x.cpp:73-74
@@ +72,4 @@
+struct A {
+  template<class U, template<U, class> //expected-note{{declared here}}
+      class UU = TT> //expected-error{{different template parameters}}
+  struct B;
----------------
rsmith wrote:
> This looks like a rejects-valid. Consider:
> 
>   template<int, class> struct Q;
>   A<Q>::B<int> b;
> 
> Here, `TT` refers to `Q`, which is valid as a template template argument for parameter `UU`.
Aah yes - nice example - I agree this should work.  Please see revision patch.


http://reviews.llvm.org/D18495





More information about the cfe-commits mailing list