Author: Kirill Bobyrev Date: 2021-12-02T10:26:08+01:00 New Revision: d3a4ef35685c6aaad74294b678e77e1b75af1918
URL: https://github.com/llvm/llvm-project/commit/d3a4ef35685c6aaad74294b678e77e1b75af1918 DIFF: https://github.com/llvm/llvm-project/commit/d3a4ef35685c6aaad74294b678e77e1b75af1918.diff LOG: [clangd] IncludeClenaer: Don't mark forward declarations of a class if it's declared in the main file This will mark more headers that are unrelated to used symbol but contain its forawrd declaration. E.g. the following are examples of headers forward declaring `llvm::StringRef`: - clang/include/clang/Basic/Cuda.h - llvm/include/llvm/Support/SHA256.h - llvm/include/llvm/Support/TrigramIndex.h - llvm/include/llvm/Support/RandomNumberGenerator. - ... and more (~50 in total) This patch is a reduced version of D112707 which was controversial. Reviewed By: kadircet Differential Revision: https://reviews.llvm.org/D114864 Added: Modified: clang-tools-extra/clangd/IncludeCleaner.cpp clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clangd/IncludeCleaner.cpp b/clang-tools-extra/clangd/IncludeCleaner.cpp index 33964d9f83ce0..76d5d626b9f88 100644 --- a/clang-tools-extra/clangd/IncludeCleaner.cpp +++ b/clang-tools-extra/clangd/IncludeCleaner.cpp @@ -33,7 +33,9 @@ namespace { class ReferencedLocationCrawler : public RecursiveASTVisitor<ReferencedLocationCrawler> { public: - ReferencedLocationCrawler(ReferencedLocations &Result) : Result(Result) {} + ReferencedLocationCrawler(ReferencedLocations &Result, + const SourceManager &SM) + : Result(Result), SM(SM) {} bool VisitDeclRefExpr(DeclRefExpr *DRE) { add(DRE->getDecl()); @@ -120,6 +122,17 @@ class ReferencedLocationCrawler void add(const Decl *D) { if (!D || !isNew(D->getCanonicalDecl())) return; + // If we see a declaration in the mainfile, skip all non-definition decls. + // We only do this for classes because forward declarations are common and + // we don't want to include every header that forward-declares the symbol + // because they're often unrelated. + if (const auto *RD = llvm::dyn_cast<RecordDecl>(D)) { + if (SM.isInMainFile(RD->getMostRecentDecl()->getBeginLoc())) { + if (const auto *Definition = RD->getMostRecentDecl()->getDefinition()) + Result.insert(Definition->getLocation()); + return; + } + } for (const Decl *Redecl : D->redecls()) Result.insert(Redecl->getLocation()); } @@ -128,6 +141,7 @@ class ReferencedLocationCrawler ReferencedLocations &Result; llvm::DenseSet<const void *> Visited; + const SourceManager &SM; }; // Given a set of referenced FileIDs, determines all the potentially-referenced @@ -253,7 +267,7 @@ FileID headerResponsible(FileID ID, const SourceManager &SM, ReferencedLocations findReferencedLocations(ParsedAST &AST) { trace::Span Tracer("IncludeCleaner::findReferencedLocations"); ReferencedLocations Result; - ReferencedLocationCrawler Crawler(Result); + ReferencedLocationCrawler Crawler(Result, AST.getSourceManager()); Crawler.TraverseAST(AST.getASTContext()); findReferencedMacros(AST, Result); return Result; diff --git a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp index f72791da4030a..ddc0a99d58356 100644 --- a/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp +++ b/clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp @@ -39,6 +39,18 @@ TEST(IncludeCleaner, ReferencedLocations) { "class ^X;", "X *y;", }, + // FIXME(kirillbobyrev): 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;", + }, // TypedefType and UsingDecls { "using ^Integer = int;", _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
