[clang-tools-extra] 7ab16be - [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 10 02:56:27 PST 2023
Author: Haojian Wu
Date: 2023-03-10T11:56:11+01:00
New Revision: 7ab16be4c0302f648f85dbf1fe3d1a5dde9b9600
URL: https://github.com/llvm/llvm-project/commit/7ab16be4c0302f648f85dbf1fe3d1a5dde9b9600
DIFF: https://github.com/llvm/llvm-project/commit/7ab16be4c0302f648f85dbf1fe3d1a5dde9b9600.diff
LOG: [clangd] UnusedIncludes: Strict config now uses the include-cleaner-library implementation.
And remove the classical clangd-own unused-include implementation.
Differential Revision: https://reviews.llvm.org/D145773
Added:
Modified:
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/IncludeCleaner.h
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/Config.h b/clang-tools-extra/clangd/Config.h
index ab346aab0e8ac..daadf0ee3d3ce 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -92,9 +92,6 @@ struct Config {
/// Diagnose missing and unused includes.
Strict,
None,
- /// The same as Strict, but using the include-cleaner library for
- /// unused includes.
- Experiment,
};
/// Controls warnings and errors when parsing code.
struct {
diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp
index 2f0ef892131ca..fb6c6e86c1acd 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -430,16 +430,24 @@ struct FragmentCompiler {
C.Diagnostics.Suppress.insert(N);
});
- if (F.UnusedIncludes)
- if (auto Val = compileEnum<Config::IncludesPolicy>("UnusedIncludes",
- **F.UnusedIncludes)
- .map("Strict", Config::IncludesPolicy::Strict)
- .map("Experiment", Config::IncludesPolicy::Experiment)
- .map("None", Config::IncludesPolicy::None)
- .value())
+ if (F.UnusedIncludes) {
+ auto Val = compileEnum<Config::IncludesPolicy>("UnusedIncludes",
+ **F.UnusedIncludes)
+ .map("Strict", Config::IncludesPolicy::Strict)
+ .map("None", Config::IncludesPolicy::None)
+ .value();
+ if (!Val && **F.UnusedIncludes == "Experiment") {
+ diag(Warning,
+ "Experiment is deprecated for UnusedIncludes, use Strict instead.",
+ F.UnusedIncludes->Range);
+ Val = Config::IncludesPolicy::Strict;
+ }
+ if (Val) {
Out.Apply.push_back([Val](const Params &, Config &C) {
C.Diagnostics.UnusedIncludes = *Val;
});
+ }
+ }
if (F.AllowStalePreamble) {
if (auto Val = F.AllowStalePreamble)
diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h
index 956e8a8599446..a56e919cbaf7a 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -232,7 +232,6 @@ struct Fragment {
///
/// Valid values are:
/// - Strict
- /// - Experiment
/// - None
std::optional<Located<std::string>> UnusedIncludes;
diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 0d74b888557ed..a990b89edb230 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -16,7 +16,6 @@
#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"
@@ -24,7 +23,6 @@
#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"
@@ -36,7 +34,6 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/Core/Replacement.h"
#include "clang/Tooling/Inclusions/HeaderIncludes.h"
-#include "clang/Tooling/Inclusions/IncludeStyle.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "clang/Tooling/Syntax/Tokens.h"
#include "llvm/ADT/ArrayRef.h"
@@ -66,181 +63,6 @@ void setIncludeCleanerAnalyzesStdlib(bool B) { AnalyzeStdlib = B; }
namespace {
-/// Crawler traverses the AST and feeds in the locations of (sometimes
-/// implicitly) used symbols into \p Result.
-class ReferencedLocationCrawler
- : public RecursiveASTVisitor<ReferencedLocationCrawler> {
-public:
- ReferencedLocationCrawler(ReferencedLocations &Result,
- const SourceManager &SM)
- : Result(Result), SM(SM) {}
-
- bool VisitDeclRefExpr(DeclRefExpr *DRE) {
- add(DRE->getDecl());
- add(DRE->getFoundDecl());
- return true;
- }
-
- bool VisitMemberExpr(MemberExpr *ME) {
- add(ME->getMemberDecl());
- add(ME->getFoundDecl().getDecl());
- return true;
- }
-
- bool VisitTagType(TagType *TT) {
- add(TT->getDecl());
- return true;
- }
-
- bool VisitFunctionDecl(FunctionDecl *FD) {
- // Function definition will require redeclarations to be included.
- if (FD->isThisDeclarationADefinition())
- add(FD);
- return true;
- }
-
- bool VisitCXXConstructExpr(CXXConstructExpr *CCE) {
- add(CCE->getConstructor());
- return true;
- }
-
- bool VisitTemplateSpecializationType(TemplateSpecializationType *TST) {
- // Using templateName case is handled by the override TraverseTemplateName.
- if (TST->getTemplateName().getKind() == TemplateName::UsingTemplate)
- return true;
- add(TST->getAsCXXRecordDecl()); // Specialization
- return true;
- }
-
- // There is no VisitTemplateName in RAV, thus we override the Traverse version
- // to handle the Using TemplateName case.
- bool TraverseTemplateName(TemplateName TN) {
- VisitTemplateName(TN);
- return Base::TraverseTemplateName(TN);
- }
- // A pseudo VisitTemplateName, dispatched by the above TraverseTemplateName!
- bool VisitTemplateName(TemplateName TN) {
- if (const auto *USD = TN.getAsUsingShadowDecl()) {
- add(USD);
- return true;
- }
- add(TN.getAsTemplateDecl()); // Primary template.
- return true;
- }
-
- bool VisitUsingType(UsingType *UT) {
- add(UT->getFoundDecl());
- return true;
- }
-
- bool VisitTypedefType(TypedefType *TT) {
- add(TT->getDecl());
- return true;
- }
-
- // Consider types of any subexpression used, even if the type is not named.
- // This is helpful in getFoo().bar(), where Foo must be complete.
- // FIXME(kirillbobyrev): Should we tweak this? It may not be desirable to
- // consider types "used" when they are not directly spelled in code.
- bool VisitExpr(Expr *E) {
- TraverseType(E->getType());
- return true;
- }
-
- bool TraverseType(QualType T) {
- if (isNew(T.getTypePtrOrNull())) // don't care about quals
- Base::TraverseType(T);
- return true;
- }
-
- bool VisitUsingDecl(UsingDecl *D) {
- for (const auto *Shadow : D->shadows())
- add(Shadow->getTargetDecl());
- return true;
- }
-
- // Enums may be usefully forward-declared as *complete* types by specifying
- // an underlying type. In this case, the definition should see the declaration
- // so they can be checked for compatibility.
- bool VisitEnumDecl(EnumDecl *D) {
- if (D->isThisDeclarationADefinition() && D->getIntegerTypeSourceInfo())
- add(D);
- return true;
- }
-
- // When the overload is not resolved yet, mark all candidates as used.
- bool VisitOverloadExpr(OverloadExpr *E) {
- for (const auto *ResolutionDecl : E->decls())
- add(ResolutionDecl);
- return true;
- }
-
-private:
- using Base = RecursiveASTVisitor<ReferencedLocationCrawler>;
-
- void add(const Decl *D) {
- if (!D || !isNew(D->getCanonicalDecl()))
- return;
- if (auto SS = StdRecognizer(D)) {
- Result.Stdlib.insert(*SS);
- return;
- }
- // Special case RecordDecls, as it is common for them to be forward
- // declared multiple times. The most common cases are:
- // - Definition available in TU, only mark that one as usage. The rest is
- // likely to be unnecessary. This might result in false positives when an
- // internal definition is visible.
- // - There's a forward declaration in the main file, no need for other
- // redecls.
- if (const auto *RD = llvm::dyn_cast<RecordDecl>(D)) {
- if (const auto *Definition = RD->getDefinition()) {
- Result.User.insert(Definition->getLocation());
- return;
- }
- if (SM.isInMainFile(RD->getMostRecentDecl()->getLocation()))
- return;
- }
- for (const Decl *Redecl : D->redecls())
- Result.User.insert(Redecl->getLocation());
- }
-
- bool isNew(const void *P) { return P && Visited.insert(P).second; }
-
- ReferencedLocations &Result;
- llvm::DenseSet<const void *> Visited;
- const SourceManager &SM;
- tooling::stdlib::Recognizer StdRecognizer;
-};
-
-// Given a set of referenced FileIDs, determines all the potentially-referenced
-// files and macros by traversing expansion/spelling locations of macro IDs.
-// This is used to map the referenced SourceLocations onto real files.
-struct ReferencedFilesBuilder {
- ReferencedFilesBuilder(const SourceManager &SM) : SM(SM) {}
- llvm::DenseSet<FileID> Files;
- llvm::DenseSet<FileID> Macros;
- const SourceManager &SM;
-
- void add(SourceLocation Loc) { add(SM.getFileID(Loc), Loc); }
-
- void add(FileID FID, SourceLocation Loc) {
- if (FID.isInvalid())
- return;
- assert(SM.isInFileID(Loc, FID));
- if (Loc.isFileID()) {
- Files.insert(FID);
- return;
- }
- // Don't process the same macro FID twice.
- if (!Macros.insert(FID).second)
- return;
- const auto &Exp = SM.getSLocEntry(FID).getExpansion();
- add(Exp.getSpellingLoc());
- add(Exp.getExpansionLocStart());
- add(Exp.getExpansionLocEnd());
- }
-};
-
// Returns the range starting at '#' and ending at EOL. Escaped newlines are not
// handled.
clangd::Range getDiagnosticRange(llvm::StringRef Code, unsigned HashOffset) {
@@ -255,31 +77,6 @@ clangd::Range getDiagnosticRange(llvm::StringRef Code, unsigned HashOffset) {
return Result;
}
-// Finds locations of macros referenced from within the main file. That includes
-// references that were not yet expanded, e.g `BAR` in `#define FOO BAR`.
-void findReferencedMacros(const SourceManager &SM, Preprocessor &PP,
- const syntax::TokenBuffer *Tokens,
- ReferencedLocations &Result) {
- trace::Span Tracer("IncludeCleaner::findReferencedMacros");
- // FIXME(kirillbobyrev): The macros from the main file are collected in
- // ParsedAST's MainFileMacros. However, we can't use it here because it
- // doesn't handle macro references that were not expanded, e.g. in macro
- // definitions or preprocessor-disabled sections.
- //
- // Extending MainFileMacros to collect missing references and switching to
- // this mechanism (as opposed to iterating through all tokens) will improve
- // the performance of findReferencedMacros and also improve other features
- // relying on MainFileMacros.
- for (const syntax::Token &Tok : Tokens->spelledTokens(SM.getMainFileID())) {
- auto Macro = locateMacroAt(Tok, PP);
- if (!Macro)
- continue;
- auto Loc = Macro->Info->getDefinitionLoc();
- if (Loc.isValid())
- Result.User.insert(Loc);
- // FIXME: support stdlib macros
- }
-}
bool isFilteredByConfig(const Config &Cfg, llvm::StringRef HeaderPath) {
// Convert the path to Unix slashes and try to match against the filter.
@@ -330,31 +127,6 @@ static bool mayConsiderUnused(const Inclusion &Inc, ParsedAST &AST,
return true;
}
-// In case symbols are coming from non self-contained header, we need to find
-// its first includer that is self-contained. This is the header users can
-// include, so it will be responsible for bringing the symbols from given
-// header into the scope.
-FileID headerResponsible(FileID ID, const SourceManager &SM,
- const IncludeStructure &Includes) {
- // Unroll the chain of non self-contained headers until we find the one that
- // can be included.
- for (const FileEntry *FE = SM.getFileEntryForID(ID); ID != SM.getMainFileID();
- FE = SM.getFileEntryForID(ID)) {
- // If FE is nullptr, we consider it to be the responsible header.
- if (!FE)
- break;
- auto HID = Includes.getID(FE);
- assert(HID && "We're iterating over headers already existing in "
- "IncludeStructure");
- if (Includes.isSelfContained(*HID))
- break;
- // The header is not self-contained: put the responsibility for its symbols
- // on its includer.
- ID = SM.getFileID(SM.getIncludeLoc(ID));
- }
- return ID;
-}
-
include_cleaner::Includes
convertIncludes(const SourceManager &SM,
const llvm::ArrayRef<Inclusion> MainFileIncludes) {
@@ -546,81 +318,6 @@ std::vector<Diag> generateUnusedIncludeDiagnostics(
}
} // namespace
-ReferencedLocations findReferencedLocations(ASTContext &Ctx, Preprocessor &PP,
- const syntax::TokenBuffer *Tokens) {
- trace::Span Tracer("IncludeCleaner::findReferencedLocations");
- ReferencedLocations Result;
- const auto &SM = Ctx.getSourceManager();
- ReferencedLocationCrawler Crawler(Result, SM);
- Crawler.TraverseAST(Ctx);
- if (Tokens)
- findReferencedMacros(SM, PP, Tokens, Result);
- return Result;
-}
-
-ReferencedLocations findReferencedLocations(ParsedAST &AST) {
- return findReferencedLocations(AST.getASTContext(), AST.getPreprocessor(),
- &AST.getTokens());
-}
-
-ReferencedFiles findReferencedFiles(
- const ReferencedLocations &Locs, const SourceManager &SM,
- llvm::function_ref<FileID(FileID)> HeaderResponsible,
- llvm::function_ref<std::optional<StringRef>(FileID)> UmbrellaHeader) {
- std::vector<SourceLocation> Sorted{Locs.User.begin(), Locs.User.end()};
- llvm::sort(Sorted); // Group by FileID.
- ReferencedFilesBuilder Builder(SM);
- for (auto It = Sorted.begin(); It < Sorted.end();) {
- FileID FID = SM.getFileID(*It);
- Builder.add(FID, *It);
- // Cheaply skip over all the other locations from the same FileID.
- // This avoids lots of redundant Loc->File lookups for the same file.
- do
- ++It;
- while (It != Sorted.end() && SM.isInFileID(*It, FID));
- }
-
- // If a header is not self-contained, we consider its symbols a logical part
- // of the including file. Therefore, mark the parents of all used
- // non-self-contained FileIDs as used. Perform this on FileIDs rather than
- // HeaderIDs, as each inclusion of a non-self-contained file is distinct.
- llvm::DenseSet<FileID> UserFiles;
- llvm::StringSet<> PublicHeaders;
- for (FileID ID : Builder.Files) {
- UserFiles.insert(HeaderResponsible(ID));
- if (auto PublicHeader = UmbrellaHeader(ID)) {
- PublicHeaders.insert(*PublicHeader);
- }
- }
-
- llvm::DenseSet<tooling::stdlib::Header> StdlibFiles;
- for (const auto &Symbol : Locs.Stdlib)
- for (const auto &Header : Symbol.headers())
- StdlibFiles.insert(Header);
-
- return {std::move(UserFiles), std::move(StdlibFiles),
- std::move(PublicHeaders)};
-}
-
-ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,
- const IncludeStructure &Includes,
- const CanonicalIncludes &CanonIncludes,
- const SourceManager &SM) {
- return findReferencedFiles(
- Locs, SM,
- [&SM, &Includes](FileID ID) {
- return headerResponsible(ID, SM, Includes);
- },
- [&SM, &CanonIncludes](FileID ID) -> std::optional<StringRef> {
- auto Entry = SM.getFileEntryRefForID(ID);
- if (!Entry)
- return std::nullopt;
- auto PublicHeader = CanonIncludes.mapHeader(*Entry);
- if (PublicHeader.empty())
- return std::nullopt;
- return PublicHeader;
- });
-}
std::vector<const Inclusion *>
getUnused(ParsedAST &AST,
@@ -648,52 +345,6 @@ getUnused(ParsedAST &AST,
return Unused;
}
-#ifndef NDEBUG
-// Is FID a <built-in>, <scratch space> etc?
-static bool isSpecialBuffer(FileID FID, const SourceManager &SM) {
- const SrcMgr::FileInfo &FI = SM.getSLocEntry(FID).getFile();
- return FI.getName().startswith("<");
-}
-#endif
-
-llvm::DenseSet<IncludeStructure::HeaderID>
-translateToHeaderIDs(const ReferencedFiles &Files,
- const IncludeStructure &Includes,
- const SourceManager &SM) {
- trace::Span Tracer("IncludeCleaner::translateToHeaderIDs");
- llvm::DenseSet<IncludeStructure::HeaderID> TranslatedHeaderIDs;
- TranslatedHeaderIDs.reserve(Files.User.size());
- for (FileID FID : Files.User) {
- const FileEntry *FE = SM.getFileEntryForID(FID);
- if (!FE) {
- assert(isSpecialBuffer(FID, SM));
- continue;
- }
- const auto File = Includes.getID(FE);
- assert(File);
- TranslatedHeaderIDs.insert(*File);
- }
- for (tooling::stdlib::Header StdlibUsed : Files.Stdlib)
- for (auto HID : Includes.StdlibHeaders.lookup(StdlibUsed))
- TranslatedHeaderIDs.insert(HID);
- return TranslatedHeaderIDs;
-}
-
-// 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.
-std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST) {
- const auto &SM = AST.getSourceManager();
-
- auto Refs = findReferencedLocations(AST);
- auto ReferencedFiles =
- findReferencedFiles(Refs, AST.getIncludeStructure(),
- AST.getCanonicalIncludes(), AST.getSourceManager());
- auto ReferencedHeaders =
- translateToHeaderIDs(ReferencedFiles, AST.getIncludeStructure(), SM);
- return getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas);
-}
-
IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST) {
const auto &SM = AST.getSourceManager();
const auto &Includes = AST.getIncludeStructure();
@@ -758,17 +409,13 @@ std::vector<Diag> issueIncludeCleanerDiagnostics(ParsedAST &AST,
const Config &Cfg = Config::current();
IncludeCleanerFindings Findings;
if (Cfg.Diagnostics.MissingIncludes == Config::IncludesPolicy::Strict ||
- Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Experiment) {
+ Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict) {
// will need include-cleaner results, call it once
Findings = computeIncludeCleanerFindings(AST);
}
std::vector<Diag> Result = generateUnusedIncludeDiagnostics(
- AST.tuPath(),
- Cfg.Diagnostics.UnusedIncludes == Config::IncludesPolicy::Strict
- ? computeUnusedIncludes(AST)
- : Findings.UnusedIncludes,
- Code);
+ AST.tuPath(), Findings.UnusedIncludes, Code);
llvm::move(
generateMissingIncludeDiagnostics(AST, Findings.MissingIncludes, Code),
std::back_inserter(Result));
diff --git a/clang-tools-extra/clangd/IncludeCleaner.h b/clang-tools-extra/clangd/IncludeCleaner.h
index 989067e3d84fb..d7edca035c965 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.h
+++ b/clang-tools-extra/clangd/IncludeCleaner.h
@@ -21,14 +21,9 @@
#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/StringSet.h"
-#include <optional>
#include <tuple>
#include <vector>
@@ -52,63 +47,6 @@ struct IncludeCleanerFindings {
std::vector<MissingIncludeDiagInfo> MissingIncludes;
};
-struct ReferencedLocations {
- llvm::DenseSet<SourceLocation> User;
- llvm::DenseSet<tooling::stdlib::Symbol> Stdlib;
-};
-
-/// Finds locations of all symbols used in the main file.
-///
-/// - RecursiveASTVisitor finds references to symbols and records their
-/// associated locations. These may be macro expansions, and are not resolved
-/// to their spelling or expansion location. These locations are later used to
-/// determine which headers should be marked as "used" and "directly used".
-/// - If \p Tokens is not nullptr, we also examine all identifier tokens in the
-/// file in case they reference macros macros.
-/// We use this to compute unused headers, so we:
-///
-/// - cover the whole file in a single traversal for efficiency
-/// - don't attempt to describe where symbols were referenced from in
-/// ambiguous cases (e.g. implicitly used symbols, multiple declarations)
-/// - err on the side of reporting all possible locations
-ReferencedLocations findReferencedLocations(ASTContext &Ctx, Preprocessor &PP,
- const syntax::TokenBuffer *Tokens);
-ReferencedLocations findReferencedLocations(ParsedAST &AST);
-
-struct ReferencedFiles {
- llvm::DenseSet<FileID> User;
- llvm::DenseSet<tooling::stdlib::Header> Stdlib;
- /// Files responsible for the symbols referenced in the main file and defined
- /// in private headers (private headers have IWYU pragma: private, include
- /// "public.h"). We store spelling of the public header (with quotes or angle
- /// brackets) files here to avoid dealing with full filenames and visibility.
- llvm::StringSet<> SpelledUmbrellas;
-};
-
-/// Retrieves IDs of all files containing SourceLocations from \p Locs.
-/// The output only includes things SourceManager sees as files (not macro IDs).
-/// This can include <built-in>, <scratch space> etc that are not true files.
-/// \p HeaderResponsible returns the public header that should be included given
-/// symbols from a file with the given FileID (example: public headers should be
-/// preferred to non self-contained and private headers).
-/// \p UmbrellaHeader returns the public public header is responsible for
-/// providing symbols from a file with the given FileID (example: MyType.h
-/// should be included instead of MyType_impl.h).
-ReferencedFiles findReferencedFiles(
- const ReferencedLocations &Locs, const SourceManager &SM,
- llvm::function_ref<FileID(FileID)> HeaderResponsible,
- llvm::function_ref<std::optional<StringRef>(FileID)> UmbrellaHeader);
-ReferencedFiles findReferencedFiles(const ReferencedLocations &Locs,
- const IncludeStructure &Includes,
- const CanonicalIncludes &CanonIncludes,
- const SourceManager &SM);
-
-/// Maps FileIDs to the internal IncludeStructure representation (HeaderIDs).
-/// FileIDs that are not true files (<built-in> etc) are dropped.
-llvm::DenseSet<IncludeStructure::HeaderID>
-translateToHeaderIDs(const ReferencedFiles &Files,
- const IncludeStructure &Includes, const SourceManager &SM);
-
/// Retrieves headers that are referenced from the main file but not used.
/// In unclear cases, headers are not marked as unused.
std::vector<const Inclusion *>
@@ -117,7 +55,6 @@ getUnused(ParsedAST &AST,
const llvm::StringSet<> &ReferencedPublicHeaders);
IncludeCleanerFindings computeIncludeCleanerFindings(ParsedAST &AST);
-std::vector<const Inclusion *> computeUnusedIncludes(ParsedAST &AST);
std::vector<Diag> issueIncludeCleanerDiagnostics(ParsedAST &AST,
llvm::StringRef Code);
diff --git a/clang-tools-extra/clangd/Preamble.cpp b/clang-tools-extra/clangd/Preamble.cpp
index c4aec747acc0f..eca7f0e6e6e07 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -128,7 +128,7 @@ class CppFilePreambleCallbacks : public PreambleCallbacks {
SourceMgr = &CI.getSourceManager();
Includes.collect(CI);
if (Config::current().Diagnostics.UnusedIncludes ==
- Config::IncludesPolicy::Experiment ||
+ Config::IncludesPolicy::Strict ||
Config::current().Diagnostics.MissingIncludes ==
Config::IncludesPolicy::Strict)
Pragmas.record(CI);
diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
index d098b986d2c8f..fc4401b8fa0ff 100644
--- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -38,7 +38,6 @@ namespace {
using ::testing::AllOf;
using ::testing::ElementsAre;
-using ::testing::ElementsAreArray;
using ::testing::IsEmpty;
using ::testing::Matcher;
using ::testing::Pointee;
@@ -64,286 +63,6 @@ std::string guard(llvm::StringRef Code) {
return "#pragma once\n" + Code.str();
}
-TEST(IncludeCleaner, ReferencedLocations) {
- struct TestCase {
- std::string HeaderCode;
- std::string MainCode;
- };
- TestCase Cases[] = {
- // DeclRefExpr
- {
- "int ^x();",
- "int y = x();",
- },
- // RecordDecl
- {
- "class ^X;",
- "X *y;",
- },
- // When definition is available, we don't need to mark forward
- // declarations as used.
- {
- "class ^X {}; class X;",
- "X *y;",
- },
- // We already have forward declaration in the main file, the other
- // non-definition declarations are not needed.
- {
- "class ^X {}; class X;",
- "class X; X *y;",
- },
- // Nested class definition can occur outside of the parent class
- // definition. Bar declaration should be visible to its definition but
- // it will always be because we will mark Foo definition as used.
- {
- "class ^Foo { class Bar; };",
- "class Foo::Bar {};",
- },
- // TypedefType and UsingDecls
- {
- "using ^Integer = int;",
- "Integer x;",
- },
- {
- "namespace ns { void ^foo(); void ^foo() {} }",
- "using ns::foo;",
- },
- {
- "namespace ns { void foo(); void foo() {}; }",
- "using namespace ns;",
- },
- {
- // Refs from UsingTypeLoc and implicit constructor!
- "struct ^A {}; using B = A; using ^C = B;",
- "C a;",
- },
- {"namespace ns { template<typename T> class A {}; } using ns::^A;",
- "A<int>* a;"},
- {"namespace ns { template<typename T> class A {}; } using ns::^A;",
- R"cpp(
- template <template <typename> class T> class X {};
- X<A> x;
- )cpp"},
- {R"cpp(
- namespace ns { template<typename T> class A {}; }
- namespace absl {using ns::^A;}
- )cpp",
- R"cpp(
- template <template <typename> class T> class X {};
- X<absl::A> x;
- )cpp"},
- {R"cpp(
- namespace ns { template<typename T> struct ^A { ^A(T); }; }
- using ns::^A;
- )cpp",
- "A CATD(123);"},
- {
- "typedef bool ^Y; template <typename T> struct ^X {};",
- "X<Y> x;",
- },
- {
- // https://github.com/clangd/clangd/issues/1036
- R"cpp(
- struct ^Base { void ^base(); };
- template <int> struct ^Derived : Base {};
- )cpp",
- R"cpp(
- class Holder {
- void foo() { Member.base(); }
- Derived<0> Member;
- };
- )cpp",
- },
- {
- "struct Foo; struct ^Foo{}; typedef Foo ^Bar;",
- "Bar b;",
- },
- {
- "namespace ns { class X; }; using ns::^X;",
- "X *y;",
- },
- // MemberExpr
- {
- "struct ^X{int ^a;}; X ^foo();",
- "int y = foo().a;",
- },
- // Expr (type is traversed)
- {
- "class ^X{}; X ^foo();",
- "auto bar() { return foo(); }",
- },
- // Redecls
- {
- "void ^foo(); void ^foo() {} void ^foo();",
- "void bar() { foo(); }",
- },
- // Constructor
- {
- "struct ^X { ^X(int) {} int ^foo(); };",
- "auto x = X(42); auto y = x.foo();",
- },
- // Function
- {
- "void ^foo();",
- "void foo() {}",
- },
- {
- "void foo() {}",
- "void foo();",
- },
- {
- "inline void ^foo() {}",
- "void bar() { foo(); }",
- },
- {
- "int ^foo(char); int ^foo(float);",
- "template<class T> int x = foo(T{});",
- },
- // Static function
- {
- "struct ^X { static bool ^foo(); }; bool X::^foo() {}",
- "auto b = X::foo();",
- },
- // TemplateRecordDecl
- {
- "template <typename> class ^X;",
- "X<int> *y;",
- },
- // Type name not spelled out in code
- {
- "class ^X{}; X ^getX();",
- "auto x = getX();",
- },
- // Enums
- {
- "enum ^Color { ^Red = 42, Green = 9000};",
- "int MyColor = Red;",
- },
- {
- "struct ^X { enum ^Language { ^CXX = 42, Python = 9000}; };",
- "int Lang = X::CXX;",
- },
- // Macros
- {
- "#define ^CONSTANT 42",
- "int Foo = CONSTANT;",
- },
- {
- "#define ^FOO x",
- "#define BAR FOO",
- },
- {
- "#define INNER 42\n"
- "#define ^OUTER INNER",
- "int answer = OUTER;",
- },
- {
- "#define ^ANSWER 42\n"
- "#define ^SQUARE(X) X * X",
- "int sq = SQUARE(ANSWER);",
- },
- {
- "#define ^FOO\n"
- "#define ^BAR",
- "#if 0\n"
- "#if FOO\n"
- "BAR\n"
- "#endif\n"
- "#endif",
- },
- // Misc
- {
- "enum class ^Color : int;",
- "enum class Color : int {};",
- },
- {
- "enum class Color : int {};",
- "enum class Color : int;",
- },
- {
- "enum class ^Color;",
- "Color c;",
- },
- {
- "enum class ^Color : int;",
- "Color c;",
- },
- {
- "enum class ^Color : char;",
- "Color *c;",
- },
- {
- "enum class ^Color : char {};",
- "Color *c;",
- },
- };
- for (const TestCase &T : Cases) {
- TestTU TU;
- TU.Code = T.MainCode;
- Annotations Header(T.HeaderCode);
- TU.HeaderCode = Header.code().str();
- TU.ExtraArgs.push_back("-std=c++17");
- auto AST = TU.build();
-
- std::vector<Position> Points;
- for (const auto &Loc : findReferencedLocations(AST).User) {
- if (AST.getSourceManager().getBufferName(Loc).endswith(
- TU.HeaderFilename)) {
- Points.push_back(offsetToPosition(
- TU.HeaderCode, AST.getSourceManager().getFileOffset(Loc)));
- }
- }
- llvm::sort(Points);
-
- EXPECT_EQ(Points, Header.points()) << T.HeaderCode << "\n---\n"
- << T.MainCode;
- }
-}
-
-TEST(IncludeCleaner, Stdlib) {
- // Smoke tests only for finding used symbols/headers.
- // Details of Decl -> stdlib::Symbol -> stdlib::Headers mapping tested there.
- auto TU = TestTU::withHeaderCode(R"cpp(
- namespace std { class error_code {}; }
- class error_code {};
- namespace nonstd { class error_code {}; }
- )cpp");
- struct {
- llvm::StringRef Code;
- std::vector<llvm::StringRef> Symbols;
- std::vector<llvm::StringRef> Headers;
- } Tests[] = {
- {"std::error_code x;", {"std::error_code"}, {"<system_error>"}},
- {"error_code x;", {}, {}},
- {"nonstd::error_code x;", {}, {}},
- };
-
- for (const auto &Test : Tests) {
- TU.Code = Test.Code.str();
- ParsedAST AST = TU.build();
- std::vector<tooling::stdlib::Symbol> WantSyms;
- for (const auto &SymName : Test.Symbols) {
- auto QName = splitQualifiedName(SymName);
- auto Sym = tooling::stdlib::Symbol::named(QName.first, QName.second);
- EXPECT_TRUE(Sym) << SymName;
- WantSyms.push_back(*Sym);
- }
- std::vector<tooling::stdlib::Header> WantHeaders;
- for (const auto &HeaderName : Test.Headers) {
- auto Header = tooling::stdlib::Header::named(HeaderName);
- EXPECT_TRUE(Header) << HeaderName;
- WantHeaders.push_back(*Header);
- }
-
- ReferencedLocations Locs = findReferencedLocations(AST);
- EXPECT_THAT(Locs.Stdlib, ElementsAreArray(WantSyms));
- ReferencedFiles Files =
- findReferencedFiles(Locs, AST.getIncludeStructure(),
- AST.getCanonicalIncludes(), AST.getSourceManager());
- EXPECT_THAT(Files.Stdlib, ElementsAreArray(WantHeaders));
- }
-}
-
MATCHER_P(writtenInclusion, Written, "") {
if (arg.Written != Written)
*result_listener << arg.Written;
@@ -372,8 +91,6 @@ TEST(IncludeCleaner, StdlibUnused) {
TU.AdditionalFiles["queue"] = "#include <bits>";
TU.ExtraArgs = {"-isystem", testRoot()};
auto AST = TU.build();
- EXPECT_THAT(computeUnusedIncludes(AST),
- ElementsAre(Pointee(writtenInclusion("<queue>"))));
IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
EXPECT_THAT(Findings.UnusedIncludes,
ElementsAre(Pointee(writtenInclusion("<queue>"))));
@@ -408,11 +125,6 @@ TEST(IncludeCleaner, GetUnusedHeaders) {
TU.ExtraArgs.push_back("-isystem" + testPath("system"));
TU.Code = MainFile.str();
ParsedAST AST = TU.build();
- EXPECT_THAT(
- computeUnusedIncludes(AST),
- UnorderedElementsAre(Pointee(writtenInclusion("\"unused.h\"")),
- Pointee(writtenInclusion("\"dir/unused.h\""))));
-
IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
EXPECT_THAT(
Findings.UnusedIncludes,
@@ -544,142 +256,6 @@ TEST(IncludeCleaner, GenerateMissingHeaderDiags) {
"#include <vector>\n", "#include <vector>")))));
}
-TEST(IncludeCleaner, VirtualBuffers) {
- TestTU TU;
- TU.Code = R"cpp(
- #include "macros.h"
-
- using flags::FLAGS_FOO;
-
- // CLI will come from a define, __cplusplus is a built-in. In both cases, they
- // come from non-existent files.
- int y = CLI + __cplusplus;
-
- int concat(a, b) = 42;
- )cpp";
- // The pasting operator in combination with DEFINE_FLAG will create
- // ScratchBuffer with `flags::FLAGS_FOO` that will have FileID but not
- // FileEntry.
- TU.AdditionalFiles["macros.h"] = R"cpp(
- #ifndef MACROS_H
- #define MACROS_H
-
- #define DEFINE_FLAG(X) \
- namespace flags { \
- int FLAGS_##X; \
- } \
-
- DEFINE_FLAG(FOO)
-
- #define ab x
- #define concat(x, y) x##y
-
- #endif // MACROS_H
- )cpp";
- TU.ExtraArgs = {"-DCLI=42"};
- ParsedAST AST = TU.build();
- auto &SM = AST.getSourceManager();
- auto &Includes = AST.getIncludeStructure();
-
- auto ReferencedFiles = findReferencedFiles(
- findReferencedLocations(AST), Includes, AST.getCanonicalIncludes(), SM);
- llvm::StringSet<> ReferencedFileNames;
- for (FileID FID : ReferencedFiles.User)
- ReferencedFileNames.insert(
- SM.getPresumedLoc(SM.getLocForStartOfFile(FID)).getFilename());
- // Note we deduped the names as _number_ of <built-in>s is uninteresting.
- EXPECT_THAT(ReferencedFileNames.keys(),
- UnorderedElementsAre("<built-in>", "<scratch space>",
- testPath("macros.h")));
-
- // Should not crash due to FileIDs that are not headers.
- auto ReferencedHeaders = translateToHeaderIDs(ReferencedFiles, Includes, SM);
- std::vector<llvm::StringRef> ReferencedHeaderNames;
- for (IncludeStructure::HeaderID HID : ReferencedHeaders)
- ReferencedHeaderNames.push_back(Includes.getRealPath(HID));
- // Non-header files are gone at this point.
- EXPECT_THAT(ReferencedHeaderNames, ElementsAre(testPath("macros.h")));
-
- // Sanity check.
- EXPECT_THAT(
- getUnused(AST, ReferencedHeaders, ReferencedFiles.SpelledUmbrellas),
- IsEmpty());
-}
-
-TEST(IncludeCleaner, DistinctUnguardedInclusions) {
- TestTU TU;
- TU.Code = R"cpp(
- #include "bar.h"
- #include "foo.h"
-
- int LocalFoo = foo::Variable;
- )cpp";
- TU.AdditionalFiles["foo.h"] = R"cpp(
- #pragma once
- namespace foo {
- #include "unguarded.h"
- }
- )cpp";
- TU.AdditionalFiles["bar.h"] = R"cpp(
- #pragma once
- namespace bar {
- #include "unguarded.h"
- }
- )cpp";
- TU.AdditionalFiles["unguarded.h"] = R"cpp(
- constexpr int Variable = 42;
- )cpp";
-
- ParsedAST AST = TU.build();
-
- auto ReferencedFiles = findReferencedFiles(
- findReferencedLocations(AST), AST.getIncludeStructure(),
- AST.getCanonicalIncludes(), AST.getSourceManager());
- llvm::StringSet<> ReferencedFileNames;
- auto &SM = AST.getSourceManager();
- for (FileID FID : ReferencedFiles.User)
- ReferencedFileNames.insert(
- SM.getPresumedLoc(SM.getLocForStartOfFile(FID)).getFilename());
- // Note that we have uplifted the referenced files from non self-contained
- // headers to header-guarded ones.
- EXPECT_THAT(ReferencedFileNames.keys(),
- UnorderedElementsAre(testPath("foo.h")));
-}
-
-TEST(IncludeCleaner, NonSelfContainedHeaders) {
- TestTU TU;
- TU.Code = R"cpp(
- #include "foo.h"
-
- int LocalFoo = Variable;
- )cpp";
- TU.AdditionalFiles["foo.h"] = R"cpp(
- #pragma once
- #include "indirection.h"
- )cpp";
- TU.AdditionalFiles["indirection.h"] = R"cpp(
- #include "unguarded.h"
- )cpp";
- TU.AdditionalFiles["unguarded.h"] = R"cpp(
- constexpr int Variable = 42;
- )cpp";
-
- ParsedAST AST = TU.build();
-
- auto ReferencedFiles = findReferencedFiles(
- findReferencedLocations(AST), AST.getIncludeStructure(),
- AST.getCanonicalIncludes(), AST.getSourceManager());
- llvm::StringSet<> ReferencedFileNames;
- auto &SM = AST.getSourceManager();
- for (FileID FID : ReferencedFiles.User)
- ReferencedFileNames.insert(
- SM.getPresumedLoc(SM.getLocForStartOfFile(FID)).getFilename());
- // Note that we have uplifted the referenced files from non self-contained
- // headers to header-guarded ones.
- EXPECT_THAT(ReferencedFileNames.keys(),
- UnorderedElementsAre(testPath("foo.h")));
-}
-
TEST(IncludeCleaner, IWYUPragmas) {
TestTU TU;
TU.Code = R"cpp(
@@ -697,52 +273,10 @@ TEST(IncludeCleaner, IWYUPragmas) {
void foo() {}
)cpp");
Config Cfg;
- Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Experiment;
+ Cfg.Diagnostics.UnusedIncludes = Config::IncludesPolicy::Strict;
WithContextValue Ctx(Config::Key, std::move(Cfg));
ParsedAST AST = TU.build();
-
- auto ReferencedFiles = findReferencedFiles(
- findReferencedLocations(AST), AST.getIncludeStructure(),
- AST.getCanonicalIncludes(), AST.getSourceManager());
- EXPECT_EQ(ReferencedFiles.SpelledUmbrellas.size(), 1u);
- EXPECT_EQ(ReferencedFiles.SpelledUmbrellas.begin()->getKey(), "\"public.h\"");
- EXPECT_EQ(ReferencedFiles.User.size(), 2u);
- EXPECT_TRUE(
- ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
- EXPECT_TRUE(
- ReferencedFiles.User.contains(AST.getSourceManager().getMainFileID()));
- EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
- EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
- IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
- EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
-}
-
-TEST(IncludeCleaner, RecursiveInclusion) {
- TestTU TU;
- TU.Code = R"cpp(
- #include "foo.h"
-
- void baz() {
- foo();
- }
- )cpp";
- TU.AdditionalFiles["foo.h"] = R"cpp(
- #ifndef FOO_H
- #define FOO_H
-
- void foo() {}
-
- #include "bar.h"
-
- #endif
- )cpp";
- TU.AdditionalFiles["bar.h"] = guard(R"cpp(
- #include "foo.h"
- )cpp");
- ParsedAST AST = TU.build();
-
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
- EXPECT_THAT(computeUnusedIncludes(AST), IsEmpty());
IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
}
@@ -768,7 +302,6 @@ TEST(IncludeCleaner, IWYUPragmaExport) {
EXPECT_THAT(AST.getDiagnostics(), llvm::ValueIs(IsEmpty()));
// 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());
IncludeCleanerFindings Findings = computeIncludeCleanerFindings(AST);
EXPECT_THAT(Findings.UnusedIncludes, IsEmpty());
}
More information about the cfe-commits
mailing list