bkramer created this revision.
bkramer added a reviewer: alexfh.
bkramer added a subscriber: cfe-commits.
This is prone to ODR violations and generally frowned upon in many
codebases (e.g. LLVM). The checker flags definitions, variables and
classes in the global namespace. Common false positives like extern "C"
functions are filtered, symbols with internal linkage or hidden
visibility are not warned on either.
On LLVM most of the instances are helper functions that should be just
made static or globals that belong into a namespace.
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/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,38 @@
+// 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: 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
