[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: lib/Sema/SemaDecl.cpp:5971
     auto *VD = dyn_cast<VarDecl>(&ND);
-    if (!ND.isExternallyVisible() || (VD && VD->isStaticLocal())) {
+    NamespaceDecl *NS = NULL;
+    if (VD)
----------------
s/NULL/nullptr

Also, I think this should be `const NamespaceDecl *`.


================
Comment at: lib/Sema/SemaDecl.cpp:5973
+    if (VD)
+      NS = dyn_cast<NamespaceDecl>(VD->getDeclContext());
+    int isAnonymousNS = NS && NS->getDeclName().isEmpty();
----------------
Does only the immediate declaration context matter? How is this handled?
```
namespace {
namespace named {
  __declspec(dllexport) int const x = 3; 
}
}
```


================
Comment at: lib/Sema/SemaDecl.cpp:5975-5977
+    if ((!ND.isExternallyVisible() &&
+      (!isAnonymousNS || !(VD && VD->hasInit()))) ||
+      (VD && VD->isStaticLocal())) {
----------------
This used to unconditionally warn, but now has predicates -- should this still warn when not in MSVC mode?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45978/new/

https://reviews.llvm.org/D45978