VitaNuo created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
VitaNuo requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D143496
Files:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
Index: clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
===================================================================
--- clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
+++ clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
@@ -73,6 +73,8 @@
std::string fixIncludes(const AnalysisResults &Results, llvm::StringRef Code,
const format::FormatStyle &IncludeStyle);
+static std::string spellHeader(const Header &H, HeaderSearch &HS,
+ const FileEntry *Main);
} // namespace include_cleaner
} // namespace clang
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -385,6 +385,43 @@
Pointee(writtenInclusion("\"dir/unused.h\""))));
}
+TEST(IncludeCleaner, GetMissingHeaders) {
+ llvm::StringLiteral MainFile = R"cpp(
+ #include "a.h"
+ #include "dir/c.h"
+ #include <e.h>
+
+ void foo() {
+ b();
+ d();
+ f();
+ })cpp";
+ // Build expected ast with symbols coming from headers.
+ TestTU TU;
+ TU.Filename = "foo.cpp";
+ TU.AdditionalFiles["foo.h"] = guard("void foo();");
+ TU.AdditionalFiles["a.h"] = guard("#include \"b.h\"");
+ TU.AdditionalFiles["b.h"] = guard("void b();");
+
+ TU.AdditionalFiles["dir/c.h"] = guard("#include \"d.h\"");
+ TU.AdditionalFiles["dir/d.h"] = guard("void d();");
+
+ TU.AdditionalFiles["system/e.h"] = guard("#include <f.h>");
+ TU.AdditionalFiles["system/f.h"] = guard("void f();");
+ TU.ExtraArgs.push_back("-isystem" + testPath("system"));
+
+ TU.Code = MainFile.str();
+ ParsedAST AST = TU.build();
+
+ std::vector<std::pair<std::string, unsigned>> MissingIncludes =
+ computeMissingIncludes(AST);
+
+ std::pair<std::string, unsigned> b{"\"b.h\"", 86};
+ std::pair<std::string, unsigned> d{"\"dir/d.h\"", 97};
+ std::pair<std::string, unsigned> h{"<f.h>", 108};
+ EXPECT_THAT(MissingIncludes, UnorderedElementsAre(b, d, h));
+}
+
TEST(IncludeCleaner, VirtualBuffers) {
TestTU TU;
TU.Code = R"cpp(
@@ -628,6 +665,7 @@
Config Cfg;
Cfg.Diagnostics.UnusedIncludes = Config::Strict;
+ Cfg.Diagnostics.MissingIncludes = Config::MissingIncludesPolicy::Strict;
WithContextValue Ctx(Config::Key, std::move(Cfg));
ParsedAST AST = TU.build();
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -691,9 +691,14 @@
if (Result.Diags) {
auto UnusedHeadersDiags =
issueUnusedIncludesDiagnostics(Result, Inputs.Contents);
+ auto MissingHeadersDiags =
+ issueMissingIncludesDiagnostics(Result, Inputs.Contents);
Result.Diags->insert(Result.Diags->end(),
make_move_iterator(UnusedHeadersDiags.begin()),
make_move_iterator(UnusedHeadersDiags.end()));
+ Result.Diags->insert(Result.Diags->end(),
+ make_move_iterator(MissingHeadersDiags.begin()),
+ make_move_iterator(MissingHeadersDiags.end()));
}
return std::move(Result);
}
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -96,12 +96,16 @@
const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
const llvm::StringSet<> &ReferencedPublicHeaders);
+std::vector<std::pair<std::string, unsigned>>
+computeMissingIncludes(ParsedAST &AST);
std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
// The same as computeUnusedIncludes, but it is an experimental and
// include-cleaner-lib-based implementation.
std::vector<const Inclusion *>
computeUnusedIncludesExperimental(ParsedAST &AST);
+std::vector<Diag> issueMissingIncludesDiagnostics(ParsedAST &AST,
+ llvm::StringRef Code);
std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
llvm::StringRef Code);
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -20,6 +20,7 @@
#include "clang/AST/ASTContext.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/Basic/FileEntry.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/HeaderSearch.h"
@@ -28,12 +29,14 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSet.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
#include <functional>
#include <optional>
+#include <vector>
namespace clang {
namespace clangd {
@@ -85,7 +88,7 @@
// Using templateName case is handled by the override TraverseTemplateName.
if (TST->getTemplateName().getKind() == TemplateName::UsingTemplate)
return true;
- add(TST->getAsCXXRecordDecl()); // Specialization
+ add(TST->getAsCXXRecordDecl()); // Specialization
return true;
}
@@ -460,6 +463,27 @@
return TranslatedHeaderIDs;
}
+std::vector<include_cleaner::SymbolReference>
+collectMacroReferences(ParsedAST &AST) {
+ const auto &SM = AST.getSourceManager();
+ // FIXME: !!this is a hacky way to collect macro references.
+ std::vector<include_cleaner::SymbolReference> Macros;
+ auto &PP = AST.getPreprocessor();
+ for (const syntax::Token &Tok :
+ AST.getTokens().spelledTokens(SM.getMainFileID())) {
+ auto Macro = locateMacroAt(Tok, PP);
+ if (!Macro)
+ continue;
+ if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid())
+ Macros.push_back(
+ {Tok.location(),
+ include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)),
+ DefLoc},
+ include_cleaner::RefType::Explicit});
+ }
+ return Macros;
+}
+
// This is the original clangd-own implementation for computing unused
// #includes. Eventually it will be deprecated and replaced by the
// include-cleaner-lib-based implementation.
@@ -474,56 +498,167 @@
translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM);
return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas);
}
-std::vector<const Inclusion *> computeUnusedIncludesExperimental(ParsedAST &AST) {
- const auto &SM = AST.getSourceManager();
- const auto &Includes = AST.getIncludeStructure();
- // FIXME: this map should probably be in IncludeStructure.
- llvm::StringMap<llvm::SmallVector<IncludeStructure::HeaderID>> BySpelling;
- for (const auto &Inc : Includes.MainFileIncludes) {
+
+// Compute unused #includes using the include-cleaner-lib-based implementation.
+std::vector<const Inclusion *>
+computeUnusedIncludesExperimental(ParsedAST &AST) {
+ const auto &SM = AST.getSourceManager();
+ const auto &Includes = AST.getIncludeStructure();
+ // FIXME: this map should probably be in IncludeStructure.
+ llvm::StringMap<llvm::SmallVector<IncludeStructure::HeaderID>> BySpelling;
+ for (const auto &Inc : Includes.MainFileIncludes) {
if (Inc.HeaderID)
BySpelling.try_emplace(Inc.Written)
.first->second.push_back(
static_cast<IncludeStructure::HeaderID>(*Inc.HeaderID));
- }
- // FIXME: !!this is a hacky way to collect macro references.
- std::vector<include_cleaner::SymbolReference> Macros;
- auto& PP = AST.getPreprocessor();
- for (const syntax::Token &Tok :
- AST.getTokens().spelledTokens(SM.getMainFileID())) {
- auto Macro = locateMacroAt(Tok, PP);
- if (!Macro)
- continue;
- if (auto DefLoc = Macro->Info->getDefinitionLoc(); DefLoc.isValid())
- Macros.push_back(
- {Tok.location(),
- include_cleaner::Macro{/*Name=*/PP.getIdentifierInfo(Tok.text(SM)),
- DefLoc},
- include_cleaner::RefType::Explicit});
- }
- llvm::DenseSet<IncludeStructure::HeaderID> Used;
- include_cleaner::walkUsed(
- AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
- AST.getPragmaIncludes(), SM,
- [&](const include_cleaner::SymbolReference &Ref,
- llvm::ArrayRef<include_cleaner::Header> Providers) {
- for (const auto &H : Providers) {
- switch (H.kind()) {
- case include_cleaner::Header::Physical:
- if (auto HeaderID = Includes.getID(H.physical()))
- Used.insert(*HeaderID);
- break;
- case include_cleaner::Header::Standard:
- for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard()))
- Used.insert(HeaderID);
- break;
- case include_cleaner::Header::Verbatim:
- for (auto HeaderID : BySpelling.lookup(H.verbatim()))
- Used.insert(HeaderID);
- break;
- }
- }
- });
- return getUnused(AST, Used, /*ReferencedPublicHeaders*/{});
+ }
+
+ std::vector<include_cleaner::SymbolReference> Macros =
+ collectMacroReferences(AST);
+
+ llvm::DenseSet<IncludeStructure::HeaderID> Used;
+ include_cleaner::walkUsed(
+ AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
+ AST.getPragmaIncludes(), SM,
+ [&](const include_cleaner::SymbolReference &Ref,
+ llvm::ArrayRef<include_cleaner::Header> Providers) {
+ for (const auto &H : Providers) {
+ switch (H.kind()) {
+ case include_cleaner::Header::Physical:
+ if (auto HeaderID = Includes.getID(H.physical()))
+ Used.insert(*HeaderID);
+ break;
+ case include_cleaner::Header::Standard:
+ for (auto HeaderID : Includes.StdlibHeaders.lookup(H.standard()))
+ Used.insert(HeaderID);
+ break;
+ case include_cleaner::Header::Verbatim:
+ for (auto HeaderID : BySpelling.lookup(H.verbatim()))
+ Used.insert(HeaderID);
+ break;
+ }
+ }
+ });
+ return getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
+}
+
+include_cleaner::Includes
+convertIncludes(const SourceManager &SM,
+ std::vector<Inclusion> MainFileIncludes) {
+ include_cleaner::Includes Includes;
+ for (const Inclusion &Inc : MainFileIncludes) {
+ llvm::ErrorOr<const FileEntry *> ResolvedOrError =
+ SM.getFileManager().getFile(Inc.Resolved);
+ const FileEntry *Resolved = nullptr;
+ if (bool(ResolvedOrError)) {
+ Resolved = ResolvedOrError.get();
+ }
+ SourceLocation HashLocation =
+ SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
+ llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written);
+ bool Angled = WrittenRef.starts_with("<") ? true : false;
+ Includes.add(include_cleaner::Include{WrittenRef.trim("\"<>"), Resolved,
+ HashLocation, (unsigned)Inc.HashLine,
+ Angled});
+ }
+ return Includes;
+}
+
+// Compute missing #includes. Uses the include-cleaner-lib-based implementation.
+std::vector<std::pair<std::string, unsigned>>
+computeMissingIncludes(ParsedAST &AST) {
+ std::vector<include_cleaner::SymbolReference> Macros =
+ collectMacroReferences(AST);
+ std::vector<Inclusion> MainFileIncludes =
+ AST.getIncludeStructure().MainFileIncludes;
+
+ include_cleaner::Includes IncludeCleanerIncludes =
+ convertIncludes(AST.getSourceManager(), MainFileIncludes);
+ std::string FileName =
+ AST.getSourceManager()
+ .getFileEntryRefForID(AST.getSourceManager().getMainFileID())
+ ->getName()
+ .str();
+ if (FileName.find("foo") != std::string::npos) {
+ vlog("Include cleaner includes: {0}", IncludeCleanerIncludes.all().size());
+ }
+
+ std::vector<std::pair<std::string, unsigned>> Missing;
+ SourceManager &SM = AST.getSourceManager();
+ const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
+ include_cleaner::walkUsed(
+ AST.getLocalTopLevelDecls(), Macros, AST.getPragmaIncludes(), SM,
+ [&](const include_cleaner::SymbolReference &Ref,
+ llvm::ArrayRef<include_cleaner::Header> Providers) {
+ bool Satisfied = false;
+ for (const include_cleaner::Header &H : Providers) {
+ if (H.kind() == include_cleaner::Header::Physical &&
+ H.physical() == MainFile) {
+ Satisfied = true;
+ }
+ if (!IncludeCleanerIncludes.match(H).empty()) {
+ Satisfied = true;
+ }
+ }
+ if (!Satisfied && !Providers.empty() &&
+ Ref.RT == include_cleaner::RefType::Explicit) {
+ std::string SpelledHeader = include_cleaner::spellHeader(
+ Providers.front(), AST.getPreprocessor().getHeaderSearchInfo(),
+ MainFile);
+ std::pair<FileID, unsigned int> FileIDAndOffset =
+ AST.getSourceManager().getDecomposedLoc(Ref.RefLocation);
+ Missing.push_back({SpelledHeader, FileIDAndOffset.second});
+ }
+ });
+ return Missing;
+}
+
+std::vector<Diag> issueMissingIncludesDiagnostics(ParsedAST &AST,
+ llvm::StringRef Code) {
+ const Config &Cfg = Config::current();
+ if (Cfg.Diagnostics.MissingIncludes == Config::MissingIncludesPolicy::None ||
+ Cfg.Diagnostics.SuppressAll ||
+ Cfg.Diagnostics.Suppress.contains("missing-includes")) {
+ return {};
+ }
+ // Interaction is only polished for C/CPP.
+ if (AST.getLangOpts().ObjC)
+ return {};
+ trace::Span Tracer("IncludeCleaner::issueMissingIncludesDiagnostics");
+ std::string FileName =
+ AST.getSourceManager()
+ .getFileEntryRefForID(AST.getSourceManager().getMainFileID())
+ ->getName()
+ .str();
+
+ unsigned FixLine = 0;
+ std::vector<Inclusion> MainFileIncludes =
+ AST.getIncludeStructure().MainFileIncludes;
+ if (!MainFileIncludes.empty())
+ FixLine = MainFileIncludes[MainFileIncludes.size() - 1].HashLine + 1;
+
+ std::vector<Diag> Result;
+ const auto &MissingIncludes = computeMissingIncludes(AST);
+ for (const std::pair<std::string, unsigned> &Missing : MissingIncludes) {
+ Diag D;
+ D.Message = llvm::formatv("header {0} is missing", Missing.first);
+ D.Name = "missing-includes";
+ D.Source = Diag::DiagSource::Clangd;
+ D.File = FileName;
+ D.Severity = DiagnosticsEngine::Warning;
+
+ D.Range = getDiagnosticRange(Code, Missing.second);
+ D.Fixes.emplace_back();
+ D.Fixes.back().Message = "add #include directive";
+ D.Fixes.back().Edits.emplace_back();
+ D.Fixes.back().Edits.back().newText = "#include " + Missing.first + "\n";
+ D.Fixes.back().Edits.back().range.start.line = FixLine;
+ D.Fixes.back().Edits.back().range.end.line = FixLine;
+ D.InsideMainFile = true;
+ Result.push_back(std::move(D));
+ }
+
+ return Result;
}
std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -128,6 +128,9 @@
Dict.handle("UnusedIncludes", [&](Node &N) {
F.UnusedIncludes = scalarValue(N, "UnusedIncludes");
});
+ Dict.handle("MissingIncludes", [&](Node &N) {
+ F.MissingIncludes = scalarValue(N, "MissingIncludes");
+ });
Dict.handle("Includes", [&](Node &N) { parse(F.Includes, N); });
Dict.handle("ClangTidy", [&](Node &N) { parse(F.ClangTidy, N); });
Dict.parse(N);
Index: clang-tools-extra/clangd/ConfigFragment.h
===================================================================
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -221,7 +221,7 @@
/// This often has other advantages, such as skipping some analysis.
std::vector<Located<std::string>> Suppress;
- /// Controls how clangd will correct "unnecessary #include directives.
+ /// Controls how clangd will correct "unnecessary" #include directives.
/// clangd can warn if a header is `#include`d but not used, and suggest
/// removing it.
//
@@ -235,6 +235,18 @@
/// - None
std::optional<Located<std::string>> UnusedIncludes;
+ /// Controls how clangd handles missing #include directives.
+ /// clangd can warn if a header for a symbol is not `#include`d (missing),
+ /// and suggest adding it.
+ ///
+ /// Strict means a header is missing if it is not *directly #include'd.
+ /// The file might still compile if the header is included transitively.
+ ///
+ /// Valid values are:
+ /// - Strict
+ /// - None
+ std::optional<Located<std::string>> MissingIncludes;
+
/// Controls IncludeCleaner diagnostics.
struct IncludesBlock {
/// Regexes that will be used to avoid diagnosing certain includes as
Index: clang-tools-extra/clangd/ConfigCompile.cpp
===================================================================
--- clang-tools-extra/clangd/ConfigCompile.cpp
+++ clang-tools-extra/clangd/ConfigCompile.cpp
@@ -441,6 +441,18 @@
Out.Apply.push_back([Val](const Params &, Config &C) {
C.Diagnostics.UnusedIncludes = *Val;
});
+
+ if (F.MissingIncludes)
+ if (auto Val =
+ compileEnum<Config::MissingIncludesPolicy>("MissingIncludes",
+ **F.MissingIncludes)
+ .map("Strict", Config::MissingIncludesPolicy::Strict)
+ .map("None", Config::MissingIncludesPolicy::None)
+ .value())
+ Out.Apply.push_back([Val](const Params &, Config &C) {
+ C.Diagnostics.MissingIncludes = *Val;
+ });
+
compile(std::move(F.Includes));
compile(std::move(F.ClangTidy));
Index: clang-tools-extra/clangd/Config.h
===================================================================
--- clang-tools-extra/clangd/Config.h
+++ clang-tools-extra/clangd/Config.h
@@ -88,6 +88,11 @@
bool StandardLibrary = true;
} Index;
+ enum class MissingIncludesPolicy {
+ /// Diagnose missing includes.
+ Strict,
+ None,
+ };
enum UnusedIncludesPolicy {
/// Diagnose unused includes.
Strict,
@@ -108,6 +113,7 @@
} ClangTidy;
UnusedIncludesPolicy UnusedIncludes = None;
+ MissingIncludesPolicy MissingIncludes = MissingIncludesPolicy::None;
/// IncludeCleaner will not diagnose usages of these headers matched by
/// these regexes.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
