bkramer updated this revision to Diff 66716.
bkramer added a comment.

Add relnote.


https://reviews.llvm.org/D23130

Files:
  clang-tidy/misc/CMakeLists.txt
  clang-tidy/misc/GlobalNamespaceCheck.cpp
  clang-tidy/misc/GlobalNamespaceCheck.h
  clang-tidy/misc/MiscTidyModule.cpp
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/misc-global-namespace.rst
  test/clang-tidy/misc-global-namespace.cpp

Index: test/clang-tidy/misc-global-namespace.cpp
===================================================================
--- /dev/null
+++ test/clang-tidy/misc-global-namespace.cpp
@@ -0,0 +1,35 @@
+// RUN: %check_clang_tidy %s misc-global-namespace %t
+
+void f() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: definition for 'f' in the global namespace
+class F {};
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: definition for 'F' in the global namespace
+int i;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: definition for 'i' in the global namespace
+
+// No warnings below.
+void g();
+
+extern "C" void h() {}
+
+extern "C" {
+void j() {}
+}
+
+struct Clike {
+  int i;
+};
+
+void *operator new(__SIZE_TYPE__, int) { return 0; }
+void operator delete(void *, int) {}
+
+__attribute__((visibility("hidden"))) void k() {}
+static void l() {}
+namespace {
+void m() {}
+}
+namespace x {
+void n() {}
+}
+
+int main() {}
Index: docs/clang-tidy/checks/misc-global-namespace.rst
===================================================================
--- /dev/null
+++ docs/clang-tidy/checks/misc-global-namespace.rst
@@ -0,0 +1,7 @@
+.. title:: clang-tidy - misc-global-namespace
+
+misc-global-namespace
+=====================
+
+Finds definitions in the global namespace. Those definitions are prone to ODR
+conflicts.
Index: docs/clang-tidy/checks/list.rst
===================================================================
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -59,6 +59,7 @@
    misc-definitions-in-headers
    misc-fold-init-type
    misc-forward-declaration-namespace
+   misc-global-namespace
    misc-inaccurate-erase
    misc-incorrect-roundings
    misc-inefficient-algorithm
Index: docs/ReleaseNotes.rst
===================================================================
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -69,6 +69,11 @@
 
   Flags classes where some, but not all, special member functions are user-defined.
 
+- New `misc-global-namespace
+  <http://clang.llvm.org/extra/clang-tidy/checks/misc-global-namespace.html>`_ check
+
+  Flags definitions in the global namespace.
+
 Improvements to include-fixer
 -----------------------------
 
Index: clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tidy/misc/MiscTidyModule.cpp
@@ -12,18 +12,18 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "ArgumentCommentCheck.h"
 #include "AssertSideEffectCheck.h"
-#include "MisplacedConstCheck.h"
-#include "UnconventionalAssignOperatorCheck.h"
 #include "BoolPointerImplicitConversionCheck.h"
 #include "DanglingHandleCheck.h"
 #include "DefinitionsInHeadersCheck.h"
 #include "FoldInitTypeCheck.h"
 #include "ForwardDeclarationNamespaceCheck.h"
+#include "GlobalNamespaceCheck.h"
 #include "InaccurateEraseCheck.h"
 #include "IncorrectRoundings.h"
 #include "InefficientAlgorithmCheck.h"
 #include "MacroParenthesesCheck.h"
 #include "MacroRepeatedSideEffectsCheck.h"
+#include "MisplacedConstCheck.h"
 #include "MisplacedWideningCastCheck.h"
 #include "MoveConstantArgumentCheck.h"
 #include "MoveConstructorInitCheck.h"
@@ -44,6 +44,7 @@
 #include "SuspiciousStringCompareCheck.h"
 #include "SwappedArgumentsCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
+#include "UnconventionalAssignOperatorCheck.h"
 #include "UndelegatedConstructor.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
@@ -62,6 +63,7 @@
     CheckFactories.registerCheck<ArgumentCommentCheck>("misc-argument-comment");
     CheckFactories.registerCheck<AssertSideEffectCheck>(
         "misc-assert-side-effect");
