[PATCH] Top-level variable declaration (all but static data members)
Larisse Voufo
lvoufo at google.com
Tue Jul 30 13:38:11 PDT 2013
================
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
----------------
Richard Smith wrote:
> 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).
Right. I've added a FIXME to this effect.
================
Comment at: lib/AST/ASTImporter.cpp:106
@@ -106,2 +106,2 @@
bool ImportDefinition(RecordDecl *From, RecordDecl *To,
----------------
Richard Smith wrote:
> 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.
So... Is this an invitation to drop these changes, or to keep them around at least for future-proofness reasons?
================
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() {
----------------
Richard Smith wrote:
> You're not using PrevDecl here; remove it?
Ow. Good catch.
================
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);
----------------
Richard Smith wrote:
> More to the point, function parameters cannot be redeclarations. There's no fundamental reason to only use PrevDecl for variable templates.
Right. I've removed PrevDecl from VarDecl.
================
Comment at: lib/AST/DeclTemplate.cpp:1208-1209
@@ +1207,4 @@
+{
+ // FIXME: Is this needed for variable templates?
+ //AdoptTemplateParameterList(Params, this);
+}
----------------
Richard Smith wrote:
> Maybe adopt them into DC (though they should presumably be there already).
I think they should be in DC by now. I leave this commented out for now, and remove it once I'm sure that there are no unforeseen corner case.
================
Comment at: lib/Sema/SemaExpr.cpp:11571
@@ -11536,1 +11570,3 @@
+
+ if (/*Var->isUsed(false) ||*/ !IsPotentiallyEvaluatedContext(SemaRef))
return;
----------------
Richard Smith wrote:
> Did you mean to leave this in the patch?
Yes. But I don't find it necessary anymore.
================
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;
+}
+
----------------
Richard Smith wrote:
> Instead of duplicating it, can you template the existing getMoreSpecializedPartialSpecialization on the type of PartialSpecializationDecl?
Marked as TODO.
================
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;
+}
+
----------------
Richard Smith wrote:
> It looks like the corresponding code in ASTDeclReader is missing?
Right. I had missed that. Thanks.
================
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);
+ */
+}
----------------
Richard Smith wrote:
> Why commented out?
I had meant to remove it. This block of code is only used in TemplateDeclInstantiator::VisitVarDecl()
================
Comment at: lib/Sema/SemaTemplate.cpp:2591
@@ +2590,3 @@
+ return true;
+ DI = SubstType(Templated->getTypeSourceInfo(),
+ MultiLevelTemplateArgumentList(TemplateArgList),
----------------
Richard Smith wrote:
> 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... };
Added a FIXME for now.
================
Comment at: lib/Sema/SemaTemplate.cpp:7330
@@ -6758,1 +7329,3 @@
+ (D.getName().getKind() != UnqualifiedId::IK_TemplateId &&
+ D.getCXXScopeSpec().isSet())))
Diag(D.getIdentifierLoc(),
----------------
Richard Smith wrote:
> 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.)
It parses "template int var" as an explicit instantiation and complains about "too few arguments".
================
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.
----------------
Richard Smith wrote:
> Add a FIXME here to revisit this.
Removed.
http://llvm-reviews.chandlerc.com/D1067
More information about the cfe-commits
mailing list