[PATCH] D61097: [Sema] Emit warning for visibility attribute on internal-linkage declaration

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: aaron.ballman.
aaron.ballman added inline comments.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:2619
+  // Visibility attributes have no effect on symbols with internal linkage.
+  if (auto ND = dyn_cast<NamedDecl>(D)) {
+    if (!ND->isExternallyVisible()) {
----------------
`const auto *ND` please.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:2621
+    if (!ND->isExternallyVisible()) {
+      S.Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL;
+      return;
----------------
I think it would be more helpful for users if we introduced a new diagnostic here that explained why the attribute is being ignored, otherwise the diagnostic is somewhat indecipherable.


================
Comment at: lib/Sema/SemaDeclAttr.cpp:2622
+      S.Diag(AL.getRange().getBegin(), diag::warn_attribute_ignored) << AL;
+      return;
+    }
----------------
We shouldn't early return here (it's not an error, just a warning), so that the other diagnostics can also trigger if needed.


Repository:
  rC Clang

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

https://reviews.llvm.org/D61097