[clang-tools-extra] 2469bd9 - [clang-tidy] Disable misc-definitions-in-headers for declarations in anonymous namespaces

[Carlos Galvez via cfe-commits]

Author: Carlos Galvez
Date: 2023-04-01T19:19:00Z
New Revision: 2469bd98a77242606426f8a5a8a797054c932bf0

URL: https://github.com/llvm/llvm-project/commit/2469bd98a77242606426f8a5a8a797054c932bf0
DIFF: https://github.com/llvm/llvm-project/commit/2469bd98a77242606426f8a5a8a797054c932bf0.diff

LOG: [clang-tidy] Disable misc-definitions-in-headers for declarations in anonymous namespaces

Anonymous namespaces are another way of providing internal linkage,
and the check already ignores other cases of internal linkage in
headers, via "static" or "const".

Anonymous namespaces in headers are definitely a source of pitfalls,
but it's not the responsibility of this check to cover that. Instead,
google-build-namespaces will specifically warn about this issue.

Fixes https://github.com/llvm/llvm-project/issues/61471

Differential Revision: https://reviews.llvm.org/D147379

Added: 
    

Modified: 
    clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
    clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
index 67e1a04a35afe..2c07b30429f96 100644
--- a/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ b/clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -90,13 +90,11 @@ void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) {
   // Internal linkage variable definitions are ignored for now:
   //   const int a = 1;
   //   static int b = 1;
+  //   namespace { int c = 1; }
   //
   // Although these might also cause ODR violations, we can be less certain and
   // should try to keep the false-positive rate down.
-  //
-  // FIXME: Should declarations in anonymous namespaces get the same treatment
-  // as static / const declarations?
-  if (!ND->hasExternalFormalLinkage() && !ND->isInAnonymousNamespace())
+  if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
     return;
 
   if (const auto *FD = dyn_cast<FunctionDecl>(ND)) {

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 156e9d73c7be0..099211e592547 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -199,6 +199,10 @@ Changes in existing checks
   <clang-tidy/checks/misc/definitions-in-headers>` check.
   Global options of the same name should be used instead.
 
+- Fixed false positive in :doc:`misc-definitions-in-headers
+  <clang-tidy/checks/misc/definitions-in-headers>` to avoid warning on
+  declarations inside anonymous namespaces.
+
 - Deprecated check-local options `HeaderFileExtensions`
   in :doc:`misc-unused-using-decls
   <clang-tidy/checks/misc/unused-using-decls>` check.

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst b/clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
index dcded6a42a6d9..08aa9d884c239 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
@@ -27,6 +27,7 @@ from multiple translation units.
    const int c = 1;
    const char* const str2 = "foo";
    constexpr int k = 1;
+   namespace { int x = 1; }
 
    // Warning: function definition.
    int g() {

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp b/clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
index f57cc2c256fef..4cf07077a230a 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
@@ -108,8 +108,7 @@ int f3() {
 int f5(); // OK: function declaration.
 inline int f6() { return 1; } // OK: inline function definition.
 namespace {
-  int f7() { return 1; }
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: function 'f7' defined in a header file;
+  int f7() { return 1; }  // OK: each TU defines the function in a unique namespace.
 }
 
 int f8() = delete; // OK: the function being marked delete is not callable.
@@ -142,8 +141,7 @@ const char* ca = "foo";
 // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: variable 'ca' defined in a header file;
 
 namespace {
-  int e = 2;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'e' defined in a header file;
+  int e = 2;  // OK: each TU defines the variable in a unique namespace.
 }
 
 const char* const g = "foo"; // OK: internal linkage variable definition.