rsmith added a comment.
I expect this patch to cause problems if the two definitions of the variable
template come from different modules, because at deserialization time we don't
merge the definitions together sensibly (it looks like we end up with a
redeclaration chain with multiple declarations, multiple of which believe they
are "the" defintiion).
================
Comment at: lib/Sema/SemaTemplate.cpp:470
@@ -470,1 +469,3 @@
+ assert(isa<TagDecl>(Instantiation) || isa<FunctionDecl>(Instantiation)
+ || isa<VarDecl>(Instantiation));
----------------
`||` on the previous line, please.
================
Comment at: lib/Sema/SemaTemplate.cpp:472
@@ -470,3 +471,3 @@
- if (PatternDef && (isa<FunctionDecl>(PatternDef)
- || !cast<TagDecl>(PatternDef)->isBeingDefined())) {
+ bool isEntityBeingDefined = false;
+ if (const TagDecl *TD = dyn_cast_or_null<TagDecl>(PatternDef))
----------------
Variable names should start with a capital letter.
================
Comment at: lib/Sema/SemaTemplate.cpp:478
@@ -473,3 +477,3 @@
NamedDecl *SuggestedDef = nullptr;
if (!hasVisibleDefinition(const_cast<NamedDecl*>(PatternDef),
&SuggestedDef,
/*OnlyNeedComplete*/false)) {
----------------
We'll need to extend `hasVisibleDefinition` to handle merged `VarDecl`s to
support this. (The `ASTReader` doesn't currently merge together `VarDecl`
definitions in a reasonable way.)
================
Comment at: lib/Sema/SemaTemplate.cpp:509
@@ -504,3 +508,3 @@
<< 1 << Instantiation->getDeclName() <<
Instantiation->getDeclContext();
- } else {
+ } else if (isa<TagDecl>(Instantiation)) {
Diag(PointOfInstantiation,
----------------
`else if` doesn't make sense here -- we either need to produce a diagnostic on
all paths through here, or suppress the notes if we didn't produce a diagnostic.
================
Comment at: lib/Sema/SemaTemplate.cpp:529
@@ +528,3 @@
+ Note = diag::note_template_decl_here;
+ } else if (isa<VarDecl>(Instantiation)) {
+ if (isa<VarTemplateSpecializationDecl>(Instantiation)) {
----------------
Likewise here.
Repository:
rL LLVM
https://reviews.llvm.org/D24508
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
