VitaNuo updated this revision to Diff 499553.
VitaNuo marked 24 inline comments as done.
VitaNuo added a comment.
Address review comments (apart from testing).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143496/new/
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/Preamble.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
clang-tools-extra/include-cleaner/include/clang-include-cleaner/Analysis.h
clang-tools-extra/include-cleaner/lib/Analysis.cpp
Index: clang-tools-extra/include-cleaner/lib/Analysis.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/Analysis.cpp
+++ clang-tools-extra/include-cleaner/lib/Analysis.cpp
@@ -49,8 +49,8 @@
}
}
-static std::string spellHeader(const Header &H, HeaderSearch &HS,
- const FileEntry *Main) {
+std::string spellHeader(const Header &H, HeaderSearch &HS,
+ const FileEntry *Main) {
switch (H.kind()) {
case Header::Physical: {
bool IsSystem = false;
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);
+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
@@ -9,11 +9,19 @@
#include "Annotations.h"
#include "Config.h"
#include "IncludeCleaner.h"
+#include "ParsedAST.h"
#include "SourceCode.h"
#include "TestFS.h"
#include "TestTU.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Types.h"
#include "support/Context.h"
+#include "clang/AST/DeclBase.h"
+#include "clang/Basic/SourceManager.h"
+#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ScopeExit.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Testing/Support/SupportHelpers.h"
#include "gmock/gmock.h"
#include "gtest/gtest.h"
@@ -342,7 +350,8 @@
auto AST = TU.build();
EXPECT_THAT(computeUnusedIncludes(AST),
ElementsAre(Pointee(writtenInclusion("<queue>"))));
- EXPECT_THAT(computeUnusedIncludesExperimental(AST),
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+ EXPECT_THAT(Findings.UnusedIncludes,
ElementsAre(Pointee(writtenInclusion("<queue>"))));
}
@@ -379,12 +388,69 @@
computeUnusedIncludes(AST),
UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")),
Pointee(writtenInclusion("\"dir/unused.h\""))));
+
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
EXPECT_THAT(
- computeUnusedIncludesExperimental(AST),
+ Findings.UnusedIncludes,
UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")),
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();
+
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+ const SourceManager &SM = AST.getSourceManager();
+ const llvm::ArrayRef<syntax::Token> &Tokens =
+ AST.getTokens().spelledTokens(SM.getMainFileID());
+ for (const auto &Token : Tokens) {
+ if (Token.text(SM) == "b") {
+ syntax::FileRange BRange{SM, Token.location(), 1};
+ include_cleaner::Header Header{*SM.getFileManager().getFile("b.h")};
+ MissingIncludeDiagInfo BInfo{"b", BRange, {Header}};
+ EXPECT_THAT(Findings.MissingIncludes, testing::Contains(BInfo));
+ }
+ if (Token.text(SM) == "d") {
+ syntax::FileRange DRange{SM, Token.location(), 1};
+ include_cleaner::Header Header{*SM.getFileManager().getFile("dir/d.h")};
+ MissingIncludeDiagInfo DInfo{"d", DRange, {Header}};
+ EXPECT_THAT(Findings.MissingIncludes, testing::Contains(DInfo));
+ }
+ if (Token.text(SM) == "f") {
+ syntax::FileRange FRange{SM, Token.location(), 1};
+ include_cleaner::Header Header{
+ *SM.getFileManager().getFile("system/f.h")};
+ MissingIncludeDiagInfo FInfo{"f", FRange, {Header}};
+ EXPECT_THAT(Findings.MissingIncludes, testing::Contains(FInfo));
+ }
+ }
+}
+
TEST(IncludeCleaner, VirtualBuffers) {
TestTU TU;
TU.Code = R"cpp(
@@ -554,7 +620,8 @@
ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
- EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty());
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+ EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
}
TEST(IncludeCleaner, RecursiveInclusion) {
@@ -583,7 +650,8 @@
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
- EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty());
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+ EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
}
TEST(IncludeCleaner, IWYUPragmaExport) {
@@ -608,7 +676,8 @@
// FIXME: This is not correct: foo.h is unused but is not diagnosed as such
// because we ignore headers with IWYU export pragmas for now.
EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
- EXPECT_THAT(computeUnusedIncludesExperimental(AST), IsEmpty());
+ IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
+ EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
}
TEST(IncludeCleaner, NoDiagsForObjC) {
@@ -627,7 +696,8 @@
TU.ExtraArgs.emplace_back("-xobjective-c");
Config Cfg;
- Cfg.Diagnostics.UnusedIncludes = Config::Strict;
+ Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
+ Cfg.Diagnostics.MissingIncludes = Config::IncludesPolicy::Strict;
WithContextValue Ctx(Config::Key, std::move(Cfg));
ParsedAST AST = TU.build();
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -1897,7 +1897,7 @@
// Off by default.
EXPECT_THAT(*TU.build().getDiagnostics(), IsEmpty());
Config Cfg;
- Cfg.Diagnostics.UnusedIncludes = Config::UnusedIncludesPolicy::Strict;
+ Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
// Set filtering.
Cfg.Diagnostics.Includes.IgnoreHeader.emplace_back(
[](llvm::StringRef Header) { return Header.endswith("ignore.h"); });
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -249,19 +249,19 @@
// Defaults to None.
EXPECT_TRUE(compileAndApply());
EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::UnusedIncludesPolicy::None);
+ Config::IncludesPolicy::None);
Frag = {};
Frag.Diagnostics.UnusedIncludes.emplace("None");
EXPECT_TRUE(compileAndApply());
EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::UnusedIncludesPolicy::None);
+ Config::IncludesPolicy::None);
Frag = {};
Frag.Diagnostics.UnusedIncludes.emplace("Strict");
EXPECT_TRUE(compileAndApply());
EXPECT_EQ(Conf.Diagnostics.UnusedIncludes,
- Config::UnusedIncludesPolicy::Strict);
+ Config::IncludesPolicy::Strict);
Frag = {};
EXPECT_TRUE(Conf.Diagnostics.Includes.IgnoreHeader.empty())
Index: clang-tools-extra/clangd/Preamble.cpp
===================================================================
--- clang-tools-extra/clangd/Preamble.cpp
+++ clang-tools-extra/clangd/Preamble.cpp
@@ -123,7 +123,7 @@
SourceMgr = &CI.getSourceManager();
Includes.collect(CI);
if (Config::current().Diagnostics.UnusedIncludes ==
- Config::UnusedIncludesPolicy::Experiment)
+ Config::IncludesPolicy::Experiment)
Pragmas.record(CI);
if (BeforeExecuteCallback)
BeforeExecuteCallback(CI);
Index: clang-tools-extra/clangd/ParsedAST.cpp
===================================================================
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -688,13 +688,9 @@
std::move(Macros), std::move(Marks), std::move(ParsedDecls),
std::move(Diags), std::move(Includes),
std::move(CanonIncludes));
- if (Result.Diags) {
- auto UnusedHeadersDiags =
- issueUnusedIncludesDiagnostics(Result, Inputs.Contents);
- Result.Diags->insert(Result.Diags->end(),
- make_move_iterator(UnusedHeadersDiags.begin()),
- make_move_iterator(UnusedHeadersDiags.end()));
- }
+ if (Result.Diags)
+ llvm::move(issueIncludeCleanerDiagnostics(Result, Inputs.Contents),
+ std::back_inserter(*Result.Diags));
return std::move(Result);
}
Index: clang-tools-extra/clangd/IncludeCleaner.h
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.h
+++ clang-tools-extra/clangd/IncludeCleaner.h
@@ -20,18 +20,40 @@
#include "Headers.h"
#include "ParsedAST.h"
+#include "clang-include-cleaner/Types.h"
#include "index/CanonicalIncludes.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
+#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringSet.h"
#include <optional>
+#include <string>
+#include <tuple>
#include <vector>
namespace clang {
namespace clangd {
+// Data needed for missing include diagnostics.
+struct MissingIncludeDiagInfo {
+ std::string SymbolName;
+ syntax::FileRange SymRefRange;
+ std::vector<include_cleaner::Header> Providers;
+
+ bool operator==(const MissingIncludeDiagInfo &Other) const {
+ return std::tie(SymRefRange, Providers, SymbolName) ==
+ std::tie(Other.SymRefRange, Other.Providers, Other.SymbolName);
+ }
+};
+
+struct IncludeCleanerFindings {
+ std::vector<const Inclusion *> UnusedIncludes;
+ std::vector<MissingIncludeDiagInfo> MissingIncludes;
+};
+
struct ReferencedLocations {
llvm::DenseSet<SourceLocation> User;
llvm::DenseSet<tooling::stdlib::Symbol> Stdlib;
@@ -96,13 +118,10 @@
const llvm::DenseSet<IncludeStructure::HeaderID> &ReferencedFiles,
const llvm::StringSet<> &ReferencedPublicHeaders);
+IncludeCleanerFindings computeIncludeCleanerFindings(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> issueUnusedIncludesDiagnostics(ParsedAST &AST,
+std::vector<Diag> issueIncludeCleanerDiagnostics(ParsedAST &AST,
llvm::StringRef Code);
/// Affects whether standard library includes should be considered for
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -8,32 +8,51 @@
#include "IncludeCleaner.h"
#include "Config.h"
+#include "Diagnostics.h"
#include "Headers.h"
#include "ParsedAST.h"
#include "Protocol.h"
#include "SourceCode.h"
+#include "URI.h"
#include "clang-include-cleaner/Analysis.h"
#include "clang-include-cleaner/Types.h"
#include "index/CanonicalIncludes.h"
#include "support/Logger.h"
+#include "support/Path.h"
#include "support/Trace.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/Expr.h"
#include "clang/AST/ExprCXX.h"
#include "clang/AST/RecursiveASTVisitor.h"
+#include "clang/AST/TemplateName.h"
+#include "clang/AST/Type.h"
+#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/LLVM.h"
#include "clang/Basic/SourceLocation.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Lex/HeaderSearch.h"
#include "clang/Lex/Preprocessor.h"
+#include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Inclusions/HeaderIncludes.h"
+#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/Casting.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FormatVariadic.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/Regex.h"
-#include <functional>
+#include <iterator>
#include <optional>
+#include <string>
+#include <vector>
namespace clang {
namespace clangd {
@@ -85,7 +104,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;
}
@@ -325,6 +344,160 @@
return ID;
}
+include_cleaner::Includes
+convertIncludes(const SourceManager &SM,
+ const llvm::ArrayRef<Inclusion> MainFileIncludes) {
+ include_cleaner::Includes Includes;
+ for (const Inclusion &Inc : MainFileIncludes) {
+ include_cleaner::Include TransformedInc;
+ llvm::StringRef WrittenRef = llvm::StringRef(Inc.Written);
+ TransformedInc.Spelled = WrittenRef.trim("\"<>");
+ TransformedInc.HashLocation =
+ SM.getComposedLoc(SM.getMainFileID(), Inc.HashOffset);
+ TransformedInc.Line = Inc.HashLine + 1;
+ TransformedInc.Angled = WrittenRef.starts_with("<");
+ auto FE = SM.getFileManager().getFile(Inc.Resolved);
+ if (!FE) {
+ elog("IncludeCleaner: Failed to get an entry for resolved path {0}: {1}",
+ Inc.Resolved, FE.getError().message());
+ continue;
+ }
+ TransformedInc.Resolved = *FE;
+ Includes.add(std::move(TransformedInc));
+ }
+ return Includes;
+}
+
+std::string spellHeader(ParsedAST &AST, const FileEntry *MainFile,
+ include_cleaner::Header Provider) {
+ std::string SpelledHeader;
+ if (Provider.kind() == include_cleaner::Header::Physical) {
+ if (auto CanonicalPath =
+ getCanonicalPath(Provider.physical(), AST.getSourceManager())) {
+ SpelledHeader =
+ llvm::cantFail(URI::includeSpelling(URI::create(*CanonicalPath)));
+ if (!SpelledHeader.empty())
+ return SpelledHeader;
+ }
+ }
+ return include_cleaner::spellHeader(
+ Provider, AST.getPreprocessor().getHeaderSearchInfo(), MainFile);
+}
+
+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;
+}
+
+bool isFilteredByConfig(const Config &Cfg, llvm::StringRef HeaderSpelling) {
+ for (auto &Filter : Cfg.Diagnostics.Includes.IgnoreHeader) {
+ if (Filter(HeaderSpelling))
+ return true;
+ }
+ return false;
+}
+
+std::vector<Diag> generateMissingIncludeDiagnostics(const Config &Cfg,
+ ParsedAST &AST, llvm::ArrayRef<MissingIncludeDiagInfo> MissingIncludes,
+ llvm::StringRef Code) {
+ std::vector<Diag> Result;
+ if (Cfg.Diagnostics.MissingIncludes != Config::IncludesPolicy::Strict ||
+ Cfg.Diagnostics.Suppress.contains("missing-includes")) {
+ return Result;
+ }
+
+ tooling::HeaderIncludes HeaderIncludes(AST.tuPath(), Code,
+ tooling::IncludeStyle{});
+ const SourceManager &SM = AST.getSourceManager();
+ const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
+ for (const auto &SymbolWithMissingInclude : MissingIncludes) {
+ std::string Spelling =
+ spellHeader(AST, MainFile, SymbolWithMissingInclude.Providers.front());
+ if (isFilteredByConfig(Cfg, Spelling)) {
+ dlog("{0} header is filtered out by the configuration", Spelling);
+ continue;
+ }
+
+ llvm::StringRef HeaderRef{Spelling};
+ bool Angled = HeaderRef.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(
+ HeaderRef.trim("\"<>"), Angled, tooling::IncludeDirective::Include);
+ if (!Replacement.has_value())
+ continue;
+
+ Diag &D = Result.emplace_back();
+ D.Message =
+ llvm::formatv("No header providing \"{0}\" is directly included",
+ SymbolWithMissingInclude.SymbolName);
+ D.Name = "missing-includes";
+ D.Source = Diag::DiagSource::Clangd;
+ D.File = AST.tuPath();
+ D.Severity = DiagnosticsEngine::Warning;
+ D.Range = clangd::Range{
+ offsetToPosition(Code,
+ SymbolWithMissingInclude.SymRefRange.beginOffset()),
+ offsetToPosition(Code,
+ SymbolWithMissingInclude.SymRefRange.endOffset())};
+ D.Fixes.emplace_back();
+ D.Fixes.back().Message = "#include " + Spelling;
+ TextEdit Edit = replacementToEdit(Code, *Replacement);
+ D.Fixes.back().Edits.emplace_back(std::move(Edit));
+ D.InsideMainFile = true;
+ }
+ return Result;
+}
+
+std::vector<Diag> generateUnusedIncludeDiagnostics(
+ PathRef FileName, llvm::ArrayRef<const Inclusion *> UnusedIncludes,
+ llvm::StringRef Code) {
+ std::vector<Diag> Result;
+ for (const auto *Inc : UnusedIncludes) {
+ Diag &D = Result.emplace_back();
+ D.Message =
+ llvm::formatv("included header {0} is not used directly",
+ llvm::sys::path::filename(
+ Inc->Written.substr(1, Inc->Written.size() - 2),
+ llvm::sys::path::Style::posix));
+ D.Name = "unused-includes";
+ D.Source = Diag::DiagSource::Clangd;
+ D.File = FileName;
+ D.Severity = DiagnosticsEngine::Warning;
+ D.Tags.push_back(Unnecessary);
+ D.Range = getDiagnosticRange(Code, Inc->HashOffset);
+ // FIXME(kirillbobyrev): Removing inclusion might break the code if the
+ // used headers are only reachable transitively through this one. Suggest
+ // including them directly instead.
+ // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
+ // (keep/export) remove the warning once we support IWYU pragmas.
+ D.Fixes.emplace_back();
+ D.Fixes.back().Message = "remove #include directive";
+ D.Fixes.back().Edits.emplace_back();
+ D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
+ D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
+ D.InsideMainFile = true;
+ }
+ return Result;
+}
} // namespace
ReferencedLocations findReferencedLocations(ASTContext &Ctx, Preprocessor &PP,
@@ -474,105 +647,112 @@
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) {
- 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*/{});
+
+IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
+ const auto &SM = AST.getSourceManager();
+ const auto &Includes = AST.getIncludeStructure();
+ include_cleaner::Includes ConvertedIncludes =
+ convertIncludes(SM, Includes.MainFileIncludes);
+ const FileEntry *MainFile = SM.getFileEntryForID(SM.getMainFileID());
+
+ std::vector<include_cleaner::SymbolReference> Macros =
+ collectMacroReferences(AST);
+ std::vector<MissingIncludeDiagInfo> MissingIncludes;
+ llvm::DenseSet<IncludeStructure::HeaderID> Used;
+ trace::Span Tracer("include_cleaner::walkUsed");
+ include_cleaner::walkUsed(
+ AST.getLocalTopLevelDecls(), /*MacroRefs=*/Macros,
+ AST.getPragmaIncludes(), SM,
+ [&](const include_cleaner::SymbolReference &Ref,
+ llvm::ArrayRef<include_cleaner::Header> Providers) {
+ bool Satisfied = false;
+ for (const auto &H : Providers) {
+ if (H.kind() == include_cleaner::Header::Physical &&
+ H.physical() == MainFile) {
+ Satisfied = true;
+ continue;
+ }
+ for (auto *Inc : ConvertedIncludes.match(H)) {
+ Satisfied = true;
+ auto HeaderID = Includes.getID(Inc->Resolved);
+ assert(HeaderID.has_value() &&
+ "ConvertedIncludes only contains resolved includes.");
+ Used.insert(*HeaderID);
+ }
+ }
+
+ if (Satisfied || Providers.empty() ||
+ Ref.RT != include_cleaner::RefType::Explicit)
+ return;
+
+ auto &Tokens = AST.getTokens();
+ auto SpelledForExpanded =
+ Tokens.spelledForExpanded(Tokens.expandedTokens(Ref.RefLocation));
+ if (!SpelledForExpanded)
+ return;
+
+ auto Range = syntax::Token::range(
+ AST.getSourceManager(), (*SpelledForExpanded)[0],
+ (*SpelledForExpanded)[(*SpelledForExpanded).size() - 1]);
+ std::string SymbolName;
+ switch (Ref.Target.kind()) {
+ case include_cleaner::Symbol::Macro:
+ SymbolName = Ref.Target.macro().Name->getName();
+ break;
+ case include_cleaner::Symbol::Declaration:
+ SymbolName = llvm::dyn_cast<NamedDecl>(&Ref.Target.declaration())
+ ->getQualifiedNameAsString();
+ break;
+ }
+ MissingIncludeDiagInfo DiagInfo{SymbolName, Range, Providers};
+ MissingIncludes.push_back(std::move(DiagInfo));
+ });
+ std::vector<const Inclusion *> UnusedIncludes =
+ getUnused(AST, Used, /*ReferencedPublicHeaders*/ {});
+ return {std::move(UnusedIncludes), std::move(MissingIncludes)};
}
-std::vector<Diag> issueUnusedIncludesDiagnostics(ParsedAST &AST,
+std::vector<Diag> issueIncludeCleanerDiagnostics(ParsedAST &AST,
llvm::StringRef Code) {
- const Config &Cfg = Config::current();
- if (Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::None ||
- Cfg.Diagnostics.SuppressAll ||
- Cfg.Diagnostics.Suppress.contains("unused-includes"))
- return {};
// Interaction is only polished for C/CPP.
if (AST.getLangOpts().ObjC)
return {};
- trace::Span Tracer("IncludeCleaner::issueUnusedIncludesDiagnostics");
- std::vector<Diag> Result;
- std::string FileName =
- AST.getSourceManager()
- .getFileEntryRefForID(AST.getSourceManager().getMainFileID())
- ->getName()
- .str();
- const auto &UnusedIncludes =
- Cfg.Diagnostics.UnusedIncludes == Config::UnusedIncludesPolicy::Experiment
- ? computeUnusedIncludesExperimental(AST)
- : computeUnusedIncludes(AST);
- for (const auto *Inc : UnusedIncludes) {
- Diag D;
- D.Message =
- llvm::formatv("included header {0} is not used directly",
- llvm::sys::path::filename(
- Inc->Written.substr(1, Inc->Written.size() - 2),
- llvm::sys::path::Style::posix));
- D.Name = "unused-includes";
- D.Source = Diag::DiagSource::Clangd;
- D.File = FileName;
- D.Severity = DiagnosticsEngine::Warning;
- D.Tags.push_back(Unnecessary);
- D.Range = getDiagnosticRange(Code, Inc->HashOffset);
- // FIXME(kirillbobyrev): Removing inclusion might break the code if the
- // used headers are only reachable transitively through this one. Suggest
- // including them directly instead.
- // FIXME(kirillbobyrev): Add fix suggestion for adding IWYU pragmas
- // (keep/export) remove the warning once we support IWYU pragmas.
- D.Fixes.emplace_back();
- D.Fixes.back().Message = "remove #include directive";
- D.Fixes.back().Edits.emplace_back();
- D.Fixes.back().Edits.back().range.start.line = Inc->HashLine;
- D.Fixes.back().Edits.back().range.end.line = Inc->HashLine + 1;
- D.InsideMainFile = true;
- Result.push_back(std::move(D));
+ const Config &Cfg = Config::current();
+ if (Cfg.Diagnostics.SuppressAll ||
+ (Cfg.Diagnostics.Suppress.contains("unused-includes") &&
+ Cfg.Diagnostics.Suppress.contains("missing-includes"))) {
+ return {};
+ }
+
+ trace::Span Tracer("IncludeCleaner::issueIncludeCleanerDiagnostics");
+
+ std::vector<const Inclusion *> UnusedIncludes;
+ std::vector<MissingIncludeDiagInfo> MissingIncludes;
+
+ IncludeCleanerFindings Findings;
+ if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict ||
+ Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Experiment) {
+ // will need include-cleaner results, call it once
+ Findings = computeIncludeCleanerFindings(AST);
}
+
+ if (!Cfg.Diagnostics.Suppress.contains("unused-includes")) {
+ switch (Cfg.Diagnostics.UnusedIncludes) {
+ case Config::IncludesPolicy::Experiment:
+ UnusedIncludes = std::move(Findings.UnusedIncludes);
+ break;
+ case Config::IncludesPolicy::Strict:
+ UnusedIncludes = computeUnusedIncludes(AST);
+ break;
+ case Config::IncludesPolicy::None:
+ // do nothing
+ break;
+ }
+ }
+
+ std::vector<Diag> Result = generateMissingIncludeDiagnostics(Cfg, AST, MissingIncludes, Code);
+ llvm::move(generateUnusedIncludeDiagnostics(AST.tuPath(), UnusedIncludes, Code),
+ std::back_inserter(Result));
return Result;
}
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,19 @@
/// - None
std::optional<Located<std::string>> UnusedIncludes;
+ /// Controls if clangd should analyze missing #include directives.
+ /// clangd will warn if no header providing a symbol is `#include`d
+ /// (missing) directly, and suggest adding it.
+ ///
+ /// Strict means a header providing a symbol 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
@@ -432,15 +432,27 @@
if (F.UnusedIncludes)
if (auto Val =
- compileEnum<Config::UnusedIncludesPolicy>("UnusedIncludes",
+ compileEnum<Config::IncludesPolicy>("UnusedIncludes",
**F.UnusedIncludes)
- .map("Strict", Config::UnusedIncludesPolicy::Strict)
- .map("Experiment", Config::UnusedIncludesPolicy::Experiment)
- .map("None", Config::UnusedIncludesPolicy::None)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("Experiment", Config::IncludesPolicy::Experiment)
+ .map("None", Config::IncludesPolicy::None)
.value())
Out.Apply.push_back([Val](const Params &, Config &C) {
C.Diagnostics.UnusedIncludes = *Val;
});
+
+ if (F.MissingIncludes)
+ if (auto Val =
+ compileEnum<Config::IncludesPolicy>("MissingIncludes",
+ **F.MissingIncludes)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("None", Config::IncludesPolicy::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,11 +88,12 @@
bool StandardLibrary = true;
} Index;
- enum UnusedIncludesPolicy {
- /// Diagnose unused includes.
+ enum IncludesPolicy {
+ /// Diagnose missing and unused includes.
Strict,
None,
- /// The same as Strict, but using the include-cleaner library.
+ /// The same as Strict, but using the include-cleaner library for
+ /// unused includes.
Experiment,
};
/// Controls warnings and errors when parsing code.
@@ -107,7 +108,8 @@
llvm::StringMap<std::string> CheckOptions;
} ClangTidy;
- UnusedIncludesPolicy UnusedIncludes = None;
+ IncludesPolicy UnusedIncludes = None;
+ IncludesPolicy MissingIncludes = 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
