rsmith added inline comments.

================
Comment at: include/clang/Basic/Attr.td:2125
@@ +2124,3 @@
+
+def InternalLinkage : InheritableAttr {
+  let Spellings = [GNU<"internal_linkage">, CXX11<"clang", 
"internal_linkage">];
----------------
`InheritableAttr` doesn't seem right for the case when this is applied to a 
namespace.

================
Comment at: lib/AST/Decl.cpp:1358-1362
@@ -1346,3 +1357,7 @@
     }
-    assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage());
+    // Linkages may also differ if one of the declarations has
+    // InternalLinkageAttr.
+    assert(!Old || Old->getCachedLinkage() == D->getCachedLinkage() ||
+           (Old->hasAttr<InternalLinkageAttr>() !=
+            D->hasAttr<InternalLinkageAttr>()));
 #endif
----------------
We shouldn't allow the linkage to be changed after the first declaration. What 
cases cause problems here?

================
Comment at: lib/Sema/Sema.cpp:539
@@ -538,3 +538,3 @@
 
-    if (!ND->isExternallyVisible()) {
+    if (!ND->isExternallyVisible() || ND->hasAttr<InternalLinkageAttr>()) {
       S.Diag(ND->getLocation(), diag::warn_undefined_internal)
----------------
Hmm, what should `isExternallyVisible` return for a declaration with this 
attribute? Presumably it should be `false`...


Repository:
  rL LLVM

http://reviews.llvm.org/D13925



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to