[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 18:26:37 PST 2016
I don't see how -Wmissing-declarations relates to this check. None of the
warnings in the MissingDeclarations group in DiagnosticSemaKinds.td seem to
be anywhere close to what this check does. Am I missing something?
On Fri, Jan 8, 2016 at 7:53 PM, David Blaikie <dblaikie at gmail.com> wrote:
> 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/20160109/d4aac813/attachment-0001.html>
More information about the cfe-commits
mailing list