[PATCH] Adding static data member templates.

Richard Smith richard at metafoo.co.uk
Wed Aug 21 13:17:42 PDT 2013


  Please split the refactoring of `CheckVariableDeclaration` out into a separate patch from the functional changes.


================
Comment at: lib/Sema/SemaDecl.cpp:5210-5211
@@ +5209,4 @@
+  if (IsVariableTemplate) {
+    if (PrevVarTemplate)
+      mergeDeclAttributes(NewVD, PrevVarTemplate->getTemplatedDecl());
+
----------------
Is this necessary? I would expect this to be handled by CheckVariableDeclaration's call to MergeVarDecl.

================
Comment at: lib/Sema/SemaDecl.cpp:5213
@@ +5212,3 @@
+
+    AddPushedVisibilityAttribute(NewVD);
+  }
----------------
This should be handled by the call to FinalizeDeclaration (called by ParseDeclarationAfterDeclaratorAndAttributes), I think?

================
Comment at: lib/Sema/SemaDecl.cpp:5351-5352
@@ -5342,4 +5350,4 @@
 
-  if (PrevVarTemplate)
-    mergeDeclAttributes(NewVD, PrevVarTemplate->getTemplatedDecl());
+    if (NewVD->isStaticDataMember() && NewVD->isOutOfLine())
+      NewTemplate->setAccess(NewVD->getAccess());
 
----------------
Looks like this only inherits access in the case where we merged it at the end of MergeVarDecl. It would be cleaner for MergeVarDecl to set the access on the template decl too.

================
Comment at: lib/Sema/SemaTemplate.cpp:2394
@@ -2393,3 +2393,3 @@
     if (PrevPartial && PrevPartial->getInstantiatedFromMember())
-      Partial->setMemberSpecialization();
+      PrevPartial->setMemberSpecialization();
 
----------------
Please split this bugfix out from the refactoring.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:2674-2678
@@ +2673,7 @@
+  // Set the initializer, to use as pattern for initialization.
+  if (VarDecl *Def = PartialSpec->getDefinition(SemaRef.getASTContext()))
+    PartialSpec = cast<VarTemplatePartialSpecializationDecl>(Def);
+  SemaRef.BuildVariableInstantiation(InstPartialSpec, PartialSpec, TemplateArgs,
+                                     LateAttrs, StartingScope);
+  InstPartialSpec->setInit(PartialSpec->getInit());
+
----------------
Likewise, please split this bugfix out to a separate change.

================
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3312-3313
@@ +3311,4 @@
+      PatternDecl->getTypeSourceInfo(),
+      MultiLevelTemplateArgumentList(*TemplateArgumentList::CreateCopy(
+          Context, Innermost.begin(), Innermost.size())),
+      PatternDecl->getTypeSpecStartLoc(), PatternDecl->getDeclName());
----------------
You can avoid making a copy here:

    MultiLevelTemplateArgumentList Innermost;
    Innermost.addOuterTemplateArguments(TemplateArgs.getInnermost());


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



More information about the cfe-commits mailing list