[PATCH] D24508: PR28752: Do not instantiate var decls which are not visible.

[Richard Smith via cfe-commits]

rsmith added a comment.

I think you'll also need to update the `ASTDeclReader` to merge `VarDecl` definitions together if it reads a definition and there already is one in the AST. I note that right now `Sema::AddInitializerToDecl` permits multiple definitions of a `VarDecl`, which doesn't seem like what we want here; we'll probably want to merge those as we parse them, too.

Now, we have a problem here: unlike with classes and functions, we can't always convert a variable definition to a declaration by just dropping the initializer. Perhaps we can add a flag to `VarDecl` to indicate "this is just a declaration, even though it looks like a definition", to handle that case? (This would also be useful for `VarTemplateSpecializationDecl`s, where we currently reject valid code such as "template<typename> int n; int k = n<int>;" because we have no way to represent the difference between a mere declaration and a definition of `n<int>`.)


================
Comment at: lib/Sema/SemaTemplate.cpp:505
@@ -499,3 +504,3 @@
     Instantiation->setInvalidDecl();
   } else if (InstantiatedFromMember) {
     if (isa<FunctionDecl>(Instantiation)) {
----------------
Why do we not issue a diagnostic in this case for a `VarDecl` when `Complain` is true and no definition is available? It seems like we should either be diagnosing this or asserting that it can't happen.

================
Comment at: lib/Sema/SemaTemplate.cpp:528
@@ +527,3 @@
+      Note = diag::note_template_decl_here;
+    } else if (isa<VarDecl>(Instantiation)) {
+      if (isa<VarTemplateSpecializationDecl>(Instantiation)) {
----------------
`else` + `assert` would make more sense here. No other kind of declaration should get here.

================
Comment at: lib/Sema/SemaType.cpp:6898-6899
@@ +6897,4 @@
+  } else if (auto *VD = dyn_cast<VarDecl>(D)) {
+    // FIXME: Handle the case where D is a VarTemplateSpecializationDecl, i.e. D
+    // is not a static data member.
+    if (auto *Pattern = VD->getInstantiatedFromStaticDataMember())
----------------
We should add a `getTemplateInstantiationPattern()` to `VarDecl` and use it from here.


https://reviews.llvm.org/D24508