[clang-tools-extra] r257178 - [clang-tidy] Add non-inline function definition and variable definition check in header files.
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 8 10:53:26 PST 2016
This sounds sort of like the missing-declaration warning in Clang, no?
(granted, it's a bit more direct & thus perhaps easier to use, but fulfills
a similar purpose)
On Fri, Jan 8, 2016 at 8:37 AM, Alexander Kornienko via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> 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.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160108/ad683a2b/attachment-0001.html>
More information about the cfe-commits
mailing list