On Fri, Jan 8, 2016 at 6:26 PM, Alexander Kornienko <ale...@google.com> wrote:
> 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? > Ah, sorry - wrong diagnostic name. Apparently it's missing-prototypes and missing-variable-declarations (close... ) If you put a function or variable definition in a header, chances are you were intending to declare it (or make it inline) & it has no prior declaration, and thus the missing-declaration warning(s) would catch it: func.cpp:1:6: warning: no previous prototype for function 'func' [-Wmissing-prototypes] void func() { ^ > > > 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