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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to