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 <dblai...@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@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@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
