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()); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