+    CheckFactories.registerCheck<GlobalNamespaceCheck>("misc-global-namespace");
     CheckFactories.registerCheck<MisplacedConstCheck>(
         "misc-misplaced-const");
     CheckFactories.registerCheck<UnconventionalAssignOperatorCheck>(
Index: clang-tidy/misc/GlobalNamespaceCheck.h
===================================================================
--- /dev/null
+++ clang-tidy/misc/GlobalNamespaceCheck.h
@@ -0,0 +1,36 @@
+//===--- GlobalNamespaceCheck.h - clang-tidy---------------------*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_GLOBAL_NAMESPACE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_GLOBAL_NAMESPACE_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds definitions in the global namespace. Those definitions are prone to
+/// ODR conflicts.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-global-namespace.html
+class GlobalNamespaceCheck : public ClangTidyCheck {
+public:
+  GlobalNamespaceCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_GLOBAL_NAMESPACE_H
Index: clang-tidy/misc/GlobalNamespaceCheck.cpp
===================================================================
--- /dev/null
+++ clang-tidy/misc/GlobalNamespaceCheck.cpp
@@ -0,0 +1,78 @@
+//===--- GlobalNamespaceCheck.cpp - clang-tidy-----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "GlobalNamespaceCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void GlobalNamespaceCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+    return;
+
+  Finder->addMatcher(
+      namedDecl(hasDeclContext(translationUnitDecl()),
+                anyOf(functionDecl(isDefinition(), unless(isDeleted()),
+                                   unless(isExternC())),
+                      varDecl(isDefinition()), cxxRecordDecl(isDefinition())))
+          .bind("decl"),
+      this);
+}
+
+void GlobalNamespaceCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedDecl = Result.Nodes.getNodeAs<NamedDecl>("decl");
+
+  // Ignore decls with internal linkage. Hidden visibility can still lead to ODR
+  // violations but cannot cross DSO boundaries, so it's often used to separate
+  // namespaces, ignore those too.
+  LinkageInfo LV = MatchedDecl->getLinkageAndVisibility();
+  if (!isExternallyVisible(LV.getLinkage()) ||
+      LV.getVisibility() == HiddenVisibility)
+    return;
+
+  if (const auto *VDecl = dyn_cast<VarDecl>(MatchedDecl)) {
+    // extern "C" globals need to be in the global namespace.
+    if (VDecl->isExternC())
+      return;
+  }
+
+  if (const auto *RDecl = dyn_cast<CXXRecordDecl>(MatchedDecl)) {
+    // Ignore C-like records. Those are often used in C wrappers and don't
+    // contain anything with linkage.
+    if (RDecl->isCLike())
+      return;
+  }
+
+  if (const auto *FDecl = dyn_cast<FunctionDecl>(MatchedDecl)) {
+    // main() should be in the global namespace.
+    if (FDecl->isMain())
+      return;
+
+    // Ignore overloaded operators.
+    // FIXME: Should this only ignore global operator new/delete?
+    if (FDecl->isOverloadedOperator())
+      return;
+  }
+
+  // FIXME: If we want to be really fancy we could move declaration-less
+  // definitions into a global namespace or make them static with a FixIt.
+  diag(MatchedDecl->getLocation(), "definition for %0 in the global namespace; "
+                                   "move it into a namespace or give it "
+                                   "internal linkage to avoid ODR conflicts")
+      << MatchedDecl;
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tidy/misc/CMakeLists.txt
+++ clang-tidy/misc/CMakeLists.txt
@@ -3,6 +3,7 @@
 add_clang_library(clangTidyMiscModule
   ArgumentCommentCheck.cpp
   AssertSideEffectCheck.cpp
+  GlobalNamespaceCheck.cpp
   MisplacedConstCheck.cpp
   UnconventionalAssignOperatorCheck.cpp
   BoolPointerImplicitConversionCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to