[PATCH] Top-level variable declaration (all but static data members)

Richard Smith richard at metafoo.co.uk
Tue Jul 9 18:26:40 PDT 2013


  Wow, big patch, but this is looking really good. Major themes:

   * You issue an ExtWarn for variable templates in C++98 and C++11, but don't actually accept them as an extension due to bailing out in one place and due to asserts elsewhere. Please remove the asserts and handle them regardless of which version of C++ we're targeting.
   * A few formatting issues.
   * Still would benefit from more test cases. Specifically:
    - Tests for PCH (serialization/deserialization)
    - Tests for name mangling of variable templates, and IR generation generally (for instance, implicit internal linkage of variable template that instantiates to a `const`-qualified type).
    - Tests for variable template with `auto` type and variable template with incomplete array type (cases where the type is influenced by the initializer)
    - Tests for variable templates actually working when not in C++1y mode
    - Plus tests exercising each diagnostic you tweaked or added for variable templates.


================
Comment at: include/clang/AST/ASTContext.h:314
@@ +313,3 @@
+  /// FIXME: With support for static data member templates, unify with
+  ///        InstantiatedFRomStaticDataMember?
+public: 
----------------
*From

================
Comment at: include/clang/AST/Decl.h:53
@@ -51,3 +52,3 @@
 /// @code
-/// TypeLoc TL = TypeSourceInfo->getTypeLoc();
+/// TypeLoc TL = TypeSourceInfo->TypeLoc();
 /// if (PointerLoc *PL = dyn_cast<PointerLoc>(&TL))
----------------
Accidental change?

================
Comment at: include/clang/AST/Decl.h:773
@@ -776,3 +772,3 @@
                          IdentifierInfo *Id, QualType T, TypeSourceInfo *TInfo,
-                         StorageClass S);
+                         StorageClass S, VarDecl *PrevDecl=0);
 
----------------
Spaces around = please.

