njames93 updated this revision to Diff 240372.
njames93 added a comment.
- fix formatting
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73413/new/
https://reviews.llvm.org/D73413
Files:
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration.cpp
@@ -0,0 +1,72 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+// <my_file.cpp> -> <my_file.h>
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+bool DeclInWrongHeader = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInWrongHeader' is defined with external linkage
+void declInWrongHeader() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInWrongHeader' is defined with external linkage
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ ns2::NS = false;
+void /*namespace*/ ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/ Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-missing-header-file-declaration-any-header.cpp
@@ -0,0 +1,77 @@
+// RUN: %check_clang_tidy %s misc-missing-header-file-declaration %t -- \
+// RUN: -config='{CheckOptions: \
+// RUN: [{key: misc-missing-header-file-declaration.CheckAnyHeader, value: 1}]}' \
+// RUN: -- -I%S/Inputs/misc-missing-header-file-declaration
+
+// NOTE in this file everything is actually in the wrong header file, as the
+// corresponding header is called
+// 'misc-missing-header-file-declaration-any-header.cpp.tmp.h'
+
+#include "misc-missing-header-file-declaration.cpp.tmp.h"
+// Include file has to be named *.cpp.tmp.h to account for the script renaming
+// this file to *.cpp.tmp.cpp.
+// Outside this test environment it will work normally
+// <my_file.cpp> -> <my_file.h>
+#include "wrong_header.h"
+
+// These declarations should be ignored by the check as they are in the same
+// file.
+extern bool DeclInSource;
+extern void declInSource();
+
+// These declarations should be ignored by the check as they are in the same
+// file, however there is a corresponding decl in the header that will prevent
+// a failing check.
+extern bool DeclInBoth;
+extern void declInBoth();
+
+// No external linkage so no warning
+static bool StaticOK = false;
+constexpr bool ConstexprOK = false;
+inline void inlineOK() {}
+static void staticOK() {}
+constexpr bool constexprOK() { return true; }
+
+// External linkage but decl in header so no warning
+bool DeclInHeader = false;
+bool DeclInBoth = false;
+void declInHeader() {}
+void declInBoth() {}
+
+//Decls don't appear in corresponding header so issue a warning
+bool DeclInSource = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'DeclInSource' is defined with external linkage
+bool NoDecl = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Variable 'NoDecl' is defined with external linkage
+void declInSource() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'declInSource' is defined with external linkage
+void noDecl() {}
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Function 'noDecl' is defined with external linkage
+
+// Checking any header, this is declared so no warning
+bool DeclInWrongHeader = false;
+void declInWrongHeader() {}
+
+// Decls in an anonymous namespace don't have external linkage, so no warning
+// should be emitted
+namespace {
+bool AnonNS = false;
+void anonNS() {}
+} // namespace
+
+// Ensure in namespace definitions are correctly resolved
+namespace ns1 {
+bool NS = false;
+void nS() {}
+} // namespace ns1
+
+// Ensure out of namespace definitions are correctly resolved
+bool /*namespace*/ ns2::NS = false;
+void /*namespace*/ ns2::nS() {}
+
+// Static class members declared in the header shouldn't be warned on.
+int /*struct*/ Foo::Bar = 0;
+
+// main is special, don't warn for it.
+int main() {
+}
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/wrong_header.h
@@ -0,0 +1,7 @@
+#ifndef WRONG_HEADER
+#define WRONG_HEADER
+
+extern bool DeclInWrongHeader;
+extern void declInWrongHeader();
+
+#endif
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/Inputs/misc-missing-header-file-declaration/misc-missing-header-file-declaration.cpp.tmp.h
@@ -0,0 +1,24 @@
+#ifndef MISC_MISSING_HEADER_FILE_DECLARATON_H
+#define MISC_MISSING_HEADER_FILE_DECLARATON_H
+
+extern bool DeclInHeader;
+extern bool DeclInBoth;
+
+extern void declInHeader();
+extern void declInBoth();
+
+struct Foo {
+ static int Bar;
+};
+
+namespace ns1 {
+extern bool NS;
+extern void nS();
+} // namespace ns1
+
+namespace ns2 {
+extern bool NS;
+extern void nS();
+} // namespace ns2
+
+#endif
\ No newline at end of file
Index: clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-missing-header-file-declaration.rst
@@ -0,0 +1,15 @@
+.. title:: clang-tidy - misc-missing-header-file-declaration
+
+misc-missing-header-file-declaration
+====================================
+
+Flags variable and function definitions with external linkage in source files
+that don't have a declaration in the corresponding header file.
+
+Options
+-------
+
+.. option:: CheckAnyHeader
+
+ If set to `1` the check will look in any header file for a matching
+ declaration.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -188,6 +188,7 @@
`llvm-twine-local <llvm-twine-local.html>`_, "Yes"
`misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes"
`misc-misplaced-const <misc-misplaced-const.html>`_,
+ `misc-missing-header-file-declaration <misc-missing-header-file-declaration.html>`_, "Yes"
`misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
`misc-non-copyable-objects <misc-non-copyable-objects.html>`_,
`misc-non-private-member-variables-in-classes <misc-non-private-member-variables-in-classes.html>`_,
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -88,6 +88,12 @@
Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
``memcmp`` and similar derivatives on non-trivial types.
+- New :doc:`misc-missing-header-file-declaration
+ <clang-tidy/checks/misc-missing-header-file-declaration>` check.
+
+ Flags definitions with external linkage in source files that don't have a
+ declaration in the corresponding header file
+
New aliases
^^^^^^^^^^^
Index: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.h
@@ -0,0 +1,38 @@
+//===--- MissingHeaderFileDeclarationCheck.h - clang-tidy -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Flags definitions with external linkage in source files that don't have a
+/// declaration in the corresponding header file.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-missing-header-file-declaration.html
+class MissingHeaderFileDeclarationCheck : public ClangTidyCheck {
+public:
+ MissingHeaderFileDeclarationCheck(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 CheckAnyHeader;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_MISSINGHEADERFILEDECLARATIONCHECK_H
Index: clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/MissingHeaderFileDeclarationCheck.cpp
@@ -0,0 +1,120 @@
+//===--- MissingHeaderFileDeclarationCheck.cpp - clang-tidy ---------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "MissingHeaderFileDeclarationCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringSwitch.h"
+#include <type_traits>
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+struct DecomposedFileName {
+ StringRef NamePart;
+ StringRef Extension;
+
+ bool isHeader() const {
+ return llvm::StringSwitch<bool>(Extension)
+ .CasesLower("h", "hpp", "hxx", true)
+ .Default(false);
+ }
+};
+
+llvm::Optional<DecomposedFileName> decomposeFileName(StringRef FileName) {
+ // Undecided about using llvm::sys::path
+ StringRef::size_type Pos = FileName.find_last_of("\\/");
+ if (Pos != StringRef::npos) {
+ FileName = FileName.drop_front(Pos + 1);
+ }
+ Pos = FileName.find_last_of('.');
+ if (Pos == StringRef::npos)
+ return None;
+ return DecomposedFileName{FileName.take_front(Pos),
+ FileName.drop_front(Pos + 1)};
+}
+
+template <typename Redecl, typename DiagEmitter>
+static void checkDefinition(const Redecl &Decl, const SourceManager &SM,
+ bool AnyHeader, StringRef PrettTypeName,
+ DiagEmitter Diags) {
+ static_assert(std::is_base_of<Redeclarable<Redecl>, Redecl>::value,
+ "Not a Redeclarable Decl");
+ static_assert(std::is_base_of<NamedDecl, Redecl>::value, "Not a Named Decl");
+ llvm::Optional<DecomposedFileName> File =
+ decomposeFileName(SM.getFilename(Decl.getBeginLoc()));
+ if (!File || File->isHeader())
+ // File being detected as a header would occur if clang-tidy was being
+ // invoked on a header file.
+ // If the file is a header and this gets tripped, probably not a good thing
+ // either, definitions in headers is a receipe for disaster, but that't not
+ // what this specific check is looking for.
+ return;
+ for (const auto *RD : Decl.redecls()) {
+ llvm::Optional<DecomposedFileName> ThisFile =
+ decomposeFileName(SM.getFilename(RD->getBeginLoc()));
+ if (!ThisFile || !ThisFile->isHeader())
+ continue;
+ if (AnyHeader || File->NamePart.equals_lower(ThisFile->NamePart))
+ return; // Found a good candidate for matching decl
+ }
+ Diags(Decl.getLocation(),
+ "%0 %1 is defined with external linkage but has no corresponding "
+ "declaration in %select{the corresponding|any}2 header file")
+ << PrettTypeName << cast<NamedDecl>(&Decl) << static_cast<int>(AnyHeader);
+}
+
+MissingHeaderFileDeclarationCheck::MissingHeaderFileDeclarationCheck(
+ StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ CheckAnyHeader(Options.get("CheckAnyHeader", false)) {}
+
+void MissingHeaderFileDeclarationCheck::storeOptions(
+ ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "CheckAnyHeader", CheckAnyHeader);
+}
+
+void MissingHeaderFileDeclarationCheck::registerMatchers(MatchFinder *Finder) {
+ if (getLangOpts().ObjC)
+ return;
+ Finder->addMatcher(varDecl(hasExternalFormalLinkage(), isDefinition(),
+ isExpansionInMainFile(), unless(isConstexpr()))
+ .bind("VarDecl"),
+ this);
+ Finder->addMatcher(
+ functionDecl(
+ hasExternalFormalLinkage(), isDefinition(), isExpansionInMainFile(),
+ unless(anyOf(isConstexpr(), isInline(), cxxMethodDecl(), isMain())))
+ .bind("FunctionDecl"),
+ this);
+}
+
+void MissingHeaderFileDeclarationCheck::check(
+ const MatchFinder::MatchResult &Result) {
+ auto DiagEmitter = [&](SourceLocation Loc, StringRef Msg) {
+ return diag(Loc, Msg);
+ };
+
+ if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("VarDecl")) {
+ checkDefinition(*VD, *Result.SourceManager, CheckAnyHeader, "Variable",
+ DiagEmitter);
+ }
+ if (const auto *FD = Result.Nodes.getNodeAs<FunctionDecl>("FunctionDecl")) {
+ checkDefinition(*FD, *Result.SourceManager, CheckAnyHeader, "Function",
+ DiagEmitter);
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -11,6 +11,7 @@
#include "../ClangTidyModuleRegistry.h"
#include "DefinitionsInHeadersCheck.h"
#include "MisplacedConstCheck.h"
+#include "MissingHeaderFileDeclarationCheck.h"
#include "NewDeleteOverloadsCheck.h"
#include "NonCopyableObjects.h"
#include "NonPrivateMemberVariablesInClassesCheck.h"
@@ -33,6 +34,8 @@
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
"misc-definitions-in-headers");
CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
+ CheckFactories.registerCheck<MissingHeaderFileDeclarationCheck>(
+ "misc-missing-header-file-declaration");
CheckFactories.registerCheck<NewDeleteOverloadsCheck>(
"misc-new-delete-overloads");
CheckFactories.registerCheck<NonCopyableObjectsCheck>(
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -4,6 +4,7 @@
DefinitionsInHeadersCheck.cpp
MiscTidyModule.cpp
MisplacedConstCheck.cpp
+ MissingHeaderFileDeclarationCheck.cpp
NewDeleteOverloadsCheck.cpp
NonCopyableObjects.cpp
NonPrivateMemberVariablesInClassesCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
