kbobyrev created this revision.
kbobyrev added a reviewer: kadircet.
Herald added subscribers: usaxena95, arphaman.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
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 <https://reviews.llvm.org/D112707>
which was controversial.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D114864
Files:
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,6 +39,14 @@
"class ^X;",
"X *y;",
},
+ {
+ "class ^X {}; class ^X;",
+ "X *y;",
+ },
+ {
+ "class ^X {}; class X;",
+ "class X; X *y;",
+ },
// TypedefType and UsingDecls
{
"using ^Integer = int;",
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -33,7 +33,9 @@
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());
@@ -48,6 +50,18 @@
}
bool VisitTagType(TagType *TT) {
+ // 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>(TT->getDecl())) {
+ const auto *MostRecent = RD->getMostRecentDecl();
+ if (SM.isInMainFile(MostRecent->getBeginLoc())) {
+ if (const auto *Definition = RD->getDefinition())
+ Result.insert(Definition->getLocation());
+ return true;
+ }
+ }
add(TT->getDecl());
return true;
}
@@ -128,6 +142,7 @@
ReferencedLocations &Result;
llvm::DenseSet<const void *> Visited;
+ const SourceManager &SM;
};
// Given a set of referenced FileIDs, determines all the potentially-referenced
@@ -253,7 +268,7 @@
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;
Index: clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
+++ clang-tools-extra/clangd/unittests/IncludeCleanerTests.cpp
@@ -39,6 +39,14 @@
"class ^X;",
"X *y;",
},
+ {
+ "class ^X {}; class ^X;",
+ "X *y;",
+ },
+ {
+ "class ^X {}; class X;",
+ "class X; X *y;",
+ },
// TypedefType and UsingDecls
{
"using ^Integer = int;",
Index: clang-tools-extra/clangd/IncludeCleaner.cpp
===================================================================
--- clang-tools-extra/clangd/IncludeCleaner.cpp
+++ clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -33,7 +33,9 @@
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());
@@ -48,6 +50,18 @@
}
bool VisitTagType(TagType *TT) {
+ // 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>(TT->getDecl())) {
+ const auto *MostRecent = RD->getMostRecentDecl();
+ if (SM.isInMainFile(MostRecent->getBeginLoc())) {
+ if (const auto *Definition = RD->getDefinition())
+ Result.insert(Definition->getLocation());
+ return true;
+ }
+ }
add(TT->getDecl());
return true;
}
@@ -128,6 +142,7 @@
ReferencedLocations &Result;
llvm::DenseSet<const void *> Visited;
+ const SourceManager &SM;
};
// Given a set of referenced FileIDs, determines all the potentially-referenced
@@ -253,7 +268,7 @@
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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
