VitaNuo created this revision.
Herald added subscribers: PiotrZSL, ChuanqiXu, kadircet, carlosgalvezp,
xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
VitaNuo requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D148793
Files:
clang-tools-extra/clang-tidy/CMakeLists.txt
clang-tools-extra/clang-tidy/google/CMakeLists.txt
clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.cpp
clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/google/include-cleaner.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
clang-tools-extra/include-cleaner/lib/Record.cpp
clang-tools-extra/test/clang-tidy/checkers/google/Inputs/bar.h
clang-tools-extra/test/clang-tidy/checkers/google/Inputs/baz.h
clang-tools-extra/test/clang-tidy/checkers/google/Inputs/foo.h
clang-tools-extra/test/clang-tidy/checkers/google/include-cleaner.cpp
clang-tools-extra/test/clang-tidy/checkers/google/system/string.h
clang-tools-extra/test/clang-tidy/checkers/google/system/vector.h
Index: clang-tools-extra/test/clang-tidy/checkers/google/system/vector.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google/system/vector.h
@@ -0,0 +1,4 @@
+#pragma once
+#include <string.h>
+
+namespace std { class vector {}; }
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/google/system/string.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google/system/string.h
@@ -0,0 +1,2 @@
+#pragma once
+namespace std { class string {}; }
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/google/include-cleaner.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google/include-cleaner.cpp
@@ -0,0 +1,15 @@
+// RUN: %check_clang_tidy %s google-include-cleaner %t -- -- -I%S/Inputs -isystem%S/system
+#include "bar.h"
+// CHECK-FIXES: {{^}}#include "baz.h"{{$}}
+#include "foo.h"
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Unused include "foo.h" [google-include-cleaner]
+// CHECK-FIXES: {{^}}
+// CHECK-FIXES: {{^}}#include <string>{{$}}
+#include <vector.h>
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: Unused include <vector.h> [google-include-cleaner]
+// CHECK-FIXES: {{^}}
+int BarResult = bar();
+int BazResult = baz();
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: Missing include "baz.h" [google-include-cleaner]
+std::string HelloString;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: Missing include <string> [google-include-cleaner]
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/google/Inputs/foo.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google/Inputs/foo.h
@@ -0,0 +1,2 @@
+#pragma once
+void foo();
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/google/Inputs/baz.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google/Inputs/baz.h
@@ -0,0 +1,2 @@
+#pragma once
+int baz();
\ No newline at end of file
Index: clang-tools-extra/test/clang-tidy/checkers/google/Inputs/bar.h
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google/Inputs/bar.h
@@ -0,0 +1,3 @@
+#pragma once
+#include "baz.h"
+int bar();
\ No newline at end of file
Index: clang-tools-extra/include-cleaner/lib/Record.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Record.cpp
+++ clang-tools-extra/include-cleaner/lib/Record.cpp
@@ -19,6 +19,8 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
+#include <memory>
+#include <utility>
namespace clang::include_cleaner {
namespace {
@@ -151,6 +153,10 @@
: SM(CI.getSourceManager()),
HeaderInfo(CI.getPreprocessor().getHeaderSearchInfo()), Out(Out),
UniqueStrings(Arena) {}
+ RecordPragma(const SourceManager &SM, const Preprocessor &P, PragmaIncludes *Out)
+ : SM(SM),
+ HeaderInfo(P.getHeaderSearchInfo()), Out(Out),
+ UniqueStrings(Arena) {}
void FileChanged(SourceLocation Loc, FileChangeReason Reason,
SrcMgr::CharacteristicKind FileType,
@@ -342,6 +348,12 @@
CI.getPreprocessor().addPPCallbacks(std::move(Record));
}
+void PragmaIncludes::record(const SourceManager &SM, Preprocessor &P) {
+ auto Record = std::make_unique<RecordPragma>(SM, P, this);
+ P.addCommentHandler(Record.get());
+ P.addPPCallbacks(std::move(Record));
+}
+
llvm::StringRef PragmaIncludes::getPublic(const FileEntry *F) const {
auto It = IWYUPublic.find(F->getUniqueID());
if (It == IWYUPublic.end())
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Record.h
@@ -18,6 +18,7 @@
#define CLANG_INCLUDE_CLEANER_RECORD_H
#include "clang-include-cleaner/Types.h"
+#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/SmallVector.h"
@@ -52,6 +53,10 @@
/// to the structure.
void record(const CompilerInstance &CI);
+ /// Installs an analysing PPCallback and CommentHandler and populates results
+ /// to the structure.
+ void record(const SourceManager &SM, Preprocessor &P);
+
/// Returns true if the given #include of the main-file should never be
/// removed.
bool shouldKeep(unsigned HashLineNumber) const {
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
@@ -223,6 +223,7 @@
`google-default-arguments <google/default-arguments.html>`_,
`google-explicit-constructor <google/explicit-constructor.html>`_, "Yes"
`google-global-names-in-headers <google/global-names-in-headers.html>`_,
+ `google-include-cleaner <google/include-cleaner.html>`_, "Yes"
`google-objc-avoid-nsobject-new <google/objc-avoid-nsobject-new.html>`_,
`google-objc-avoid-throwing-exception <google/objc-avoid-throwing-exception.html>`_,
`google-objc-function-naming <google/objc-function-naming.html>`_,
@@ -477,7 +478,7 @@
`cppcoreguidelines-explicit-virtual-functions <cppcoreguidelines/explicit-virtual-functions.html>`_, `modernize-use-override <modernize/use-override.html>`_, "Yes"
`cppcoreguidelines-macro-to-enum <cppcoreguidelines/macro-to-enum.html>`_, `modernize-macro-to-enum <modernize/macro-to-enum.html>`_, "Yes"
`cppcoreguidelines-non-private-member-variables-in-classes <cppcoreguidelines/non-private-member-variables-in-classes.html>`_, `misc-non-private-member-variables-in-classes <misc/non-private-member-variables-in-classes.html>`_,
- `cppcoreguidelines-use-default-member-init <cppcoreguidelines/use-default-member-init.html>`_, `modernize-use-default-member-init <modernize/use-default-member-init.html>`_,
+ `cppcoreguidelines-use-default-member-init <cppcoreguidelines/use-default-member-init.html>`_, `modernize-use-default-member-init <modernize/use-default-member-init.html>`_, "Yes"
`fuchsia-header-anon-namespaces <fuchsia/header-anon-namespaces.html>`_, `google-build-namespaces <google/build-namespaces.html>`_,
`google-readability-braces-around-statements <google/readability-braces-around-statements.html>`_, `readability-braces-around-statements <readability/braces-around-statements.html>`_, "Yes"
`google-readability-function-size <google/readability-function-size.html>`_, `readability-function-size <readability/function-size.html>`_,
Index: clang-tools-extra/docs/clang-tidy/checks/google/include-cleaner.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/google/include-cleaner.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - google-include-cleaner
+
+google-include-cleaner
+======================
+
+FIXME: Describe what patterns does the check detect and why. Give examples.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -138,6 +138,11 @@
Warns when an rvalue reference function parameter is never moved within
the function body.
+- New :doc:`google-include-cleaner
+ <clang-tidy/checks/google/include-cleaner>` check.
+
+ FIXME: add release notes.
+
- New :doc:`llvmlibc-inline-function-decl
<clang-tidy/checks/llvmlibc/inline-function-decl>` check.
Index: clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.h
@@ -0,0 +1,55 @@
+//===--- IncludeCleanerCheck.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_GOOGLE_INCLUDECLEANERCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_INCLUDECLEANERCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../ClangTidyDiagnosticConsumer.h"
+#include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/Basic/LLVM.h"
+#include "clang/Basic/SourceLocation.h"
+#include "clang/Lex/HeaderSearch.h"
+#include "clang/Tooling/Syntax/Tokens.h"
+#include <string>
+#include <vector>
+
+namespace clang::tidy::google {
+
+struct MissingIncludeInfo {
+ SourceLocation SymRefLocation;
+ std::string MissingHeaderSpelling;
+};
+
+/// FIXME: Write a short description.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/google/include-cleaner.html
+class IncludeCleanerCheck : public ClangTidyCheck {
+public:
+ IncludeCleanerCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
+ void onEndOfTranslationUnit() override;
+
+private:
+ include_cleaner::RecordedPP RecordedPreprocessor;
+ include_cleaner::PragmaIncludes RecordedPI;
+ HeaderSearch *HS;
+ Decl *TUDecl;
+ const SourceManager *SM;
+};
+
+} // namespace clang::tidy::google
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_GOOGLE_INCLUDECLEANERCHECK_H
Index: clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/google/IncludeCleanerCheck.cpp
@@ -0,0 +1,175 @@
+//===--- IncludeCleanerCheck.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 "IncludeCleanerCheck.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/FileEntry.h"
+#include "clang/Format/Format.h"
+#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Inclusions/HeaderIncludes.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/StringRef.h"
+#include <optional>
+#include <string>
+#include <vector>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::google {
+
+void IncludeCleanerCheck::registerMatchers(MatchFinder *Finder) {
+ if (!getLangOpts().CPlusPlus)
+ return;
+ Finder->addMatcher(translationUnitDecl().bind("top"), this);
+}
+
+void IncludeCleanerCheck::registerPPCallbacks(const SourceManager &SM,
+ Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) {
+ // llvm::errs() << "Registering PP callbacks!!\n";
+ PP->addPPCallbacks(RecordedPreprocessor.record(*PP));
+ HS = &PP->getHeaderSearchInfo();
+ RecordedPI.record(SM, *PP);
+}
+
+void IncludeCleanerCheck::check(const MatchFinder::MatchResult &Result) {
+ // llvm::errs() << "Running the check!!\n";
+ TUDecl = const_cast<Decl *>(Result.Nodes.getNodeAs<Decl>("top"));
+ SM = Result.SourceManager;
+
+ // llvm::errs() << "Finished!!\n";
+}
+
+// TODO(bakalova) Check/fix pragmas
+// TODO(bakalova) Write unit tests
+// TODO(bakalova) Refactor/cleanup
+// TODO(bakalova) Write docs
+void IncludeCleanerCheck::onEndOfTranslationUnit() {
+ const FileEntry *MainFile = SM->getFileEntryForID(SM->getMainFileID());
+ llvm::DenseSet<const include_cleaner::Include *> Used;
+ std::vector<MissingIncludeInfo> Missing;
+ // llvm::errs() << "Let's run the analysis!!\n";
+ walkUsed(TUDecl, RecordedPreprocessor.MacroReferences, &RecordedPI, *SM,
+ [&](const include_cleaner::SymbolReference &Ref,
+ llvm::ArrayRef<include_cleaner::Header> Providers) {
+ // std::string SymName;
+ // switch (Ref.Target.kind()) {
+ // case include_cleaner::Symbol::Declaration:
+ // SymName =
+ // llvm::dyn_cast<NamedDecl>(&Ref.Target.declaration())
+ // ->getQualifiedNameAsString();
+ // llvm::errs() << SymName << "\n";
+ // break;
+ // }
+ bool Satisfied = false;
+ for (const include_cleaner::Header &H : Providers) {
+ // llvm::errs() << "Checking provider: " <<
+ // include_cleaner::spellHeader(H,
+ // *HS, MainFile)
+ // << "\n";
+ if (H.kind() == include_cleaner::Header::Physical &&
+ H.physical() == MainFile)
+ Satisfied = true;
+ // llvm::errs() << SymName << " satisfied because provided by
+ // main file\n";
+ for (const include_cleaner::Include *I :
+ RecordedPreprocessor.Includes.match(H)) {
+ Used.insert(I);
+ Satisfied = true;
+ // llvm::errs() << SymName << " satisfied because provider
+ // matched\n";
+ }
+ }
+ if (!Satisfied && !Providers.empty() &&
+ Ref.RT == include_cleaner::RefType::Explicit)
+ // llvm::errs() << "missing include in walkUsed: "
+ // <<
+ // include_cleaner::spellHeader(Providers.front(),
+ // *HS, MainFile)
+ // << "\n";
+ Missing.push_back(
+ {Ref.RefLocation, include_cleaner::spellHeader(
+ Providers.front(), *HS, MainFile)});
+ });
+
+ std::vector<const include_cleaner::Include *> Unused;
+ for (const include_cleaner::Include &I :
+ RecordedPreprocessor.Includes.all()) {
+ // llvm::errs() << "recorded include: " << I.quote() << "\n";
+ if (Used.contains(&I) || !I.Resolved)
+ continue;
+ if (RecordedPI.shouldKeep(I.Line))
+ continue;
+ // Check if main file is the public interface for a private header. If so
+ // we shouldn't diagnose it as unused.
+ if (auto PHeader = RecordedPI.getPublic(I.Resolved); !PHeader.empty()) {
+ PHeader = PHeader.trim("<>\"");
+ // Since most private -> public mappings happen in a verbatim way, we
+ // check textually here. This might go wrong in presence of symlinks or
+ // header mappings. But that's not different than rest of the places.
+ if (MainFile->tryGetRealPathName().endswith(PHeader))
+ continue;
+ }
+
+ Unused.push_back(&I);
+ }
+
+ // TODO(bakalova) Check pragmas
+ for (const auto *Inc : Unused) {
+ // llvm::errs() << "unused include in analysis: " << Inc->quote() << "\n";
+ std::string Description("Unused include ");
+ Description += Inc->quote();
+ diag(Inc->HashLocation, Description)
+ << FixItHint::CreateRemoval(Inc->HashLocation);
+ }
+
+ // TODO(bakalova) Move to separate function
+ auto FileStyle = format::getStyle(
+ format::DefaultFormatStyle, MainFile->tryGetRealPathName(),
+ format::DefaultFallbackStyle, SM->getBufferData(SM->getMainFileID()),
+ &SM->getFileManager().getVirtualFileSystem());
+ if (!FileStyle) {
+ FileStyle = format::getLLVMStyle();
+ }
+ for (const auto &Inc : Missing) {
+ // llvm::errs() << "missing include in analysis: " <<
+ // Inc.MissingHeaderSpelling
+ // << "\n";
+ std::string Description("Missing include ");
+ Description += Inc.MissingHeaderSpelling;
+
+ // TODO(bakalova) Move to separate function
+ tooling::HeaderIncludes HeaderIncludes(
+ MainFile->tryGetRealPathName(), SM->getBufferData(SM->getMainFileID()),
+ FileStyle->IncludeStyle);
+ bool Angled = llvm::StringRef{Inc.MissingHeaderSpelling}.starts_with("<");
+ // We might suggest insertion of an existing include in edge cases, e.g.,
+ // include is present in a PP-disabled region, or spelling of the header
+ // turns out to be the same as one of the unresolved includes in the
+ // main file.
+ std::optional<tooling::Replacement> Replacement = HeaderIncludes.insert(
+ llvm::StringRef{Inc.MissingHeaderSpelling}.trim("\"<>"), Angled,
+ tooling::IncludeDirective::Include);
+ if (!Replacement.has_value())
+ continue;
+ diag(Inc.SymRefLocation, Description) << FixItHint::CreateInsertion(
+ SM->getComposedLoc(SM->getMainFileID(), Replacement->getOffset()),
+ "#include " + Inc.MissingHeaderSpelling + "\n");
+ }
+}
+
+} // namespace clang::tidy::google
Index: clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
+++ clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp
@@ -22,6 +22,7 @@
#include "FunctionNamingCheck.h"
#include "GlobalNamesInHeadersCheck.h"
#include "GlobalVariableDeclarationCheck.h"
+#include "IncludeCleanerCheck.h"
#include "IntegerTypesCheck.h"
#include "OverloadedUnaryAndCheck.h"
#include "TodoCommentCheck.h"
@@ -49,6 +50,7 @@
"google-explicit-constructor");
CheckFactories.registerCheck<readability::GlobalNamesInHeadersCheck>(
"google-global-names-in-headers");
+ CheckFactories.registerCheck<IncludeCleanerCheck>("google-include-cleaner");
CheckFactories.registerCheck<objc::AvoidNSObjectNewCheck>(
"google-objc-avoid-nsobject-new");
CheckFactories.registerCheck<objc::AvoidThrowingObjCExceptionCheck>(
Index: clang-tools-extra/clang-tidy/google/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/google/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/google/CMakeLists.txt
@@ -15,6 +15,7 @@
GlobalNamesInHeadersCheck.cpp
GlobalVariableDeclarationCheck.cpp
GoogleTidyModule.cpp
+ IncludeCleanerCheck.cpp
IntegerTypesCheck.cpp
OverloadedUnaryAndCheck.cpp
TodoCommentCheck.cpp
Index: clang-tools-extra/clang-tidy/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/CMakeLists.txt
@@ -7,6 +7,7 @@
${CMAKE_CURRENT_SOURCE_DIR}/clang-tidy-config.h.cmake
${CMAKE_CURRENT_BINARY_DIR}/clang-tidy-config.h)
include_directories(BEFORE ${CMAKE_CURRENT_BINARY_DIR})
+include_directories(BEFORE "${CMAKE_CURRENT_SOURCE_DIR}/../include-cleaner/include")
add_clang_library(clangTidy
ClangTidy.cpp
@@ -38,6 +39,7 @@
clangSerialization
clangTooling
clangToolingCore
+ clangIncludeCleaner
)
if(CLANG_TIDY_ENABLE_STATIC_ANALYZER)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