================
Comment at: include/clang/AST/Decl.h:951
@@ -954,2 +950,3 @@
   bool isFileVarDecl() const {
-    if (getKind() != Decl::Var)
+    Kind k = getKind();
+    if ((k == ParmVar) || (k == ImplicitParam))
----------------
Capital 'K'.

================
Comment at: include/clang/AST/Decl.h:1205
@@ -1188,1 +1204,3 @@
+    : VarDecl(DK, DC, StartLoc, IdLoc, Id, T, TInfo, S, 0) {
+    // Function parameters cannot be variable templates. Hence, PrevDecl=0.
     assert(ParmVarDeclBits.HasInheritedDefaultArg == false);
----------------
More to the point, function parameters cannot be redeclarations. There's no fundamental reason to only use PrevDecl for variable templates.

================
Comment at: include/clang/AST/DeclTemplate.h:2260
@@ +2259,3 @@
+/// Variable template specializations represent both explicit
+/// specialization of variable templates, as in the example below, and
+/// implicit instantiations of variable templates.
----------------
*specializations

================
Comment at: include/clang/AST/DeclTemplate.h:2285
@@ +2284,3 @@
+
+  /// \brief The template that this specialization specializes
+  llvm::PointerUnion<VarTemplateDecl *, SpecializedPartialSpecialization *>
----------------
Missing period.

================
Comment at: include/clang/AST/DeclTemplate.h:2310
@@ +2309,3 @@
+
+  /// \brief The point where this template was instantiated (if any)
+  SourceLocation PointOfInstantiation;
----------------
Missing period.

================
Comment at: include/clang/AST/DeclTemplate.h:2318-2326
@@ +2317,11 @@
+protected:
+  VarTemplateSpecializationDecl(ASTContext &Context, Kind DK,
+                                  DeclContext *DC, SourceLocation StartLoc,
+                                  SourceLocation IdLoc,
+                                  VarTemplateDecl *SpecializedTemplate,
+                                  QualType T, TypeSourceInfo *TInfo,
+                                  StorageClass S,
+                                  const TemplateArgument *Args,
+                                  unsigned NumArgs,
+                                  VarTemplateSpecializationDecl *PrevDecl);
+
----------------
Indentation.

================
Comment at: include/clang/AST/Decl.h:1150
@@ +1149,3 @@
+  void setInstantiationOfStaticDataMember(VarDecl *VD,
+                                     TemplateSpecializationKind TSK);
+
----------------
Indentation.

================
Comment at: include/clang/AST/DeclTemplate.h:2443
@@ +2442,3 @@
+  /// \brief Retrieve the set of template arguments that should be used
+  /// to instantiate members of the variable template or variable template partial
+  /// specialization from which this variable template specialization was
----------------
"members of the variable template"? Maybe "the initializer of the variable template"?

================
Comment at: include/clang/AST/DeclTemplate.h:2484
@@ +2483,3 @@
+  /// \brief Sets the type of this specialization as it was written by
+  /// the user. This will be a variable template specialization type.
+  void setTypeAsWritten(TypeSourceInfo *T) {
----------------
Last sentence here seems incorrect; drop it?

================
Comment at: include/clang/AST/DeclTemplate.h:2545
@@ +2544,3 @@
+  /// \brief The list of template parameters
+  TemplateParameterList* TemplateParams;
+
----------------
"*" on the right, please.

================
Comment at: include/clang/AST/DeclTemplate.h:2552-2555
@@ +2551,6 @@
+
+  /// \brief Sequence number indicating when this variable template partial
+  /// specialization was added to the set of partial specializations for
+  /// its owning variable template.
+  unsigned SequenceNumber;
+
----------------
Is this necessary, given that we store specializations in a FoldingSetVector (which maintains order itself)?

================
Comment at: include/clang/AST/DeclTemplate.h:2565-2578
@@ +2564,16 @@
+
+  VarTemplatePartialSpecializationDecl(ASTContext &Context,
+                                         DeclContext *DC,
+                                         SourceLocation StartLoc,
+                                         SourceLocation IdLoc,
+                                         TemplateParameterList *Params,
+                                         VarTemplateDecl *SpecializedTemplate,
+                                         QualType T, TypeSourceInfo *TInfo,
+                                         StorageClass S,
+                                         const TemplateArgument *Args,
+                                         unsigned NumArgs,
+                                         TemplateArgumentLoc *ArgInfos,
+                                         unsigned NumArgInfos,
+                               VarTemplatePartialSpecializationDecl *PrevDecl,
+                                         unsigned SequenceNumber);
+
----------------
Indentation.

================
Comment at: include/clang/AST/DeclTemplate.h:2596
@@ +2595,3 @@
+         unsigned NumArgs,
+	 const TemplateArgumentListInfo &ArgInfos,
+         VarTemplatePartialSpecializationDecl *PrevDecl,
----------------
Indentation.

================
Comment at: include/clang/AST/DeclTemplate.h:2660-2661
@@ +2659,4 @@
+
+  /// \brief Determines whether this variable template partial specialization
+  /// template was a specialization of a member partial specialization.
+  ///
----------------
Extra "template" at the start of the second line.

================
Comment at: include/clang/AST/DeclTemplate.h:2691
@@ +2690,3 @@
+
+  // FIXME: Add Profile support!
+
----------------
Isn't this handled by the Profile support in the base class?

================
Comment at: include/clang/AST/DeclTemplate.h:2712-2713
@@ +2711,4 @@
+
+    /// \brief The variable template specializations for this class
+    /// template, including explicit specializations and instantiations.
+    llvm::FoldingSetVector<VarTemplateSpecializationDecl> Specializations;
----------------
... for this *variable* template

================
Comment at: include/clang/AST/DeclTemplate.h:2716-2717
@@ +2715,4 @@
+
+    /// \brief The variable template partial specializations for this class
+    /// template.
+    llvm::FoldingSetVector<VarTemplatePartialSpecializationDecl>
----------------
... for this *variable* template

================
Comment at: include/clang/AST/DeclTemplate.h:2742
@@ +2741,3 @@
+  VarTemplateDecl(DeclContext *DC, SourceLocation L, DeclarationName Name,
+                    TemplateParameterList *Params, NamedDecl *Decl)
+    : RedeclarableTemplateDecl(VarTemplate, DC, L, Name, Params, Decl) { }
----------------
Indentation.

================
Comment at: include/clang/AST/DeclTemplate.h:2756
@@ +2755,3 @@
+public:
+  /// \brief Get the underlying class declarations of the template.
+  VarDecl *getTemplatedDecl() const {
----------------
... underlying variable declaration

================
Comment at: include/clang/AST/DeclTemplate.h:2762
@@ +2761,3 @@
+  /// \brief Returns whether this template declaration defines the primary
+  /// class pattern.
+  bool isThisDeclarationADefinition() const {
----------------
... primary variable template pattern.

================
Comment at: include/clang/AST/DeclTemplate.h:2769-2773
@@ +2768,7 @@
+  static VarTemplateDecl *Create(ASTContext &C, DeclContext *DC,
+                                   SourceLocation L,
+                                   DeclarationName Name,
+                                   TemplateParameterList *Params,
+                                   NamedDecl *Decl,
+                                   VarTemplateDecl *PrevDecl);
+
----------------
Indentation.

================
Comment at: include/clang/AST/DeclTemplate.h:2836-2844
@@ +2835,11 @@
+
+  /// \brief Find a variable template partial specialization with the given
+  /// type T.
+  ///
+  /// \param T a dependent type that names a specialization of this class
+  /// template.
+  ///
+  /// \returns the variable template partial specialization that exactly matches
+  /// the type \p T, or NULL if no such partial specialization exists.
+  //VarTemplatePartialSpecializationDecl *findPartialSpecialization(QualType T);
+
----------------
This doesn't make sense for a variable template; just remove it.

================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1484-1532
@@ -1482,2 +1483,51 @@
 
+// A helper method for traversing the implicit instantiations of a
+// variable template.
+template<typename Derived>
+bool RecursiveASTVisitor<Derived>::TraverseVariableInstantiations(
+    VarTemplateDecl *D) {
+  VarTemplateDecl::spec_iterator end = D->spec_end();
+  for (VarTemplateDecl::spec_iterator it = D->spec_begin(); it != end; ++it) {
+    VarTemplateSpecializationDecl* SD = *it;
+
+    switch (SD->getSpecializationKind()) {
+    // Visit the implicit instantiations with the requested pattern.
+    case TSK_Undeclared:
+    case TSK_ImplicitInstantiation:
+      TRY_TO(TraverseDecl(SD));
+      break;
+
+    // We don't need to do anything on an explicit instantiation
+    // or explicit specialization because there will be an explicit
+    // node for it elsewhere.
+    case TSK_ExplicitInstantiationDeclaration:
+    case TSK_ExplicitInstantiationDefinition:
+    case TSK_ExplicitSpecialization:
+      break;
+    }
+  }
+
+  return true;
+}
+
+DEF_TRAVERSE_DECL(VarTemplateDecl, {
+    VarDecl* TempDecl = D->getTemplatedDecl();
+    TRY_TO(TraverseDecl(TempDecl));
+    TRY_TO(TraverseTemplateParameterListHelper(D->getTemplateParameters()));
+
+    // By default, we do not traverse the instantiations of
+    // class templates since they do not appear in the user code. The
+    // following code optionally traverses them.
+    //
+    // We only traverse the class instantiations when we see the canonical
+    // declaration of the template, to ensure we only visit them once.
+    if (getDerived().shouldVisitTemplateInstantiations() &&
+        D == D->getCanonicalDecl())
+      TRY_TO(TraverseVariableInstantiations(D));
+
+    // Note that getInstantiatedFromMemberTemplate() is just a link
+    // from a template instantiation back to the template from which
+    // it was instantiated, and thus should not be traversed.
+  })
+
 // A helper method for traversing the instantiations of a
----------------
Once this patch lands, it'd be good to go back over this and clean up this duplication: traversal for variable templates, class templates, and function templates all use exactly the same code (other than some trivial differences in class names).

================
Comment at: include/clang/AST/RecursiveASTVisitor.h:1886
@@ +1885,3 @@
+    }
+    // The args that remains unspecialized.
+    TRY_TO(TraverseTemplateArgumentLocsHelper(
----------------
Grammar.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3113
@@ +3112,3 @@
+def err_var_default_arg_in_partial_spec : Error<
+    "default template argument in a variable template partial specialization">;
+def err_var_partial_spec_redeclared : Error<
----------------
Indentation.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2843
@@ -2840,3 +2842,3 @@
 
-def err_template_variable : Error<"variable %0 declared as a template">;
+def ext_template_variable : ExtWarn<"variable %0 declared as a template">, InGroup<CXX1y>;
 def err_template_variable_noparams : Error<
----------------
We usually word this as "variable templates are a C++1y extension". Please also add a CXXPre1yCompat warning, "variable templates are incompatible with C++ standards before C++1y". (The idea is to allow people to compile with -std=c++1y, and get warned if their code would not be portable to earlier standard versions.)

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3114-3115
@@ +3113,4 @@
+    "default template argument in a variable template partial specialization">;
+def err_var_partial_spec_redeclared : Error<
+  "variable template partial specialization %0 cannot be redeclared">;
+def note_var_prev_partial_spec_here : Note<
----------------
Should this say "... cannot be redefined" ? The corresponding rule for class template partial specializations in 14.5.5/5 allows redeclarations.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3119
@@ -3099,1 +3118,3 @@
+def err_var_spec_no_template : Error<
+    "no variable template matches %select{|partial}0 specialization">;
   
----------------
Indentation.

================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:3274-3276
@@ -3250,2 +3273,5 @@
   "explicit instantiation definition is here">;
+def err_invalid_var_template_spec_type : Error<"type %2 "
+  "of %select{explicit instantiation|explicit specialization|"
+  "partial specialization|redeclaration}0 does not match expected type %3">;
   
----------------
Where's %1?

================
Comment at: include/clang/Sema/Sema.h:1386
@@ -1378,1 +1385,3 @@
+      assert(Kind == NC_TypeTemplate || Kind == NC_FunctionTemplate
+              || Kind == NC_VarTemplate);
       return Template;
----------------
"||" on previous line, please.

================
Comment at: include/clang/Sema/Sema.h:1391-1392
@@ -1381,3 +1390,4 @@
     TemplateNameKind getTemplateNameKind() const {
-      assert(Kind == NC_TypeTemplate || Kind == NC_FunctionTemplate);
-      return Kind == NC_TypeTemplate? TNK_Type_template : TNK_Function_template;
+      assert(Kind == NC_TypeTemplate || Kind == NC_FunctionTemplate 
+              || Kind == NC_VarTemplate);
+      switch (Kind) {
----------------
Remove this assert; it's covered by your default case below.

================
Comment at: include/clang/Sema/Sema.h:5033
@@ +5032,3 @@
+                                         TemplateParameterList *TemplateParams,
+                                            bool isPartialSpecialization);
+
----------------
Capital I.

================
Comment at: include/clang/Sema/Sema.h:6373-6374
@@ -6316,1 +6372,4 @@
                                      bool DefinitionRequired = false);
+  void InstantiateVariableInitializer(VarDecl* Var, 
+                                      VarDecl* OldVar,
+                      const MultiLevelTemplateArgumentList &TemplateArgs);
----------------
"*" on the right, please

================
Comment at: include/clang/Sema/Template.h:385
@@ +384,3 @@
+    SmallVector<std::pair<VarTemplateDecl *,
+                                VarTemplatePartialSpecializationDecl *>, 4>
+      OutOfLineVarPartialSpecs;
----------------
Indentation

================
Comment at: include/clang/Sema/Template.h:451
@@ +450,3 @@
+      SmallVectorImpl<std::pair<VarTemplateDecl *,
+                                     VarTemplatePartialSpecializationDecl *> >
+        ::iterator
----------------
Indentation (and probably unnecessary line wrapping)

================
Comment at: include/clang/Sema/Template.h:499-501
@@ -477,1 +498,5 @@
+    VarTemplatePartialSpecializationDecl *
+    InstantiateVarTemplatePartialSpecialization(
+                                              VarTemplateDecl *VarTemplate,
+                           VarTemplatePartialSpecializationDecl *PartialSpec);
     void InstantiateEnumDefinition(EnumDecl *Enum, EnumDecl *Pattern);
----------------
Reformat this, please

================
Comment at: lib/AST/ASTContext.cpp:1062
@@ -1051,3 +1061,3 @@
                                                 TemplateSpecializationKind TSK,
-                                          SourceLocation PointOfInstantiation) {
+						SourceLocation PointOfInstantiation) {
   assert(Inst->isStaticDataMember() && "Not a static data member");
----------------
Accidental change?

================
Comment at: lib/AST/ASTImporter.cpp:2038
@@ +2037,3 @@
+
+  // FIXME: Other bits to merge?
+  
----------------
I think this FIXME fits better in the caller.

================
Comment at: lib/AST/ASTImporter.cpp:4327
@@ +4326,3 @@
+        // unit only had a forward declaration anyway; call it the same
+        // function.
+        return Importer.Imported(D, FoundDef);
----------------
function?

================
Comment at: lib/AST/ASTImporter.cpp:4318
@@ +4317,3 @@
+  if (D2) {
+    // We already have a class template specialization with these template
+    // arguments.
----------------
class?

================
Comment at: lib/AST/ASTImporter.cpp:4236-4237
@@ +4235,4 @@
+  // getInit() here.
+  D2Templated->setInit(Importer.Import(
+      const_cast<Expr *>(DTemplated->getAnyInitializer())));
+
----------------
Did you intend to reuse ImportDefinition here?

================
Comment at: lib/AST/ASTImporter.cpp:4175
@@ +4174,3 @@
+  // We may already have a template of the same name; try to find and match it.
+  if (!DC->isFunctionOrMethod()) {
+    SmallVector<NamedDecl *, 4> ConflictingDecls;
----------------
We don't support variable templates declared at function scope, right?

================
Comment at: lib/AST/ASTImporter.cpp:106
@@ -106,2 +106,2 @@
 
     bool ImportDefinition(RecordDecl *From, RecordDecl *To, 
----------------
Updates to ASTImporter.cpp aren't generally necessary: it's already *very* incomplete (it doesn't support function templates, for instance) and would require substantial work to make it complete.

================
Comment at: lib/AST/Decl.cpp:1563
@@ +1562,3 @@
+        QualType T, TypeSourceInfo *TInfo, StorageClass SC,
+        VarDecl *PrevDecl)
+  : DeclaratorDecl(DK, DC, IdLoc, Id, T, TInfo, StartLoc), Init() {
----------------
You're not using PrevDecl here; remove it?

================
Comment at: lib/AST/Decl.cpp:2000-2002
@@ +1999,5 @@
+    if (TSK != TSK_ExplicitSpecialization &&
+          PointOfInstantiation.isValid() &&
+          Spec->getPointOfInstantiation().isInvalid())
+        Spec->setPointOfInstantiation(PointOfInstantiation);
+    return; 
----------------
Indentation

================
Comment at: lib/AST/Decl.cpp:2009-2011
@@ +2008,5 @@
+    if (TSK != TSK_ExplicitSpecialization &&
+          PointOfInstantiation.isValid() &&
+          MSI->getPointOfInstantiation().isInvalid())
+        MSI->setPointOfInstantiation(PointOfInstantiation);
+    return;
----------------
Indentation.

================
Comment at: lib/AST/DeclTemplate.cpp:971-976
@@ +970,8 @@
+VarTemplateDecl *VarTemplateDecl::Create(ASTContext &C,
+                                             DeclContext *DC,
+                                             SourceLocation L,
+                                             DeclarationName Name,
+                                             TemplateParameterList *Params,
+                                             NamedDecl *Decl,
+                                             VarTemplateDecl *PrevDecl) {
+  VarTemplateDecl *New = new (C) VarTemplateDecl(DC, L, Name, Params, Decl);
----------------
Indentation.

================
Comment at: lib/AST/DeclTemplate.cpp:983
@@ +982,3 @@
+VarTemplateDecl *VarTemplateDecl::CreateDeserialized(ASTContext &C, 
+                                                         unsigned ID) {
+  void *Mem = AllocateDeserializedDecl(C, ID, sizeof(VarTemplateDecl));
----------------
Indentation (and likewise later functions in this file).

================
Comment at: lib/AST/DeclTemplate.cpp:1208-1209
@@ +1207,4 @@
+{ 
+  // FIXME: Is this needed for variable templates?
+  //AdoptTemplateParameterList(Params, this);
+}
----------------
Maybe adopt them into DC (though they should presumably be there already).

================
Comment at: lib/Parse/Parser.cpp:1431
@@ -1429,3 +1430,3 @@
   case Sema::NC_FunctionTemplate: {
     // We have a type or function template followed by '<'.
     ConsumeToken();
----------------
Update this comment.

================
Comment at: lib/Parse/Parser.cpp:1646-1647
@@ -1644,2 +1645,4 @@
       return false;
     }
+    else if (TemplateId->Kind == TNK_Var_template)
+      return false;
----------------
No linebreak here.

================
Comment at: lib/Sema/SemaDecl.cpp:4914-4918
@@ +4913,7 @@
+          // There is no such thing as a variable template.
+          Diag(D.getIdentifierLoc(), diag::ext_template_variable)
+            << II
+            << SourceRange(TemplateParams->getTemplateLoc(),
+                           TemplateParams->getRAngleLoc());
+          return 0;
+        } else {
----------------
Don't bail out here, just accept the variable template as if we were in C++1y mode.

================
Comment at: lib/Sema/SemaDecl.cpp:4939
@@ +4938,3 @@
+
+        } else if (TemplateParams->size() > 0) {
+          // C++1y supports variable templates (N3651).
----------------
What happens in the "else" case of this? (template<> int n;)

================
Comment at: lib/Sema/SemaDecl.cpp:4931
@@ +4930,3 @@
+        if (Previous.begin() != Previous.end())
+            PrevDecl = (*Previous.begin())->getUnderlyingDecl();
+        PrevVarTemplate = dyn_cast_or_null<VarTemplateDecl>(PrevDecl);
----------------
Indentation.

================
Comment at: lib/Sema/SemaDecl.cpp:4980
@@ +4979,3 @@
+                PrevVarTemplate? PrevVarTemplate->getTemplateParameters() : 0,
+                                         ( D.getCXXScopeSpec().isSet() && DC &&
+                                          DC->isRecord() &&
----------------
Extra space after paren.

================
Comment at: lib/Sema/SemaDecl.cpp:4987-4990
@@ +4986,6 @@
+
+          if ( D.getCXXScopeSpec().isSet()) {
+            // If the name of the template was qualified, we must be defining the
+            // template out-of-line.
+            if (! D.getCXXScopeSpec().isInvalid() && !Invalid && !PrevVarTemplate) {
+              Diag(D.getIdentifierLoc(), diag::err_member_def_does_not_match)
----------------
Extra spaces after ( here.

================
Comment at: lib/Sema/SemaDecl.cpp:5017-5018
@@ +5016,4 @@
+      AddToScope = false;
+    }
+    else
+      NewVD = VarDecl::Create(Context, DC, D.getLocStart(),
----------------
No linebreak here.

================
Comment at: lib/Sema/SemaDecl.cpp:5033
@@ -4933,2 +5032,3 @@
 
-    if (TemplateParamLists.size() > 0 && D.getCXXScopeSpec().isSet()) {
+    // FIXME: Do we need D.getCXXScopeSpec().isSet()?
+    if (TemplateParams && TemplateParamLists.size() > 1 &&
----------------
Doesn't look like it; it looks incorrect for variable templates and redundant otherwise.

================
Comment at: lib/Sema/SemaDecl.cpp:4833-4835
@@ -4827,2 +4832,5 @@
 
   bool isExplicitSpecialization = false;
+  bool isVariableTemplateSpecialization = false;
+  bool isPartialSpecialization = false;
+  bool Invalid = false;
----------------
"Is", not "is", please.

================
Comment at: lib/Sema/SemaDecl.cpp:5231-5232
@@ -5108,1 +5230,4 @@
 
+  if (getLangOpts().CPlusPlus1y && TemplateParams &&
+      !isVariableTemplateSpecialization) {
+    // C++1y supports variable templates (N3651)
----------------
Should do this regardless of language mode.

================
Comment at: lib/Sema/SemaDecl.cpp:5249
@@ +5248,3 @@
+        PrevVarTemplate->getInstantiatedFromMemberTemplate())
+      PrevVarTemplate->setMemberSpecialization();
+
----------------
It might be clearer to set this on NewTemplate (even though they share a CommonPtr).

================
Comment at: lib/Sema/SemaDecl.cpp:4836
@@ +4835,3 @@
+  bool isPartialSpecialization = false;
+  bool Invalid = false;
+  TemplateParameterList *TemplateParams = 0;
----------------
It seems error-prone to have this in scope for the entire function but only use it when declaring a variable template.

================
Comment at: lib/Sema/SemaExpr.cpp:1541-1542
@@ -1544,1 +1540,4 @@
+  if (isa<VarTemplateSpecializationDecl>(D)) {
+    assert(getLangOpts().CPlusPlus1y &&
+           "Variable templates are only supported in C++1y");
 
----------------
Remove this.

================
Comment at: lib/Sema/SemaExpr.cpp:1553-1559
@@ +1552,9 @@
+                            NameInfo.getLoc(), Ty, VK, FoundD, TemplateArgs);
+  } else {
+  E = DeclRefExpr::Create(Context,
+                          SS ? SS->getWithLocInContext(Context)
+                              : NestedNameSpecifierLoc(),
+                          SourceLocation(),
+                          D, refersToEnclosingScope,
+                          NameInfo, Ty, VK, FoundD);
+  }
----------------
Reindent. Also, assert(!TemplateArgs) here?

================
Comment at: lib/Sema/SemaExpr.cpp:2036
@@ +2035,3 @@
+    // The single lookup result must be a variable template declaration.
+    if (getLangOpts().CPlusPlus1y &&
+        Id.getKind() == UnqualifiedId::IK_TemplateId && 
----------------
No C++1y check here; we're supporting variable templates in earlier languages as an extension.

================
Comment at: lib/Sema/SemaExpr.cpp:2045-2046
@@ +2044,4 @@
+  
+  // FIXME: Should we be able to deduce variable template arguments here, 
+  // e.g. based on the context of its use? 
+  
----------------
Nope.

================
Comment at: lib/Sema/SemaExpr.cpp:11184-11185
@@ -11149,2 +11183,4 @@
     else {
+      // In C++1y, only diagnose when the variable is not a 
+      // variable specialization.
       if (BuildAndDiagnose)
----------------
Remove comment: no need for a special case here; variable template specializations never have automatic storage duration so aren't candidates to be captured anyway.

================
Comment at: lib/Sema/SemaExpr.cpp:11571
@@ -11536,1 +11570,3 @@
+  
+  if (/*Var->isUsed(false) ||*/ !IsPotentiallyEvaluatedContext(SemaRef))
     return;
----------------
Did you mean to leave this in the patch?

================
Comment at: lib/Sema/SemaExpr.cpp:11603-11604
@@ +11602,4 @@
+  else if(isa<VarTemplateSpecializationDecl>(Var)) {
+    assert(SemaRef.getLangOpts().CPlusPlus1y &&
+           "Variable templates are only supported in C++1y");
+    assert(!isa<VarTemplatePartialSpecializationDecl>(Var) && 
----------------
Remove this.

================
Comment at: lib/Sema/SemaExpr.cpp:11622-11623
@@ +11621,4 @@
+        const TemplateArgumentListInfo &VarTemplateArgs = VarSpec->getTemplateArgsInfo();
+        if (!TemplateSpecializationType::anyDependentTemplateArguments(
+               VarTemplateArgs, InstantiationDependent))
+          // Do not instantiate those specializations that are still type-dependent.
----------------
Should this check be outside the usable-in-constant-expressions check?

================
Comment at: lib/Sema/SemaExpr.cpp:11641-11647
@@ -11572,8 +11640,9 @@
   if (E && !isa<ParmVarDecl>(Var) &&
       Var->isUsableInConstantExpressions(SemaRef.Context) &&
-      Var->getAnyInitializer(DefVD) && DefVD->checkInitIsICE()) {
+      (Var->getAnyInitializer(DefVD) && DefVD->checkInitIsICE())) {
     if (!Var->getType()->isReferenceType())
       SemaRef.MaybeODRUseExprs.insert(E);
-  } else
+  } else {
     MarkVarDeclODRUsed(SemaRef, Var, Loc);
+  }
 }
----------------
Revert these (non-)changes.

================
Comment at: lib/Sema/SemaTemplate.cpp:2281-2283
@@ +2280,5 @@
+                                                                           
+static bool CheckTemplatePartialSpecializationArgs(Sema &S,
+			 TemplateParameterList *TemplateParams,
+		SmallVectorImpl<TemplateArgument> &TemplateArgs);
+
----------------
Indentation.

================
Comment at: lib/Sema/SemaTemplate.cpp:2293
@@ +2292,3 @@
+
+// This adapted from ActOnClassTemplateSpecialization()
+DeclResult
----------------
Drop this comment, it's not really useful to future readers of the code.

================
Comment at: lib/Sema/SemaTemplate.cpp:2300
@@ +2299,3 @@
+                                     TemplateParameterList *TemplateParams,
+                                     bool isPartialSpecialization) {
+  assert(VarTemplate && "A variable template id without template?");
----------------
"Is", not "is"

================
Comment at: lib/Sema/SemaTemplate.cpp:2303
@@ +2302,3 @@
+
+  // D must be declarator id.
+  assert(D.getName().getKind() == UnqualifiedId::IK_TemplateId &&
----------------
"D must be a template-id" ?

================
Comment at: lib/Sema/SemaTemplate.cpp:2354
@@ +2353,3 @@
+  // as candidate for redefinition... (?)
+  if(!Context.hasSameType(DI->getType(), ExpectedDI->getType())) {
+    unsigned ErrStr = isPartialSpecialization ? 2 : 1;
----------------
Space after "if".

================
Comment at: lib/Sema/SemaTemplate.cpp:2388-2395
@@ +2387,10 @@
+    // FIXME: Template parameter list matters, too
+    PrevDecl
+      = VarTemplate->findPartialSpecialization(Converted.data(),
+                                                 Converted.size(),
+                                                 InsertPos);
+  else
+    PrevDecl
+      = VarTemplate->findSpecialization(Converted.data(),
+                                          Converted.size(), InsertPos);
+
----------------
Indentation.

================
Comment at: lib/Sema/SemaTemplate.cpp:2407
@@ +2406,3 @@
+  if (PrevDecl &&
+      (PrevDecl->getSpecializationKind() == TSK_Undeclared)) {
+    // Since the only prior variable template specialization with these
----------------
No parens here.

================
Comment at: lib/Sema/SemaTemplate.cpp:2443
@@ +2442,3 @@
+    if (PrevPartial && PrevPartial->getInstantiatedFromMember())
+      PrevPartial->setMemberSpecialization();
+
----------------
As before, this might be clearer if you call setMemberSpecialization on Partial.

================
Comment at: lib/Sema/SemaTemplate.cpp:2440
@@ +2439,3 @@
+
+    // If we are providing an explicit specialization of a member class
+    // template specialization, make a note of that.
----------------
...of a member variable

================
Comment at: lib/Sema/SemaTemplate.cpp:2445
@@ +2444,3 @@
+
+    // Check that all of the template parameters of the class template
+    // partial specialization are deducible from the template
----------------
...of the variable template

================
Comment at: lib/Sema/SemaTemplate.cpp:2447
@@ +2446,3 @@
+    // partial specialization are deducible from the template
+    // arguments. If not, this class template partial specialization
+    // will never be used.
----------------
...this variable template...

================
Comment at: lib/Sema/SemaTemplate.cpp:2524
@@ +2523,3 @@
+
+  if(PrevDecl) {
+    // Check that this isn't a redefinition of this specialization, 
----------------
Missing space after 'if'.

================
Comment at: lib/Sema/SemaTemplate.cpp:2559
@@ +2558,3 @@
+  assert(Template && "A variable template id without template?");
+  VarDecl* Templated = Template->getTemplatedDecl();
+  SourceLocation PointOfInstantiation = TemplateNameLoc;
----------------
"*" on the right.

================
Comment at: lib/Sema/SemaTemplate.cpp:2567
@@ +2566,3 @@
+  if (CheckTemplateArgumentList(Template, TemplateNameLoc, 
+                      const_cast<TemplateArgumentListInfo &>(TemplateArgs),
+                                false, Converted, &ExpansionIntoFixedList))
----------------
Indentation.

================
Comment at: lib/Sema/SemaTemplate.cpp:2552-2553
@@ +2551,4 @@
+
+// This is adapted from a combination of CheckTemplateIdType() and
+// TemplateDeclInstantiator::VisitVarDecl().
+DeclResult Sema::CheckVarTemplateId(VarTemplateDecl *Template,
----------------
Remove.

================
Comment at: lib/Sema/SemaTemplate.cpp:2591-2594
@@ +2590,6 @@
+        return true;
+      DI = SubstType(Templated->getTypeSourceInfo(),
+                                MultiLevelTemplateArgumentList(TemplateArgList),
+                                     Templated->getTypeSpecStartLoc(),
+                                     Templated->getDeclName());
+    }
----------------
Indentation.

================
Comment at: lib/Sema/SemaTemplate.cpp:2591
@@ +2590,3 @@
+        return true;
+      DI = SubstType(Templated->getTypeSourceInfo(),
+                                MultiLevelTemplateArgumentList(TemplateArgList),
----------------
Richard Smith wrote:
> Indentation.
What happens when the type of the variable depends on the initializer:

    template<typename T> auto var1 = T();
    template<int...N> int var2[] = { N... };

================
Comment at: lib/Sema/SemaTemplate.cpp:2617-2660
@@ +2616,46 @@
+
+    Decl->setTemplateArgsInfo(TemplateArgs);
+    Template->AddSpecialization(Decl, InsertPos);
+    if (Template->isOutOfLine())
+      Decl->setLexicalDeclContext(Templated->getLexicalDeclContext());
+    Decl->setTSCSpec(Templated->getTSCSpec());
+    Decl->setInitStyle(Templated->getInitStyle());
+    Decl->setCXXForRangeDecl(Templated->isCXXForRangeDecl());
+    Decl->setConstexpr(Templated->isConstexpr());
+    Decl->setAccess(Templated->getAccess());
+
+    if (!Templated->isStaticDataMember()) {
+      Decl->setUsed(Templated->isUsed(false));
+      Decl->setReferenced(Templated->isReferenced());
+    }
+
+    // FIXME: LateAttrs et al.?
+    InstantiateAttrs(MultiLevelTemplateArgumentList(TemplateArgList), 
+                      Templated, Decl/*, LateAttrs, StartingScope*/);
+
+    if (Decl->hasAttrs())
+      CheckAlignasUnderalignment(Decl);
+
+    // FIXME: having to fake up a LookupResult is dumb.
+    LookupResult Previous(*this, Decl->getDeclName(), Decl->getLocation(),
+                          Sema::LookupOrdinaryName, Sema::ForRedeclaration);
+    if (Templated->isStaticDataMember())
+      LookupQualifiedName(Previous, Templated->getDeclContext(), false);
+
+    CheckVariableDeclaration(Decl, Previous);
+
+    if (Templated->isOutOfLine()) {
+      Templated->getLexicalDeclContext()->addDecl(Decl);
+      Templated->getDeclContext()->makeDeclVisibleInContext(Decl);
+    } else {
+      Templated->getDeclContext()->addDecl(Decl);
+      if (Templated->getDeclContext()->isFunctionOrMethod())
+        CurrentInstantiationScope->InstantiatedLocal(Templated, Decl);
+    }
+
+    // Link instantiations of static data members back to the template from
+    // which they were instantiated.
+    if (Decl->isStaticDataMember())
+      Context.setInstantiatedFromStaticDataMember(Decl, Templated,
+                                                  TSK_ImplicitInstantiation);
+
----------------
Please factor the common code out of TemplateDeclInstantiator::VisitVarDecl and here.

================
Comment at: lib/Sema/SemaTemplate.cpp:2668
@@ +2667,3 @@
+    // specializations yet.
+    // FIXME: Unify with InstantiateClassTemplateSpecialization()?
+    bool InstantiationDependent = false;
----------------
Please do. It looks like it should be straightforward to extract this as a template parameterized on the relevant type of partial specialization decl.

================
Comment at: lib/Sema/SemaTemplate.cpp:2781
@@ +2780,3 @@
+  
+  //build an ordinary singleton decl ref.
+  return BuildDeclarationNameExpr(SS, NameInfo, cast<NamedDecl>(Decl.get()), 
----------------
"// Build"

================
Comment at: lib/Sema/SemaTemplate.cpp:2807-2808
@@ +2806,4 @@
+  if (R.getAsSingle<VarTemplateDecl>()) {
+    assert(getLangOpts().CPlusPlus1y &&
+           "Variable templates are only supported in C++1y");
+    return Owned(CheckVarTemplateId(SS, R.getLookupNameInfo(), 
----------------
Remove.

================
Comment at: lib/Sema/SemaTemplate.cpp:7260-7261
@@ +7259,4 @@
+      // Explicitly instantiate a variable template.
+      assert(getLangOpts().CPlusPlus1y &&
+       "Variable templates are only supported in C++1y");
+
----------------
Remove.

================
Comment at: lib/Sema/SemaTemplate.cpp:7277
@@ +7276,3 @@
+                                          D.getIdentifierLoc(),
+                 const_cast<const TemplateArgumentListInfo &>(TemplateArgs));
+      if(Res.isInvalid())
----------------
Is this const_cast necessary?

================
Comment at: lib/Sema/SemaTemplate.cpp:7278
@@ +7277,3 @@
+                 const_cast<const TemplateArgumentListInfo &>(TemplateArgs));
+      if(Res.isInvalid())
+        return true;
----------------
Space after 'if'.

================
Comment at: lib/Sema/SemaTemplate.cpp:7289
@@ +7288,3 @@
+      // as candidate for redefinition... (?)
+      if(!Context.hasSameType(Prev->getType(), R)) {
+        Diag(D.getIdentifierLoc(),
----------------
Space after 'if'

================
Comment at: lib/Sema/SemaTemplate.cpp:7297-7298
@@ -6738,1 +7296,4 @@
+      }
     }
+    else {
+      if (!Prev || !Prev->isStaticDataMember()) {
----------------
No linebreak here.

================
Comment at: lib/Sema/SemaTemplate.cpp:7330
@@ -6758,1 +7329,3 @@
+            (D.getName().getKind() != UnqualifiedId::IK_TemplateId &&
+                D.getCXXScopeSpec().isSet())))
       Diag(D.getIdentifierLoc(),
----------------
Why check the scope specifier here? It looks like this won't reject:

    template<typename T> T var = T();
    template int var;

Do we handle that elsewhere?

Please also update the preceding quotation based on N3690 (the new wording includes some relevant text about variable templates.)

================
Comment at: lib/Sema/SemaTemplate.cpp:7341
@@ +7340,3 @@
+    if(MemberSpecializationInfo *MSInfo = Prev->getMemberSpecializationInfo()) {
+      assert(MSInfo && "Missing static data member specialization info?");
+      if (CheckSpecializationInstantiationRedecl(D.getIdentifierLoc(), TSK, Prev,
----------------
This assert is dead. Maybe change the "else if (PrevTemplate) {" below to "else { assert(PrevTemplate && ...)" ?

================
Comment at: lib/Sema/SemaTemplate.cpp:7347
@@ +7346,3 @@
+        return true;
+    } else if(PrevTemplate) {
+      if (CheckSpecializationInstantiationRedecl(D.getIdentifierLoc(), TSK, Prev,
----------------
Space after 'if'

================
Comment at: lib/Sema/SemaTemplateDeduction.cpp:2412
@@ +2411,3 @@
+/// \brief Perform template argument deduction to determine whether
+/// the given template arguments match the given class template
+/// partial specialization per C++ [temp.class.spec.match].
----------------
...given variable template

================
Comment at: lib/Sema/SemaTemplateDeduction.cpp:4477-4488
@@ -4311,1 +4476,14 @@
 
+/// \brief Returns the more specialized class template partial specialization
+/// according to the rules of partial ordering of class template partial
+/// specializations (C++ [temp.class.order]).
+///
+/// \param PS1 the first class template partial specialization
+///
+/// \param PS2 the second class template partial specialization
+///
+/// \returns the more specialized class template partial specialization. If
+/// neither partial specialization is more specialized, returns NULL.
+/// TODO: Unify with ClassTemplatePartialSpecializationDecl version.
+VarTemplatePartialSpecializationDecl *
+Sema::getMoreSpecializedPartialSpecialization(
----------------
s/class/variable/g

================
Comment at: lib/Sema/SemaTemplateDeduction.cpp:4497-4498
@@ +4496,4 @@
+
+  //QualType PT1 = PS1->getInjectedSpecializationType();
+  //QualType PT2 = PS2->getInjectedSpecializationType();
+  assert(PS1->getSpecializedTemplate() == PS1->getSpecializedTemplate() &&
----------------
Remove?

================
Comment at: lib/Sema/SemaTemplateDeduction.cpp:4489-4550
@@ +4488,64 @@
+VarTemplatePartialSpecializationDecl *
+Sema::getMoreSpecializedPartialSpecialization(
+                                  VarTemplatePartialSpecializationDecl *PS1,
+                                  VarTemplatePartialSpecializationDecl *PS2,
+                                              SourceLocation Loc) {
+
+  SmallVector<DeducedTemplateArgument, 4> Deduced;
+  TemplateDeductionInfo Info(Loc);
+
+  //QualType PT1 = PS1->getInjectedSpecializationType();
+  //QualType PT2 = PS2->getInjectedSpecializationType();
+  assert(PS1->getSpecializedTemplate() == PS1->getSpecializedTemplate() &&
+          "the partial specializations being compared should specialize"
+          " the same template.");
+  TemplateName Name(PS1->getSpecializedTemplate());
+  TemplateName CanonTemplate = Context.getCanonicalTemplateName(Name);
+  QualType PT1
+    = Context.getTemplateSpecializationType(CanonTemplate, 
+                                            PS1->getTemplateArgs().data(),
+                                            PS1->getTemplateArgs().size());
+  QualType PT2
+    = Context.getTemplateSpecializationType(CanonTemplate, 
+                                            PS2->getTemplateArgs().data(),
+                                            PS2->getTemplateArgs().size());
+
+  // Determine whether PS1 is at least as specialized as PS2
+  Deduced.resize(PS2->getTemplateParameters()->size());
+  bool Better1 = !DeduceTemplateArgumentsByTypeMatch(*this,
+                                            PS2->getTemplateParameters(),
+                                            PT2, PT1, Info, Deduced, TDF_None,
+                                            /*PartialOrdering=*/true,
+                                            /*RefParamComparisons=*/0);
+  if (Better1) {
+    SmallVector<TemplateArgument, 4> DeducedArgs(Deduced.begin(),Deduced.end());
+    InstantiatingTemplate Inst(*this, PS2->getLocation(), PS2,
+                               DeducedArgs, Info);
+    Better1 = !::FinishTemplateArgumentDeduction(*this, PS2,
+                                                 PS1->getTemplateArgs(),
+                                                 Deduced, Info);
+  }
+
+  // Determine whether PS2 is at least as specialized as PS1
+  Deduced.clear();
+  Deduced.resize(PS1->getTemplateParameters()->size());
+  bool Better2 = !DeduceTemplateArgumentsByTypeMatch(*this,
+                                            PS1->getTemplateParameters(),
+                                            PT1, PT2, Info, Deduced, TDF_None,
+                                            /*PartialOrdering=*/true,
+                                            /*RefParamComparisons=*/0);
+  if (Better2) {
+    SmallVector<TemplateArgument, 4> DeducedArgs(Deduced.begin(),Deduced.end());
+    InstantiatingTemplate Inst(*this, PS1->getLocation(), PS1,
+                               DeducedArgs, Info);
+    Better2 = !::FinishTemplateArgumentDeduction(*this, PS1,
+                                                 PS2->getTemplateArgs(),
+                                                 Deduced, Info);
+  }
+
+  if (Better1 == Better2)
+    return 0;
+
+  return Better1? PS1 : PS2;
+}
+
----------------
Instead of duplicating it, can you template the existing getMoreSpecializedPartialSpecialization on the type of PartialSpecializationDecl?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:336
@@ +335,3 @@
+    assert(VarTemplate &&
+	   "A template specialization without specialized template?");
+    
----------------
Indentation / join with previous line.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:324-325
@@ +323,4 @@
+  
+  // In C++11: Check whether there is already a variable template
+  // specialization for this declaration.
+  TemplateArgumentListInfo VarTemplateArgsInfo;
----------------
"In C++11"?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:330-331
@@ +329,4 @@
+  if (isa<VarTemplateSpecializationDecl>(D)) {
+    assert(SemaRef.getLangOpts().CPlusPlus1y &&
+           "Variable templates are only supported in C++1y");
+    VarTemplateSpecializationDecl *VarSpec
----------------
Remove.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:389
@@ +388,3 @@
+  VarDecl *PrevDecl = 0;
+  if (SemaRef.getLangOpts().CPlusPlus1y && D->getPreviousDecl()) {
+    if (D->getDescribedVarTemplate()) {
----------------
No test for C++1y here.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:490-491
@@ +489,4 @@
+    // Instantiation is handled by DoMarkVarDeclReferenced().
+  }
+  else
+    SemaRef.InstantiateVariableInitializer(Var, D, TemplateArgs);
----------------
No linebreak here.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:1090-1092
@@ +1089,5 @@
+  
+  assert(SemaRef.getLangOpts().CPlusPlus1y && 
+          "Variable template are only supported in C++1y");
+
+  // Create a local instantiation scope for this variable template, which
----------------
Remove.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:1114-1115
@@ +1113,4 @@
+  DeclContext *DC = Owner;
+  /*
+  // If this is the variable for an anonymous struct or union,
+  // instantiate the anonymous struct/union type first.
----------------
Add a FIXME here to revisit this.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:1219
@@ +1218,3 @@
+  if (Found.empty())
+    return 0;
+
----------------
Can this happen? Should we assert here?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:1223-1224
@@ +1222,4 @@
+    = dyn_cast<VarTemplateDecl>(Found.front());
+  if (!InstVarTemplate)
+    return 0;
+
----------------
Likewise.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:2789
@@ +2788,3 @@
+    SemaRef.Diag(PrevDecl->getLocation(), diag::note_var_prev_partial_spec_here);
+      //<< SemaRef.Context.getTypeDeclType(PrevDecl);
+    return 0;
----------------
Remove?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3427
@@ +3426,3 @@
+      } else
+        ActOnUninitializedDecl(Var, TypeMayContainAuto);
+    } else {
----------------
Can this happen?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3429-3431
@@ +3428,5 @@
+    } else {
+      // FIXME: Not too happy about invalidating the declaration
+      // because of a bogus initializer.
+      Var->setInvalidDecl();
+    }
----------------
Should we call ActOnInitializerError here?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3439-3445
@@ +3438,9 @@
+  
+  /*
+  // Diagnose unused local variables with dependent types, where the diagnostic
+  // will have been deferred.
+  if (!Var->isInvalidDecl() && Owner->isFunctionOrMethod() && !Var->isUsed() &&
+      D->getType()->isDependentType())
+    SemaRef.DiagnoseUnusedDecl(Var);
+  */
+}
----------------
Why commented out?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3494
@@ +3493,3 @@
+    if (DefinitionRequired) {
+      VarDecl* Def = Var->getInstantiatedFromStaticDataMember();
+      if (!Def)
----------------
"*" on the right. Also, maybe inline the call into the 'if', since you're not otherwise using the variable.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3514
@@ +3513,3 @@
+    
+   return;
+  }
----------------
Indentation.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3540
@@ +3539,3 @@
+  // PushDeclContext because we don't have a scope.
+  ContextRAII previousContext(*this, Var->getDeclContext());
+  LocalInstantiationScope Scope(*this);
----------------
Capital P.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3546
@@ +3545,3 @@
+
+  if(MemberSpecializationInfo *MSInfo = Var->getMemberSpecializationInfo())
+    Var->setTemplateSpecializationKind(MSInfo->getTemplateSpecializationKind(),
----------------
Space after 'if'

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3885-3898
@@ -3353,2 +3884,16 @@
 }
+/*
+static bool isInstantiationOf(VarTemplateDecl *Pattern,
+                              VarTemplateDecl *Instance) {
+  Pattern = Pattern->getCanonicalDecl();
+
+  do {
+    Instance = Instance->getCanonicalDecl();
+    if (Pattern == Instance) return true;
+    Instance = Instance->getInstantiatedFromMemberTemplate();
+  } while (Instance);
+
+  return false;
+}
+ */
 
----------------
Remove?

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4161-4163
@@ +4160,5 @@
+    if (TemplateSpecializationType::anyDependentTemplateArguments(
+          VarTemplateArgs, InstantiationDependent)) {
+      D = cast<NamedDecl>(SubstDecl(D, CurContext, TemplateArgs));
+    }
+    return D;
----------------
No braces.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4155
@@ +4154,3 @@
+  // that are still type-dependent.
+  if(isa<VarTemplateSpecializationDecl>(D)) {
+    VarTemplateSpecializationDecl *VarSpec
----------------
Space after 'if'.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4155-4157
@@ +4154,5 @@
+  // that are still type-dependent.
+  if(isa<VarTemplateSpecializationDecl>(D)) {
+    VarTemplateSpecializationDecl *VarSpec
+      = cast<VarTemplateSpecializationDecl>(D);
+    bool InstantiationDependent = false;
----------------
Richard Smith wrote:
> Space after 'if'.
Use

    if (VarTemplateSpecializationDecl *VarSpec = dyn_cast<VarTemplateSpecializationDecl>(D)) {

Rather than isa + cast.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:4348-4349
@@ +4347,4 @@
+          = dyn_cast<VarTemplateSpecializationDecl>(Inst.first)) {
+      assert(getLangOpts().CPlusPlus1y && 
+             "variable templates are only supported in c++1y");
+      PrettyDeclStackTraceEntry CrashInfo(*this, VarSpec, SourceLocation(),
----------------
Remove.

================
Comment at: lib/Sema/TreeTransform.h:6205
@@ -6205,2 +6205,2 @@
                                                          E->getDecl()));
   if (!ND)
----------------
Nothing changed in this file but whitespace; revert?

================
Comment at: lib/Serialization/ASTWriterDecl.cpp:1195-1207
@@ +1194,15 @@
+  if (D->isFirstDeclaration()) {
+    typedef llvm::FoldingSetVector<VarTemplateSpecializationDecl> CTSDSetTy;
+    CTSDSetTy &CTSDSet = D->getSpecializations();
+    Record.push_back(CTSDSet.size());
+    for (CTSDSetTy::iterator I=CTSDSet.begin(), E = CTSDSet.end(); I!=E; ++I) {
+      assert(I->isCanonicalDecl() && "Expected only canonical decls in set");
+      Writer.AddDeclRef(&*I, Record);
+    }
+
+    typedef llvm::FoldingSetVector<VarTemplatePartialSpecializationDecl>
+      CTPSDSetTy;
+    CTPSDSetTy &CTPSDSet = D->getPartialSpecializations();
+    Record.push_back(CTPSDSet.size());
+    for (CTPSDSetTy::iterator I=CTPSDSet.begin(), E=CTPSDSet.end(); I!=E; ++I) {
+      assert(I->isCanonicalDecl() && "Expected only canonical decls in set");
----------------
s/CT/VT/g

================
Comment at: lib/Serialization/ASTWriterDecl.cpp:1211-1212
@@ +1210,4 @@
+    }
+
+   // Writer.AddTypeRef(D->getCommonPtr()->InjectedClassNameType, Record);
+  }
----------------
Remove

================
Comment at: lib/Serialization/ASTWriterDecl.cpp:1217-1271
@@ +1216,57 @@
+
+void ASTDeclWriter::VisitVarTemplateSpecializationDecl(
+                                           VarTemplateSpecializationDecl *D) {
+  VisitVarDecl(D);
+
+  llvm::PointerUnion<VarTemplateDecl *,
+                     VarTemplatePartialSpecializationDecl *> InstFrom
+    = D->getSpecializedTemplateOrPartial();
+  if (Decl *InstFromD = InstFrom.dyn_cast<VarTemplateDecl *>()) {
+    Writer.AddDeclRef(InstFromD, Record);
+  } else {
+    Writer.AddDeclRef(InstFrom.get<VarTemplatePartialSpecializationDecl *>(),
+                      Record);
+    Writer.AddTemplateArgumentList(&D->getTemplateInstantiationArgs(), Record);
+  }
+
+  // Explicit info.
+  Writer.AddTypeSourceInfo(D->getTypeAsWritten(), Record);
+  if (D->getTypeAsWritten()) {
+    Writer.AddSourceLocation(D->getExternLoc(), Record);
+    Writer.AddSourceLocation(D->getTemplateKeywordLoc(), Record);
+  }
+
+  Writer.AddTemplateArgumentList(&D->getTemplateArgs(), Record);
+  Writer.AddSourceLocation(D->getPointOfInstantiation(), Record);
+  Record.push_back(D->getSpecializationKind());
+  Record.push_back(D->isCanonicalDecl());
+
+  if (D->isCanonicalDecl()) {
+    // When reading, we'll add it to the folding set of the following template. 
+    Writer.AddDeclRef(D->getSpecializedTemplate()->getCanonicalDecl(), Record);
+  }
+
+  Code = serialization::DECL_VAR_TEMPLATE_SPECIALIZATION;
+}
+
+void ASTDeclWriter::VisitVarTemplatePartialSpecializationDecl(
+                                    VarTemplatePartialSpecializationDecl *D) {
+  VisitVarTemplateSpecializationDecl(D);
+
+  Writer.AddTemplateParameterList(D->getTemplateParameters(), Record);
+
+  Record.push_back(D->getNumTemplateArgsAsWritten());
+  for (int i = 0, e = D->getNumTemplateArgsAsWritten(); i != e; ++i)
+    Writer.AddTemplateArgumentLoc(D->getTemplateArgsAsWritten()[i], Record);
+
+  Record.push_back(D->getSequenceNumber());
+
+  // These are read/set from/to the first declaration.
+  if (D->getPreviousDecl() == 0) {
+    Writer.AddDeclRef(D->getInstantiatedFromMember(), Record);
+    Record.push_back(D->isMemberSpecialization());
+  }
+
+  Code = serialization::DECL_VAR_TEMPLATE_PARTIAL_SPECIALIZATION;
+}
+
----------------
It looks like the corresponding code in ASTDeclReader is missing?


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



More information about the cfe-commits mailing list