[PATCH] Variable templates w/ partial support for static data members

[Larisse Voufo]

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3066-3068
@@ -3051,2 +3065,5 @@
   "class template or class template partial specialization">;
+def err_var_template_qualified_declarator_no_match : Error<
+  "nested name specifier '%0' for declaration does not refer into a variable, "
+  "variable template or variable template partial specialization">;
 def err_specialize_member_of_template : Error<
----------------
Richard Smith wrote:
> It looks like you don't use this diagnostic; remove.
Right. This must have been lost with previous revisions.

================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1902-1903
@@ +1901,4 @@
+
+DEF_TRAVERSE_DECL(VarTemplatePartialSpecializationDecl,
+                  {
+  // The partial specialization.
----------------
Richard Smith wrote:
> Join these lines.
Yeah. I need to be a bit more attentive with clang-format.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3110
@@ -3084,1 +3109,3 @@
+def err_var_spec_no_template : Error<
+  "no variable template matches %select{|partial}0 specialization">;
   
----------------
Richard Smith wrote:
> This has two spaces in a row if %0 is 0.
Fixed in one of the lasts patches after this...

================
Comment at: include/clang/Sema/Template.h:381-386
@@ -380,1 +380,8 @@
 
+    /// \brief A list of out-of-line variable template partial
+    /// specializations that will need to be instantiated after the
+    /// enclosing variable's instantiation is complete.
+    SmallVector<
+        std::pair<VarTemplateDecl *, VarTemplatePartialSpecializationDecl *>, 4>
+    OutOfLineVarPartialSpecs;
+
----------------
Richard Smith wrote:
> You don't appear to use these for anything yet; I assume support for class-scope partial specialization of variable templates will follow in a separate patch?
Probably. We have some class-scope partial specialization right now. However, it looks like I have not well mapped the behavior from nested templates. So, I guess I'll know for sure after that is done.

================
Comment at: lib/AST/ASTContext.cpp:1044-1049
@@ -1044,7 +1044,7 @@
 MemberSpecializationInfo *
 ASTContext::getInstantiatedFromStaticDataMember(const VarDecl *Var) {
   assert(Var->isStaticDataMember() && "Not a static data member");
-  llvm::DenseMap<const VarDecl *, MemberSpecializationInfo *>::iterator Pos
-    = InstantiatedFromStaticDataMember.find(Var);
-  if (Pos == InstantiatedFromStaticDataMember.end())
-    return 0;
+  return getTemplateOrSpecializationInfo(Var)
+      .dyn_cast<MemberSpecializationInfo *>();
+}
+
----------------
Richard Smith wrote:
> It looks like this has no callers; remove it.
I have replaced it with getTemplateOrSpecializationInfo(). However, I'd like to make sure that static data member templates are working well before I remove it completely...

================
Comment at: lib/AST/ASTContext.cpp:7995-7996
@@ +7994,4 @@
+         llvm::capacity_in_bytes(TemplateOrInstantiation)
+         //   + llvm::capacity_in_bytes(InstantiatedFromStaticDataMember)
+         +
+         llvm::capacity_in_bytes(InstantiatedFromUsingDecl) +
----------------
Richard Smith wrote:
> Why is this commented out?
I was in the process of replacing it with TemplateOrInstantiation. Removed.

================
Comment at: lib/AST/Decl.cpp:1987
@@ -1962,3 +1986,3 @@
   if (isStaticDataMember())
-    return getASTContext().getInstantiatedFromStaticDataMember(this);
-
+    // return getASTContext().getInstantiatedFromStaticDataMember(this);
+    return getASTContext().getTemplateOrSpecializationInfo(this)
----------------
Richard Smith wrote:
> Remove commented out code.
Same logic as earlier. I am not 100% confident that it is not needed yet. I will eventually removed it when/if I get static data member templates to work properly. I added a FIXME for now. 

================
Comment at: lib/Parse/ParseCXXInlineMethods.cpp:59-62
@@ +58,6 @@
+    NamedDecl *VD = FnD;
+    if (VarTemplateDecl *VT = VD ? dyn_cast<VarTemplateDecl>(VD) : 0)
+      // Re-direct this decl to refer to the templated decl so that we can
+      // initialize it.
+      VD = VT->getTemplatedDecl();
+
----------------
Richard Smith wrote:
> This can't happen: FnD is always a function declaration here. (We only handle initializers here in order to deal with a "= 0" pure-specifier.)
Ok.

================
Comment at: lib/Parse/ParseDecl.cpp:1805-1816
@@ -1805,12 +1804,14 @@
 
-  case ParsedTemplateInfo::ExplicitInstantiation: {
-    DeclResult ThisRes
-      = Actions.ActOnExplicitInstantiation(getCurScope(),
-                                           TemplateInfo.ExternLoc,
-                                           TemplateInfo.TemplateLoc,
-                                           D);
-    if (ThisRes.isInvalid()) {
+    // If this is a forward declaration of a variable template or variable
+    // template partial specialization with nested name specifier, complain.
+    CXXScopeSpec &SS = D.getCXXScopeSpec();
+    if (Tok.is(tok::semi) && ThisDecl && SS.isNotEmpty() &&
+        (isa<VarTemplateDecl>(ThisDecl) ||
+         isa<VarTemplatePartialSpecializationDecl>(ThisDecl))) {
+      Diag(SS.getBeginLoc(), diag::err_forward_var_nested_name_specifier)
+          << isa<VarTemplatePartialSpecializationDecl>(ThisDecl)
+          << SS.getRange();
       SkipUntil(tok::semi, true, true);
       return 0;
     }
 
----------------
Richard Smith wrote:
> This check seems like it belongs in Sema.
Ok.

================
Comment at: lib/Parse/ParseExprCXX.cpp:564-569
@@ -563,2 +563,8 @@
   CXXScopeSpec SS;
+  if (Tok.getKind() == tok::annot_template_id) {
+    TemplateIdAnnotation *TemplateId = takeTemplateIdAnnotation(Tok);
+    // FIXME: This is a hack for now. It may need to be done from within
+    // ParseUnqualifiedId();
+    SS = TemplateId->SS;
+  }
   ParseOptionalCXXScopeSpecifier(SS, ParsedType(), /*EnteringContext=*/false);
----------------
Richard Smith wrote:
> This should be handled elsewhere: perhaps in ParseOptionalCXXScopeSpecifier's handling of annot_template_id.
Ok. Noted.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:2313-2315
@@ +2312,5 @@
+
+  // If this is the variable for an anonymous struct or union,
+  // instantiate the anonymous struct/union type first.
+  if (const RecordType *RecordTy = D->getType()->getAs<RecordType>())
+    if (RecordTy->getDecl()->isAnonymousStructOrUnion())
----------------
Richard Smith wrote:
> Is it really valid for a type to be defined in a variable template declaration? That seems like a big can of worms.
I'm not sure. But perhaps this is another question to add onto my "notes".

I interpret this as allowing things like 
  template<typename T> class { } var = ...;
since "class { }" is anonymous and this is okay:
  class {} var;

But that can lead to things like 
  template<typename T> class {T foo;} var = ...;

Which can be "tricky" to manage...

Should we just reject all such cases, or at least just allow the first one, where the anonymous type is non-dependent?






http://llvm-reviews.chandlerc.com/D1067