VitaNuo created this revision.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
VitaNuo updated this revision to Diff 531275.
VitaNuo added a comment.
VitaNuo updated this revision to Diff 531278.
VitaNuo added a reviewer: kadircet.
VitaNuo published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.
Remove redundant variables.
VitaNuo added a comment.
Remove redundancy.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D152900
Files:
clang-tools-extra/clangd/index/IndexAction.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/index/SymbolCollector.h
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -9,6 +9,7 @@
#include "Annotations.h"
#include "TestFS.h"
#include "TestTU.h"
+#include "URI.h"
#include "index/SymbolCollector.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
@@ -226,23 +227,18 @@
class SymbolIndexActionFactory : public tooling::FrontendActionFactory {
public:
- SymbolIndexActionFactory(SymbolCollector::Options COpts,
- CommentHandler *PragmaHandler)
- : COpts(std::move(COpts)), PragmaHandler(PragmaHandler) {}
+ SymbolIndexActionFactory(SymbolCollector::Options COpts)
+ : COpts(std::move(COpts)) {}
std::unique_ptr<FrontendAction> create() override {
class IndexAction : public ASTFrontendAction {
public:
IndexAction(std::shared_ptr<index::IndexDataConsumer> DataConsumer,
- const index::IndexingOptions &Opts,
- CommentHandler *PragmaHandler)
- : DataConsumer(std::move(DataConsumer)), Opts(Opts),
- PragmaHandler(PragmaHandler) {}
+ const index::IndexingOptions &Opts)
+ : DataConsumer(std::move(DataConsumer)), Opts(Opts) {}
std::unique_ptr<ASTConsumer>
CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
- if (PragmaHandler)
- CI.getPreprocessor().addCommentHandler(PragmaHandler);
return createIndexingASTConsumer(DataConsumer, Opts,
CI.getPreprocessorPtr());
}
@@ -256,20 +252,17 @@
private:
std::shared_ptr<index::IndexDataConsumer> DataConsumer;
index::IndexingOptions Opts;
- CommentHandler *PragmaHandler;
};
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = true;
Collector = std::make_shared<SymbolCollector>(COpts);
- return std::make_unique<IndexAction>(Collector, std::move(IndexOpts),
- PragmaHandler);
+ return std::make_unique<IndexAction>(Collector, std::move(IndexOpts));
}
std::shared_ptr<SymbolCollector> Collector;
SymbolCollector::Options COpts;
- CommentHandler *PragmaHandler;
};
class SymbolCollectorTest : public ::testing::Test {
@@ -289,8 +282,7 @@
llvm::IntrusiveRefCntPtr<FileManager> Files(
new FileManager(FileSystemOptions(), InMemoryFileSystem));
- auto Factory = std::make_unique<SymbolIndexActionFactory>(
- CollectorOpts, PragmaHandler.get());
+ auto Factory = std::make_unique<SymbolIndexActionFactory>(CollectorOpts);
std::vector<std::string> Args = {"symbol_collector", "-fsyntax-only",
"-xc++", "-include", TestHeaderName};
@@ -324,7 +316,6 @@
RefSlab Refs;
RelationSlab Relations;
SymbolCollector::Options CollectorOpts;
- std::unique_ptr<CommentHandler> PragmaHandler;
};
TEST_F(SymbolCollectorTest, CollectSymbols) {
@@ -1573,9 +1564,6 @@
TEST_F(SymbolCollectorTest, IWYUPragma) {
CollectorOpts.CollectIncludePath = true;
- CanonicalIncludes Includes;
- PragmaHandler = collectIWYUHeaderMaps(&Includes);
- CollectorOpts.Includes = &Includes;
const std::string Header = R"(
// IWYU pragma: private, include the/good/header.h
class Foo {};
@@ -1588,9 +1576,6 @@
TEST_F(SymbolCollectorTest, IWYUPragmaWithDoubleQuotes) {
CollectorOpts.CollectIncludePath = true;
- CanonicalIncludes Includes;
- PragmaHandler = collectIWYUHeaderMaps(&Includes);
- CollectorOpts.Includes = &Includes;
const std::string Header = R"(
// IWYU pragma: private, include "the/good/header.h"
class Foo {};
@@ -1601,6 +1586,27 @@
includeHeader("\"the/good/header.h\""))));
}
+TEST_F(SymbolCollectorTest, IWYUPragmaExport) {
+ CollectorOpts.CollectIncludePath = true;
+ const std::string Header = R"cpp(#pragma once
+ #include "exporter.h"
+ )cpp";
+ auto ExporterFile = testPath("exporter.h");
+ InMemoryFileSystem->addFile(
+ ExporterFile, 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(#pragma once
+ #include "private.h" // IWYU pragma: export
+ )cpp"));
+ auto PrivateFile = testPath("private.h");
+ InMemoryFileSystem->addFile(
+ PrivateFile, 0, llvm::MemoryBuffer::getMemBuffer("class Foo {};"));
+ runSymbolCollector(Header, /*Main=*/"",
+ /*ExtraArgs=*/{"-I", testRoot()});
+ EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(
+ qName("Foo"),
+ includeHeader(URI::create(ExporterFile).toString()),
+ declURI(URI::create(PrivateFile).toString()))));
+}
+
TEST_F(SymbolCollectorTest, SkipIncFileWhenCanonicalizeHeaders) {
auto IncFile = testPath("test.inc");
auto IncURI = URI::create(IncFile).toString();
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -9,6 +9,8 @@
#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_SYMBOLCOLLECTOR_H
#include "CollectMacros.h"
+#include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
#include "index/CanonicalIncludes.h"
#include "index/Ref.h"
#include "index/Relation.h"
@@ -24,6 +26,7 @@
#include "clang/Sema/CodeCompleteConsumer.h"
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
#include <functional>
#include <optional>
@@ -123,8 +126,12 @@
void setPreprocessor(std::shared_ptr<Preprocessor> PP) override {
this->PP = PP.get();
+ PI.record(*this->PP);
+ }
+ void setPreprocessor(Preprocessor &PP) {
+ this->PP = &PP;
+ PI.record(*this->PP);
}
- void setPreprocessor(Preprocessor &PP) { this->PP = &PP; }
bool
handleDeclOccurrence(const Decl *D, index::SymbolRoleSet Roles,
@@ -166,13 +173,24 @@
// All Symbols collected from the AST.
SymbolSlab::Builder Symbols;
- // File IDs for Symbol.IncludeHeaders.
- // The final spelling is calculated in finish().
+ // File IDs to distinguish imports from includes.
llvm::DenseMap<SymbolID, FileID> IncludeFiles;
+ void setIncludeLocation(const Symbol &S, SourceLocation);
+
+ // Providers for Symbol.IncludeHeaders.
+ // The final spelling is calculated in finish().
+ llvm::DenseMap<SymbolID, llvm::SmallVector<include_cleaner::Header>>
+ SymbolProviders;
+ // Mapping from symbol ID to final include spelling.
+ // Needs to be persisted here because Symbols don't store any of their data.
+ llvm::DenseMap<SymbolID, std::string> SymbolIncludeSpelling;
// Files which contain ObjC symbols.
// This is finalized and used in finish().
llvm::DenseSet<FileID> FilesWithObjCConstructs;
- void setIncludeLocation(const Symbol &S, SourceLocation);
+
+ void
+ setSymbolProviders(const Symbol &S,
+ const llvm::SmallVector<include_cleaner::Header> &Headers);
// Indexed macros, to be erased if they turned out to be include guards.
llvm::DenseSet<const IdentifierInfo *> IndexedMacros;
// All refs collected from the AST. It includes:
@@ -184,6 +202,7 @@
RelationSlab::Builder Relations;
ASTContext *ASTCtx;
Preprocessor *PP = nullptr;
+ include_cleaner::PragmaIncludes PI;
std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator;
std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo;
Options Opts;
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -13,8 +13,12 @@
#include "ExpectedTypes.h"
#include "SourceCode.h"
#include "URI.h"
+#include "clang-include-cleaner/Analysis.h"
+#include "clang-include-cleaner/Record.h"
+#include "clang-include-cleaner/Types.h"
#include "index/CanonicalIncludes.h"
#include "index/Relation.h"
+#include "index/Symbol.h"
#include "index/SymbolID.h"
#include "index/SymbolLocation.h"
#include "clang/AST/Decl.h"
@@ -30,10 +34,15 @@
#include "clang/Lex/Token.h"
#include "clang/Tooling/Inclusions/HeaderAnalysis.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallVector.h"
+#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
+#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/FileSystem.h"
#include "llvm/Support/Path.h"
#include <optional>
+#include <string>
namespace clang {
namespace clangd {
@@ -196,12 +205,14 @@
llvm::StringMap<std::string> CachePathToFrameworkSpelling;
llvm::StringMap<FrameworkUmbrellaSpelling>
CacheFrameworkToUmbrellaHeaderSpelling;
+ const include_cleaner::PragmaIncludes &PI;
public:
HeaderFileURICache(Preprocessor *&PP, const SourceManager &SM,
- const SymbolCollector::Options &Opts)
- : PP(PP), SM(SM), Includes(Opts.Includes), FallbackDir(Opts.FallbackDir) {
- }
+ const SymbolCollector::Options &Opts,
+ const include_cleaner::PragmaIncludes &PI)
+ : PP(PP), SM(SM), Includes(Opts.Includes), FallbackDir(Opts.FallbackDir),
+ PI(PI) {}
// Returns a canonical URI for the file \p FE.
// We attempt to make the path absolute first.
@@ -377,6 +388,15 @@
if (!FE || FE->getName().empty())
return "";
llvm::StringRef Filename = FE->getName();
+
+ // Fallback pragma handling for Objective-C.
+ const auto *FileEntry = SM.getFileEntryForID(FID);
+ for (const auto *Export : PI.getExporters(FileEntry, SM.getFileManager()))
+ return toURI(Export->tryGetRealPathName());
+
+ if (auto Verbatim = PI.getPublic(FileEntry); !Verbatim.empty())
+ return Verbatim;
+
// If a file is mapped by canonical headers, use that mapping, regardless
// of whether it's an otherwise-good header (header guards etc).
if (Includes) {
@@ -436,7 +456,7 @@
void SymbolCollector::initialize(ASTContext &Ctx) {
ASTCtx = &Ctx;
HeaderFileURIs = std::make_unique<HeaderFileURICache>(
- this->PP, ASTCtx->getSourceManager(), Opts);
+ this->PP, ASTCtx->getSourceManager(), Opts, PI);
CompletionAllocator = std::make_shared<GlobalCodeCompletionAllocator>();
CompletionTUInfo =
std::make_unique<CodeCompletionTUInfo>(CompletionAllocator);
@@ -765,6 +785,11 @@
IndexedMacros.insert(Name);
setIncludeLocation(S, DefLoc);
+ llvm::SmallVector<include_cleaner::Header> Headers =
+ include_cleaner::headersForSymbol(
+ include_cleaner::Macro{const_cast<IdentifierInfo *>(Name), DefLoc},
+ ASTCtx->getSourceManager(), &PI);
+ setSymbolProviders(S, Headers);
Symbols.insert(S);
return true;
}
@@ -807,6 +832,14 @@
PP->getSourceManager().getDecomposedExpansionLoc(Loc).first;
}
+void SymbolCollector::setSymbolProviders(
+ const Symbol &S,
+ const llvm::SmallVector<include_cleaner::Header> &Headers) {
+ if (Opts.CollectIncludePath &&
+ shouldCollectIncludePath(S.SymInfo.Kind) != Symbol::Invalid)
+ SymbolProviders[S.ID] = Headers;
+}
+
void SymbolCollector::finish() {
// At the end of the TU, add 1 to the refcount of all referenced symbols.
for (const auto &ID : ReferencedSymbols) {
@@ -847,37 +880,77 @@
IncludeHeader = "<algorithm>";
}
}
- // Otherwise find the approprate include header for the defining file.
- if (IncludeHeader.empty())
- IncludeHeader = HeaderFileURIs->getIncludeHeader(FID);
-
- // Symbols in slabs aren't mutable, insert() has to walk all the strings
- if (!IncludeHeader.empty()) {
- Symbol::IncludeDirective Directives = Symbol::Invalid;
- auto CollectDirectives = shouldCollectIncludePath(S->SymInfo.Kind);
- if ((CollectDirectives & Symbol::Include) != 0)
- Directives |= Symbol::Include;
- // Only allow #import for symbols from ObjC-like files.
- if ((CollectDirectives & Symbol::Import) != 0) {
- auto [It, Inserted] = FileToContainsImportsOrObjC.try_emplace(FID);
- if (Inserted)
- It->second = FilesWithObjCConstructs.contains(FID) ||
- tooling::codeContainsImports(
- ASTCtx->getSourceManager().getBufferData(FID));
- if (It->second)
- Directives |= Symbol::Import;
- }
- if (Directives != Symbol::Invalid) {
- Symbol NewSym = *S;
+
+ // Determine if the FID is #include'd or #import'ed.
+ Symbol::IncludeDirective Directives = Symbol::Invalid;
+ auto CollectDirectives = shouldCollectIncludePath(S->SymInfo.Kind);
+ if ((CollectDirectives & Symbol::Include) != 0)
+ Directives |= Symbol::Include;
+ // Only allow #import for symbols from ObjC-like files.
+ if ((CollectDirectives & Symbol::Import) != 0) {
+ auto [It, Inserted] = FileToContainsImportsOrObjC.try_emplace(FID);
+ if (Inserted)
+ It->second = FilesWithObjCConstructs.contains(FID) ||
+ tooling::codeContainsImports(
+ ASTCtx->getSourceManager().getBufferData(FID));
+ if (It->second)
+ Directives |= Symbol::Import;
+ }
+
+ Symbol NewSym;
+ if (Directives != Symbol::Invalid) {
+ NewSym = *S;
+ if (!IncludeHeader.empty()) {
NewSym.IncludeHeaders.push_back({IncludeHeader, 1, Directives});
Symbols.insert(NewSym);
+ continue;
+ }
+ }
+
+ // Use the include location-based logic for Objective-C symbols.
+ if (Directives & Symbol::Import) {
+ // If not overridden manually, find the approprate include header for
+ // the defining file.
+ NewSym.IncludeHeaders.push_back(
+ {HeaderFileURIs->getIncludeHeader(FID), 1, Directives});
+ Symbols.insert(NewSym);
+ continue;
+ }
+
+ // For #include's, use the providers computed by the include-cleaner
+ // library.
+ if (Directives == Symbol::Include) {
+ auto It = SymbolProviders.find(SID);
+ if (It != SymbolProviders.end()) {
+ for (const auto &H : It->second) {
+ switch (H.kind()) {
+ case include_cleaner::Header::Kind::Verbatim:
+ SymbolIncludeSpelling[SID] = H.verbatim().str();
+ break;
+ case include_cleaner::Header::Kind::Standard:
+ SymbolIncludeSpelling[SID] = H.standard().name().str();
+ break;
+ case include_cleaner::Header::Kind::Physical:
+ SymbolIncludeSpelling[SID] = HeaderFileURIs->getIncludeHeader(
+ ASTCtx->getSourceManager().getOrCreateFileID(
+ H.physical(), SrcMgr::CharacteristicKind::C_User));
+ break;
+ }
+ if (!SymbolIncludeSpelling[SID].empty()) {
+ NewSym.IncludeHeaders.push_back(
+ {SymbolIncludeSpelling[SID], 1, Directives});
+ break;
+ }
+ }
}
+ Symbols.insert(NewSym);
}
}
}
ReferencedSymbols.clear();
IncludeFiles.clear();
+ SymbolProviders.clear();
FilesWithObjCConstructs.clear();
}
@@ -952,6 +1025,10 @@
Symbols.insert(S);
setIncludeLocation(S, ND.getLocation());
+ llvm::SmallVector<include_cleaner::Header> Headers =
+ include_cleaner::headersForSymbol(include_cleaner::Symbol{ND},
+ ASTCtx->getSourceManager(), &PI);
+ setSymbolProviders(S, Headers);
if (S.SymInfo.Lang == index::SymbolLanguage::ObjC)
FilesWithObjCConstructs.insert(FID);
return Symbols.find(S.ID);
Index: clang-tools-extra/clangd/index/IndexAction.cpp
===================================================================
--- clang-tools-extra/clangd/index/IndexAction.cpp
+++ clang-tools-extra/clangd/index/IndexAction.cpp
@@ -135,8 +135,7 @@
: SymbolsCallback(SymbolsCallback), RefsCallback(RefsCallback),
RelationsCallback(RelationsCallback),
IncludeGraphCallback(IncludeGraphCallback), Collector(C),
- Includes(std::move(Includes)), Opts(Opts),
- PragmaHandler(collectIWYUHeaderMaps(this->Includes.get())) {
+ Includes(std::move(Includes)), Opts(Opts) {
this->Opts.ShouldTraverseDecl = [this](const Decl *D) {
// Many operations performed during indexing is linear in terms of depth
// of the decl (USR generation, name lookups, figuring out role of a
@@ -154,7 +153,6 @@
std::unique_ptr<ASTConsumer>
CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override {
- CI.getPreprocessor().addCommentHandler(PragmaHandler.get());
Includes->addSystemHeadersMapping(CI.getLangOpts());
if (IncludeGraphCallback != nullptr)
CI.getPreprocessor().addPPCallbacks(
@@ -203,7 +201,6 @@
std::shared_ptr<SymbolCollector> Collector;
std::unique_ptr<CanonicalIncludes> Includes;
index::IndexingOptions Opts;
- std::unique_ptr<CommentHandler> PragmaHandler;
IncludeGraph IG;
};
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
