[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 llvm-svn: 257178
This commit is contained in:
parent
c00ad6c5fb
commit
b816ba0fb3
|
@ -5,6 +5,7 @@ add_clang_library(clangTidyMiscModule
|
|||
AssertSideEffectCheck.cpp
|
||||
AssignOperatorSignatureCheck.cpp
|
||||
BoolPointerImplicitConversionCheck.cpp
|
||||
DefinitionsInHeadersCheck.cpp
|
||||
InaccurateEraseCheck.cpp
|
||||
InefficientAlgorithmCheck.cpp
|
||||
MacroParenthesesCheck.cpp
|
||||
|
|
|
@ -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
|
|
@ -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
|
|
@ -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>(
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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.
|
|
@ -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
|
||||
|
|
|
@ -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.
|
|
@ -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.
|
||||
|
|
Loading…
Reference in New Issue