[clang-tools-extra] r257178 - [clang-tidy] Add non-inline function definition and variable definition check in header files.
Alexander Kornienko via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 8 08:37:12 PST 2016
Author: alexfh
Date: Fri Jan 8 10:37:11 2016
New Revision: 257178
URL: http://llvm.org/viewvc/llvm-project?rev=257178&view=rev
Log:
[clang-tidy] Add non-inline function definition and variable definition check in header files.
Summary: The new check will find all functionand variable definitions which may violate cpp one definition rule in header file.
Reviewers: aaron.ballman, alexfh
Subscribers: aaron.ballman, cfe-commits
Patch by Haojian Wu!
Differential Revision: http://reviews.llvm.org/D15710
Added:
clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
Modified:
clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
clang-tools-extra/trunk/test/lit.cfg
Modified: clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt?rev=257178&r1=257177&r2=257178&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/CMakeLists.txt Fri Jan 8 10:37:11 2016
@@ -5,6 +5,7 @@ add_clang_library(clangTidyMiscModule
AssertSideEffectCheck.cpp
AssignOperatorSignatureCheck.cpp
BoolPointerImplicitConversionCheck.cpp
+ DefinitionsInHeadersCheck.cpp
InaccurateEraseCheck.cpp
InefficientAlgorithmCheck.cpp
MacroParenthesesCheck.cpp
Added: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp?rev=257178&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.cpp Fri Jan 8 10:37:11 2016
@@ -0,0 +1,126 @@
+//===--- DefinitionsInHeadersCheck.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 "DefinitionsInHeadersCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+namespace {
+
+AST_MATCHER(NamedDecl, isHeaderFileExtension) {
+ SourceManager& SM = Finder->getASTContext().getSourceManager();
+ SourceLocation ExpansionLoc = SM.getExpansionLoc(Node.getLocStart());
+ StringRef Filename = SM.getFilename(ExpansionLoc);
+ return Filename.endswith(".h") || Filename.endswith(".hh") ||
+ Filename.endswith(".hpp") || Filename.endswith(".hxx") ||
+ llvm::sys::path::extension(Filename).empty();
+}
+
+} // namespace
+
+DefinitionsInHeadersCheck::DefinitionsInHeadersCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ UseHeaderFileExtension(Options.get("UseHeaderFileExtension", true)) {}
+
+void DefinitionsInHeadersCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "UseHeaderFileExtension", UseHeaderFileExtension);
+}
+
+void DefinitionsInHeadersCheck::registerMatchers(MatchFinder *Finder) {
+ if (UseHeaderFileExtension) {
+ Finder->addMatcher(
+ namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())),
+ isHeaderFileExtension()).bind("name-decl"),
+ this);
+ } else {
+ Finder->addMatcher(
+ namedDecl(anyOf(functionDecl(isDefinition()), varDecl(isDefinition())),
+ anyOf(isHeaderFileExtension(),
+ unless(isExpansionInMainFile()))).bind("name-decl"),
+ this);
+ }
+}
+
+void DefinitionsInHeadersCheck::check(const MatchFinder::MatchResult &Result) {
+ // C++ [basic.def.odr] p6:
+ // There can be more than one definition of a class type, enumeration type,
+ // inline function with external linkage, class template, non-static function
+ // template, static data member of a class template, member function of a
+ // class template, or template specialization for which some template
+ // parameters are not specifiedin a program provided that each definition
+ // appears in a different translation unit, and provided the definitions
+ // satisfy the following requirements.
+ const auto *ND = Result.Nodes.getNodeAs<NamedDecl>("name-decl");
+ assert(ND);
+
+ // Internal linkage variable definitions are ignored for now:
+ // const int a = 1;
+ // static int b = 1;
+ //
+ // Although these might also cause ODR violations, we can be less certain and
+ // should try to keep the false-positive rate down.
+ if (ND->getLinkageInternal() == InternalLinkage)
+ return;
+
+ if (const auto *FD = dyn_cast<FunctionDecl>(ND)) {
+ // Inline functions are allowed.
+ if (FD->isInlined())
+ return;
+ // Function templates are allowed.
+ if (FD->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate)
+ return;
+ // Function template full specialization is prohibited in header file.
+ if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+ return;
+ // Member function of a class template and member function of a nested class
+ // in a class template are allowed.
+ if (const auto *MD = dyn_cast<CXXMethodDecl>(FD)) {
+ const auto *DC = MD->getDeclContext();
+ while (DC->isRecord()) {
+ if (const auto *RD = dyn_cast<CXXRecordDecl>(DC))
+ if (RD->getDescribedClassTemplate())
+ return;
+ DC = DC->getParent();
+ }
+ }
+
+ diag(FD->getLocation(),
+ "function '%0' defined in a header file; "
+ "function definitions in header files can lead to ODR violations")
+ << FD->getNameInfo().getName().getAsString()
+ << FixItHint::CreateInsertion(FD->getSourceRange().getBegin(),
+ "inline ");
+ } else if (const auto *VD = dyn_cast<VarDecl>(ND)) {
+ // Static data members of a class template are allowed.
+ if (VD->getDeclContext()->isDependentContext() && VD->isStaticDataMember())
+ return;
+ if (VD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
+ return;
+ // Ignore variable definition within function scope.
+ if (VD->hasLocalStorage() || VD->isStaticLocal())
+ return;
+
+ diag(VD->getLocation(),
+ "variable '%0' defined in a header file; "
+ "variable definitions in header files can lead to ODR violations")
+ << VD->getName();
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.h?rev=257178&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/misc/DefinitionsInHeadersCheck.h Fri Jan 8 10:37:11 2016
@@ -0,0 +1,43 @@
+//===--- DefinitionsInHeadersCheck.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_DEFINITIONS_IN_HEADERS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEFINITIONS_IN_HEADERS_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+// Finds non-extern non-inline function and variable definitions in header
+// files, which can lead to potential ODR violations.
+//
+// There is one option:
+// - `UseHeaderFileExtension`: Whether to use file extension (h, hh, hpp, hxx)
+// to distinguish header files. True by default.
+//
+// For the user-facing documentation see:
+// http://clang.llvm.org/extra/clang-tidy/checks/misc-definitions-in-headers.html
+class DefinitionsInHeadersCheck : public ClangTidyCheck {
+public:
+ DefinitionsInHeadersCheck(StringRef Name, ClangTidyContext *Context);
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+ const bool UseHeaderFileExtension;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DEFINITIONS_IN_HEADERS_H
Modified: clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp?rev=257178&r1=257177&r2=257178&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/misc/MiscTidyModule.cpp Fri Jan 8 10:37:11 2016
@@ -14,6 +14,7 @@
#include "AssertSideEffectCheck.h"
#include "AssignOperatorSignatureCheck.h"
#include "BoolPointerImplicitConversionCheck.h"
+#include "DefinitionsInHeadersCheck.h"
#include "InaccurateEraseCheck.h"
#include "InefficientAlgorithmCheck.h"
#include "MacroParenthesesCheck.h"
@@ -48,6 +49,8 @@ public:
"misc-assign-operator-signature");
CheckFactories.registerCheck<BoolPointerImplicitConversionCheck>(
"misc-bool-pointer-implicit-conversion");
+ CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
+ "misc-definitions-in-headers");
CheckFactories.registerCheck<InaccurateEraseCheck>(
"misc-inaccurate-erase");
CheckFactories.registerCheck<InefficientAlgorithmCheck>(
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=257178&r1=257177&r2=257178&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Fri Jan 8 10:37:11 2016
@@ -40,6 +40,7 @@ Clang-Tidy Checks
misc-assert-side-effect
misc-assign-operator-signature
misc-bool-pointer-implicit-conversion
+ misc-definitions-in-headers
misc-inaccurate-erase
misc-inefficient-algorithm
misc-macro-parentheses
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst?rev=257178&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/misc-definitions-in-headers.rst Fri Jan 8 10:37:11 2016
@@ -0,0 +1,37 @@
+misc-definitions-in-headers
+===========================
+
+Finds non-extern non-inline function and variable definitions in header files, which can lead to potential ODR violations.
+
+.. code:: c++
+ // Foo.h
+ int a = 1; // Warning.
+ extern int d; // OK: extern variable.
+
+ namespace N {
+ int e = 2; // Warning.
+ }
+
+ // Internal linkage variable definitions are ignored for now.
+ // Although these might also cause ODR violations, we can be less certain and
+ // should try to keep the false-positive rate down.
+ static int b = 1;
+ const int c = 1;
+
+ // Warning.
+ int g() {
+ return 1;
+ }
+
+ // OK: inline function definition.
+ inline int e() {
+ return 1;
+ }
+
+ class A {
+ public:
+ int f1() { return 1; } // OK: inline member function definition.
+ int f2();
+ };
+
+ int A::f2() { return 1; } // Warning.
Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py?rev=257178&r1=257177&r2=257178&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py (original)
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py Fri Jan 8 10:37:11 2016
@@ -52,6 +52,8 @@ def main():
extension = '.cpp'
if (input_file_name.endswith('.c')):
extension = '.c'
+ if (input_file_name.endswith('.hpp')):
+ extension = '.hpp'
temp_file_name = temp_file_name + extension
clang_tidy_extra_args = extra_args
Added: clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp?rev=257178&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp (added)
+++ clang-tools-extra/trunk/test/clang-tidy/misc-definitions-in-headers.hpp Fri Jan 8 10:37:11 2016
@@ -0,0 +1,135 @@
+// RUN: %check_clang_tidy %s misc-definitions-in-headers %t
+
+int f() {
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: function 'f' defined in a header file; function definitions in header files can lead to ODR violations [misc-definitions-in-headers]
+// CHECK-FIXES: inline int f() {
+ return 1;
+}
+
+class CA {
+ void f1() {} // OK: inline class member function definition.
+ void f2();
+ template<typename T>
+ T f3() {
+ T a = 1;
+ return a;
+ }
+ template<typename T>
+ struct CAA {
+ struct CAB {
+ void f4();
+ };
+ };
+};
+
+void CA::f2() { }
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: function 'f2' defined in a header file;
+// CHECK-FIXES: inline void CA::f2() {
+
+template <>
+int CA::f3() {
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: function 'f3' defined in a header file;
+ int a = 1;
+ return a;
+}
+
+template <typename T>
+void CA::CAA<T>::CAB::f4() {
+// OK: member function definition of a nested template class in a class.
+}
+
+template <typename T>
+struct CB {
+ void f1();
+ struct CCA {
+ void f2(T a);
+ };
+ struct CCB; // OK: forward declaration.
+ static int a; // OK: class static data member declaration.
+};
+
+template <typename T>
+void CB<T>::f1() { // OK: Member function definition of a class template.
+}
+
+template <typename T>
+void CB<T>::CCA::f2(T a) {
+// OK: member function definition of a nested class in a class template.
+}
+
+template <typename T>
+struct CB<T>::CCB {
+ void f3();
+};
+
+template <typename T>
+void CB<T>::CCB::f3() {
+// OK: member function definition of a nested class in a class template.
+}
+
+template <typename T>
+int CB<T>::a = 2; // OK: static data member definition of a class template.
+
+template <typename T>
+T tf() { // OK: template function definition.
+ T a;
+ return a;
+}
+
+
+namespace NA {
+ int f() { return 1; }
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: function 'f' defined in a header file;
+// CHECK-FIXES: inline int f() { return 1; }
+}
+
+template <typename T>
+T f3() {
+ T a = 1;
+ return a;
+}
+
+template <>
+// CHECK-MESSAGES: :[[@LINE+1]]:5: warning: function 'f3' defined in a header file;
+int f3() {
+ int a = 1;
+ return a;
+}
+
+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 a = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: variable 'a' defined in a header file; variable definitions in header files can lead to ODR violations [misc-definitions-in-headers]
+CA a1;
+// CHECK-MESSAGES: :[[@LINE-1]]:4: warning: variable 'a1' defined in a header file;
+
+namespace NB {
+ int b = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'b' defined in a header file;
+ const int c = 1; // OK: internal linkage variable definition.
+}
+
+class CC {
+ static int d; // OK: class static data member declaration.
+};
+
+int CC::d = 1;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: variable 'd' defined in a header file;
+
+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;
+}
+
+const char* const g = "foo"; // OK: internal linkage variable definition.
+static int h = 1; // OK: internal linkage variable definition.
+const int i = 1; // OK: internal linkage variable definition.
+extern int j; // OK: internal linkage variable definition.
Modified: clang-tools-extra/trunk/test/lit.cfg
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/lit.cfg?rev=257178&r1=257177&r2=257178&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/lit.cfg (original)
+++ clang-tools-extra/trunk/test/lit.cfg Fri Jan 8 10:37:11 2016
@@ -43,7 +43,8 @@ else:
config.test_format = lit.formats.ShTest(execute_external)
# suffixes: A list of file extensions to treat as test files.
-config.suffixes = ['.c', '.cpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s', '.modularize', '.module-map-checker']
+config.suffixes = ['.c', '.cpp', '.hpp', '.m', '.mm', '.cu', '.ll', '.cl', '.s',
+ '.modularize', '.module-map-checker']
# Test-time dependencies located in directories called 'Inputs' are excluded
# from test suites; there won't be any lit tests within them.
More information about the cfe-commits
mailing list