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

[Carlos Galvez via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2469bd98a772: [clang-tidy] Disable 
misc-definitions-in-headers for declarations in anonymous… (authored by 
carlosgalvezp).

Changed prior to commit:
  https://reviews.llvm.org/D147379?vs=510233&id=510257#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147379

Files:
  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


Index: 
clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
@@ -108,8 +108,7 @@
 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 @@
 // 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.
Index: clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
@@ -27,6 +27,7 @@
    const int c = 1;
    const char* const str2 = "foo";
    constexpr int k = 1;
+   namespace { int x = 1; }
 
    // Warning: function definition.
    int g() {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -199,6 +199,10 @@
   <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.
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -90,13 +90,11 @@
   // 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)) {



Index: clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
===================================================================
--- clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
+++ clang-tools-extra/test/clang-tidy/checkers/misc/definitions-in-headers.hpp
@@ -108,8 +108,7 @@
 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 @@
 // 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.
Index: clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
+++ clang-tools-extra/docs/clang-tidy/checks/misc/definitions-in-headers.rst
@@ -27,6 +27,7 @@
    const int c = 1;
    const char* const str2 = "foo";
    constexpr int k = 1;
+   namespace { int x = 1; }
 
    // Warning: function definition.
    int g() {
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -199,6 +199,10 @@
   <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.
Index: clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
@@ -90,13 +90,11 @@
   // 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)) {

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