kadircet updated this revision to Diff 478614.
kadircet added a comment.
- Rebase
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D138200/new/
https://reviews.llvm.org/D138200
Files:
clang-tools-extra/include-cleaner/lib/Analysis.cpp
clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
clang-tools-extra/include-cleaner/test/Inputs/foo2.h
clang-tools-extra/include-cleaner/test/html.cpp
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
Index: clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
===================================================================
--- clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
+++ clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
@@ -25,6 +25,7 @@
namespace clang::include_cleaner {
namespace {
+using testing::Contains;
using testing::Pair;
using testing::UnorderedElementsAre;
@@ -32,8 +33,45 @@
return "#pragma once\n" + Code.str();
}
-TEST(WalkUsed, Basic) {
- // FIXME: Have a fixture for setting up tests.
+class WalkUsedTest : public testing::Test {
+protected:
+ TestInputs Inputs;
+ PragmaIncludes PI;
+ WalkUsedTest() {
+ Inputs.MakeAction = [this] {
+ struct Hook : public SyntaxOnlyAction {
+ public:
+ Hook(PragmaIncludes *Out) : Out(Out) {}
+ bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
+ Out->record(CI);
+ return true;
+ }
+
+ PragmaIncludes *Out;
+ };
+ return std::make_unique<Hook>(&PI);
+ };
+ }
+
+ llvm::DenseMap<size_t, std::vector<Header>>
+ offsetToProviders(TestAST &AST, SourceManager &SM) {
+ llvm::SmallVector<Decl *> TopLevelDecls;
+ for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) {
+ TopLevelDecls.emplace_back(D);
+ }
+ llvm::DenseMap<size_t, std::vector<Header>> OffsetToProviders;
+ walkUsed(TopLevelDecls, /*MacroRefs=*/{}, &PI, SM,
+ [&](const SymbolReference &Ref, llvm::ArrayRef<Header> Providers) {
+ auto [FID, Offset] = SM.getDecomposedLoc(Ref.RefLocation);
+ if (FID != SM.getMainFileID())
+ ADD_FAILURE() << "Reference outside of the main file!";
+ OffsetToProviders.try_emplace(Offset, Providers.vec());
+ });
+ return OffsetToProviders;
+ }
+};
+
+TEST_F(WalkUsedTest, Basic) {
llvm::Annotations Code(R"cpp(
#include "header.h"
#include "private.h"
@@ -43,7 +81,7 @@
std::$vector^vector $vconstructor^v;
}
)cpp");
- TestInputs Inputs(Code.code());
+ Inputs.Code = Code.code();
Inputs.ExtraFiles["header.h"] = guard(R"cpp(
void foo();
namespace std { class vector {}; }
@@ -53,51 +91,53 @@
class Private {};
)cpp");
- PragmaIncludes PI;
- Inputs.MakeAction = [&PI] {
- struct Hook : public SyntaxOnlyAction {
- public:
- Hook(PragmaIncludes *Out) : Out(Out) {}
- bool BeginSourceFileAction(clang::CompilerInstance &CI) override {
- Out->record(CI);
- return true;
- }
-
- PragmaIncludes *Out;
- };
- return std::make_unique<Hook>(&PI);
- };
TestAST AST(Inputs);
-
- llvm::SmallVector<Decl *> TopLevelDecls;
- for (Decl *D : AST.context().getTranslationUnitDecl()->decls()) {
- TopLevelDecls.emplace_back(D);
- }
-
auto &SM = AST.sourceManager();
- llvm::DenseMap<size_t, std::vector<Header>> OffsetToProviders;
- walkUsed(TopLevelDecls, /*MacroRefs=*/{}, &PI, SM,
- [&](const SymbolReference &Ref, llvm::ArrayRef<Header> Providers) {
- auto [FID, Offset] = SM.getDecomposedLoc(Ref.RefLocation);
- EXPECT_EQ(FID, SM.getMainFileID());
- OffsetToProviders.try_emplace(Offset, Providers.vec());
- });
- auto &FM = AST.fileManager();
- auto HeaderFile = Header(FM.getFile("header.h").get());
+ auto HeaderFile = Header(AST.fileManager().getFile("header.h").get());
+ auto PrivateFile = Header(AST.fileManager().getFile("private.h").get());
+ auto PublicFile = Header("\"path/public.h\"");
auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
auto VectorSTL = Header(tooling::stdlib::Header::named("<vector>").value());
EXPECT_THAT(
- OffsetToProviders,
+ offsetToProviders(AST, SM),
UnorderedElementsAre(
Pair(Code.point("bar"), UnorderedElementsAre(MainFile)),
Pair(Code.point("private"),
- UnorderedElementsAre(Header("\"path/public.h\""),
- Header(FM.getFile("private.h").get()))),
+ UnorderedElementsAre(PublicFile, PrivateFile)),
Pair(Code.point("foo"), UnorderedElementsAre(HeaderFile)),
Pair(Code.point("vector"), UnorderedElementsAre(VectorSTL)),
Pair(Code.point("vconstructor"), UnorderedElementsAre(VectorSTL))));
}
+TEST_F(WalkUsedTest, MultipleProviders) {
+ llvm::Annotations Code(R"cpp(
+ #include "header1.h"
+ #include "header2.h"
+ void foo();
+
+ void bar() {
+ $foo^foo();
+ }
+ )cpp");
+ Inputs.Code = Code.code();
+ Inputs.ExtraFiles["header1.h"] = guard(R"cpp(
+ void foo();
+ )cpp");
+ Inputs.ExtraFiles["header2.h"] = guard(R"cpp(
+ void foo();
+ )cpp");
+
+ TestAST AST(Inputs);
+ auto &SM = AST.sourceManager();
+ auto HeaderFile1 = Header(AST.fileManager().getFile("header1.h").get());
+ auto HeaderFile2 = Header(AST.fileManager().getFile("header2.h").get());
+ auto MainFile = Header(SM.getFileEntryForID(SM.getMainFileID()));
+ EXPECT_THAT(
+ offsetToProviders(AST, SM),
+ Contains(Pair(Code.point("foo"),
+ UnorderedElementsAre(HeaderFile1, HeaderFile2, MainFile))));
+}
+
TEST(WalkUsed, MacroRefs) {
llvm::Annotations Hdr(R"cpp(
#define ^ANSWER 42
@@ -134,6 +174,5 @@
UnorderedElementsAre(Pair(Main.point(), UnorderedElementsAre(HdrFile))));
}
-
} // namespace
} // namespace clang::include_cleaner
Index: clang-tools-extra/include-cleaner/test/html.cpp
===================================================================
--- clang-tools-extra/include-cleaner/test/html.cpp
+++ clang-tools-extra/include-cleaner/test/html.cpp
@@ -1,6 +1,11 @@
// RUN: clang-include-cleaner -html=- %s -- -I %S/Inputs | FileCheck %s
#include "bar.h"
#include "foo.h"
+#include "foo2.h"
int n = foo();
+// CHECK: Symbol{{.*}}foo
+// CHECK-NEXT: int foo()
+// CHECK-NEXT: Location{{.*}}foo.h:4:5
+// CHECK-NEXT: Location{{.*}}foo2.h:4:5
// CHECK: <span class='ref sel' data-hover='t{{[0-9]+}}'>foo</span>()
Index: clang-tools-extra/include-cleaner/test/Inputs/foo2.h
===================================================================
--- /dev/null
+++ clang-tools-extra/include-cleaner/test/Inputs/foo2.h
@@ -0,0 +1,6 @@
+#ifndef FOO2_H
+#define FOO2_H
+
+int foo();
+
+#endif
Index: clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
===================================================================
--- clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
+++ clang-tools-extra/include-cleaner/lib/HTMLReport.cpp
@@ -137,22 +137,10 @@
Target makeTarget(const SymbolReference &SR) {
Target T{SR.Target, SR.RT, {}, {}};
-
- // Duplicates logic from walkUsed(), which doesn't expose SymbolLocations.
- // FIXME: use locateDecl and friends once implemented.
- // This doesn't use stdlib::Recognizer, but locateDecl will soon do that.
- switch (SR.Target.kind()) {
- case Symbol::Declaration:
- T.Locations.push_back(SR.Target.declaration().getLocation());
- break;
- case Symbol::Macro:
- T.Locations.push_back(SR.Target.macro().Definition);
- break;
- }
-
+ for (auto &Loc : locateSymbol(SR.Target))
+ T.Locations.push_back(Loc);
for (const auto &Loc : T.Locations)
- T.Headers = findHeaders(Loc, SM, PI);
-
+ T.Headers.append(findHeaders(Loc, SM, PI));
return T;
}
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
@@ -8,38 +8,56 @@
#include "clang-include-cleaner/Analysis.h"
#include "AnalysisInternal.h"
+#include "clang-include-cleaner/Record.h"
#include "clang-include-cleaner/Types.h"
#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/SmallVector.h"
+#include <unordered_map>
namespace clang::include_cleaner {
+namespace {
+// Gets all the providers for a symbol by tarversing each location.
+llvm::SmallVector<Header> findAllHeaders(const Symbol &S,
+ const SourceManager &SM,
+ const PragmaIncludes *PI) {
+ llvm::SmallVector<Header> Headers;
+ for (auto &Loc : locateSymbol(S))
+ // FIXME: findHeaders in theory returns the same result for all source
+ // locations in the same FileID. Have a cache for that, since we can have
+ // multiple declarations coming from the same file.
+ Headers.append(findHeaders(Loc, SM, PI));
+ return Headers;
+}
+} // namespace
+
void walkUsed(llvm::ArrayRef<Decl *> ASTRoots,
llvm::ArrayRef<SymbolReference> MacroRefs,
const PragmaIncludes *PI, const SourceManager &SM,
UsedSymbolCB CB) {
// This is duplicated in writeHTMLReport, changes should be mirrored there.
+
+ // Cache for decl to header mappings, as the same decl might be referenced in
+ // multiple locations and finding providers for each location is expensive.
+ std::unordered_map<NamedDecl *, llvm::SmallVector<Header>> DeclToHeaders;
tooling::stdlib::Recognizer Recognizer;
for (auto *Root : ASTRoots) {
auto &SM = Root->getASTContext().getSourceManager();
walkAST(*Root, [&](SourceLocation Loc, NamedDecl &ND, RefType RT) {
SymbolReference SymRef{Loc, ND, RT};
- if (auto SS = Recognizer(&ND)) {
- // FIXME: Also report forward decls from main-file, so that the caller
- // can decide to insert/ignore a header.
- return CB(SymRef, findHeaders(*SS, SM, PI));
- }
- // FIXME: Extract locations from redecls.
- return CB(SymRef, findHeaders(ND.getLocation(), SM, PI));
+ auto Inserted = DeclToHeaders.try_emplace(&ND);
+ if (Inserted.second)
+ Inserted.first->second = findAllHeaders(ND, SM, PI);
+ return CB(SymRef, Inserted.first->second);
});
}
for (const SymbolReference &MacroRef : MacroRefs) {
assert(MacroRef.Target.kind() == Symbol::Macro);
- return CB(MacroRef,
- findHeaders(MacroRef.Target.macro().Definition, SM, PI));
+ return CB(MacroRef, findAllHeaders(MacroRef.Target, SM, PI));
}
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
