ioeric created this revision.
ioeric added reviewers: ilya-biryukov, sammccall.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay.
Pros:
o Loading macros from preamble for every completion is slow (see profile).
o Calculating macro USR is also slow (see profile).
o Sema can provide a lot of macro completion results (e.g. when filter is empty,
60k for some large TUs!).
Cons:
o Slight memory increase in dynamic index (~1%).
o Some extra work during preamble build (should be fine as preamble build and
indexAST is way slower).
[profile place holder]
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D52078
Files:
clangd/ClangdServer.cpp
clangd/index/FileIndex.cpp
clangd/index/FileIndex.h
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/TestTU.cpp
Index: unittests/clangd/TestTU.cpp
===================================================================
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -51,7 +51,7 @@
std::unique_ptr<SymbolIndex> TestTU::index() const {
auto AST = build();
auto Content = indexAST(AST.getASTContext(), AST.getPreprocessorPtr(),
- AST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
return MemIndex::build(std::move(Content.first), std::move(Content.second));
}
Index: unittests/clangd/IndexTests.cpp
===================================================================
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -244,7 +244,7 @@
Test.Filename = "test.cc";
auto AST = Test.build();
Dyn.update(Test.Filename, &AST.getASTContext(), AST.getPreprocessorPtr(),
- AST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
// Build static index for test.cc.
Test.HeaderCode = HeaderCode;
@@ -254,7 +254,7 @@
// Add stale refs for test.cc.
StaticIndex.update(Test.Filename, &StaticAST.getASTContext(),
StaticAST.getPreprocessorPtr(),
- StaticAST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, StaticAST.getLocalTopLevelDecls());
// Add refs for test2.cc
Annotations Test2Code(R"(class $Foo[[Foo]] {};)");
@@ -265,7 +265,7 @@
StaticAST = Test2.build();
StaticIndex.update(Test2.Filename, &StaticAST.getASTContext(),
StaticAST.getPreprocessorPtr(),
- StaticAST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, StaticAST.getLocalTopLevelDecls());
RefsRequest Request;
Request.IDs = {Foo.ID};
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -15,6 +15,7 @@
#include "index/FileIndex.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Index/IndexSymbol.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Tooling/CompilationDatabase.h"
#include "gtest/gtest.h"
@@ -135,7 +136,8 @@
File.HeaderFilename = (Basename + ".h").str();
File.HeaderCode = Code;
auto AST = File.build();
- M.update(File.Filename, &AST.getASTContext(), AST.getPreprocessorPtr());
+ M.update(File.Filename, &AST.getASTContext(), AST.getPreprocessorPtr(),
+ /*IndexMacros=*/true);
}
TEST(FileIndexTest, CustomizedURIScheme) {
@@ -333,23 +335,38 @@
Test.Filename = "test.cc";
auto AST = Test.build();
Index.update(Test.Filename, &AST.getASTContext(), AST.getPreprocessorPtr(),
- AST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
// Add test2.cc
TestTU Test2;
Test2.HeaderCode = HeaderCode;
Test2.Code = MainCode.code();
Test2.Filename = "test2.cc";
AST = Test2.build();
Index.update(Test2.Filename, &AST.getASTContext(), AST.getPreprocessorPtr(),
- AST.getLocalTopLevelDecls());
+ /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
EXPECT_THAT(getRefs(Index, Foo.ID),
RefsAre({AllOf(RefRange(MainCode.range("foo")),
FileURI("unittest:///test.cc")),
AllOf(RefRange(MainCode.range("foo")),
FileURI("unittest:///test2.cc"))}));
}
+TEST(FileIndexTest, CollectMacros) {
+ FileIndex M;
+ update(M, "f", "#define MAC 1; class X {};");
+
+ FuzzyFindRequest Req;
+ Req.Query = "";
+ bool SeenSymbol = false;
+ M.fuzzyFind(Req, [&](const Symbol &Sym) {
+ EXPECT_EQ(Sym.Name, "MAC");
+ EXPECT_EQ(Sym.SymInfo.Kind, index::SymbolKind::Macro);
+ SeenSymbol = true;
+ });
+ EXPECT_TRUE(SeenSymbol);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -223,12 +223,13 @@
void TestAfterDotCompletion(clangd::CodeCompleteOptions Opts) {
auto Results = completions(
R"cpp(
- #define MACRO X
-
int global_var;
int global_func();
+ // Make sure this is not in preamble.
+ #define MACRO X
+
struct GlobalClass {};
struct ClassWithMembers {
@@ -276,11 +277,12 @@
void TestGlobalScopeCompletion(clangd::CodeCompleteOptions Opts) {
auto Results = completions(
R"cpp(
- #define MACRO X
-
int global_var;
int global_func();
+ // Make sure this is not in preamble.
+ #define MACRO X
+
struct GlobalClass {};
struct ClassWithMembers {
@@ -430,10 +432,11 @@
TEST(CompletionTest, Kinds) {
auto Results = completions(
R"cpp(
- #define MACRO X
int variable;
struct Struct {};
int function();
+ // make sure MACRO is not included in preamble.
+ #define MACRO 10
int X = ^
)cpp",
{func("indexFunction"), var("indexVariable"), cls("indexClass")});
@@ -1905,6 +1908,21 @@
UnorderedElementsAre(Named("Clangd_Macro_Test")));
}
+TEST(CompletionTest, NoMacroFromPreambleIfIndexIsSet) {
+ auto Results = completions(
+ R"cpp(
+ #define CLANGD_PREAMBLE x
+ int x = 0;
+ #define CLANGD_MAIN x
+ void f() { CLANGD_^ }
+ )cpp",
+ {func("CLANGD_INDEX")});
+ // Index is overriden in code completion options, so the preamble symbol is
+ // not seen.
+ EXPECT_THAT(Results.Completions, UnorderedElementsAre(Named("CLANGD_MAIN"),
+ Named("CLANGD_INDEX")));
+}
+
TEST(CompletionTest, DeprecatedResults) {
std::string Body = R"cpp(
void TestClangd();
Index: clangd/index/FileIndex.h
===================================================================
--- clangd/index/FileIndex.h
+++ clangd/index/FileIndex.h
@@ -67,10 +67,12 @@
/// nullptr, this removes all symbols in the file.
/// If \p AST is not null, \p PP cannot be null and it should be the
/// preprocessor that was used to build \p AST.
+ /// If \p IndexMacros is true, also collect macro definitions in \p PP.
/// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all
/// top level decls obtained from \p AST are indexed.
void
update(PathRef Path, ASTContext *AST, std::shared_ptr<Preprocessor> PP,
+ bool IndexMacros = false,
llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls = llvm::None);
private:
@@ -86,8 +88,10 @@
/// If URISchemes is empty, the default schemes in SymbolCollector will be used.
/// If \p TopLevelDecls is set, only these decls are indexed. Otherwise, all top
/// level decls obtained from \p AST are indexed.
+/// If \p IndexMacros is true, also collect macro definitions in \p PP.
std::pair<SymbolSlab, RefSlab>
indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
+ bool IndexMacros = false,
llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls = llvm::None,
llvm::ArrayRef<std::string> URISchemes = {});
Index: clangd/index/FileIndex.cpp
===================================================================
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -10,14 +10,17 @@
#include "FileIndex.h"
#include "Logger.h"
#include "SymbolCollector.h"
+#include "index/Index.h"
+#include "clang/Index/IndexSymbol.h"
#include "clang/Index/IndexingAction.h"
+#include "clang/Lex/MacroInfo.h"
#include "clang/Lex/Preprocessor.h"
namespace clang {
namespace clangd {
std::pair<SymbolSlab, RefSlab>
-indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP,
+indexAST(ASTContext &AST, std::shared_ptr<Preprocessor> PP, bool IndexMacros,
llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls,
llvm::ArrayRef<std::string> URISchemes) {
SymbolCollector::Options CollectorOpts;
@@ -28,6 +31,7 @@
// CommentHandler for IWYU pragma) to canonicalize includes.
CollectorOpts.CollectIncludePath = false;
CollectorOpts.CountReferences = false;
+ CollectorOpts.CollectMacro = true;
if (!URISchemes.empty())
CollectorOpts.URISchemes = URISchemes;
CollectorOpts.Origin = SymbolOrigin::Dynamic;
@@ -54,13 +58,26 @@
SymbolCollector Collector(std::move(CollectorOpts));
Collector.setPreprocessor(PP);
+ Collector.initialize(AST);
+
+ if (IndexMacros) {
+ // FIXME: also collect macro references?
+ for (const auto &M : PP->macros())
+ if (MacroDirective *MD = M.second.getLatest())
+ Collector.handleMacroOccurence(
+ M.first, MD->getMacroInfo(),
+ static_cast<unsigned>(index::SymbolRole::Definition),
+ MD->getLocation());
+ }
+
+ // Collector will be re-initialized.
index::indexTopLevelDecls(AST, DeclsToIndex, Collector, IndexOpts);
const auto &SM = AST.getSourceManager();
const auto *MainFileEntry = SM.getFileEntryForID(SM.getMainFileID());
std::string FileName = MainFileEntry ? MainFileEntry->getName() : "";
- auto Syms = Collector.takeSymbols();
+ auto Syms = Collector.takeSymbols();;
auto Refs = Collector.takeRefs();
vlog("index {0}AST for {1}: \n"
" symbol slab: {2} symbols, {3} bytes\n"
@@ -142,13 +159,13 @@
}
void FileIndex::update(PathRef Path, ASTContext *AST,
- std::shared_ptr<Preprocessor> PP,
+ std::shared_ptr<Preprocessor> PP, bool IndexMacros,
llvm::Optional<llvm::ArrayRef<Decl *>> TopLevelDecls) {
if (!AST) {
FSymbols.update(Path, nullptr, nullptr);
} else {
assert(PP);
- auto Contents = indexAST(*AST, PP, TopLevelDecls, URISchemes);
+ auto Contents = indexAST(*AST, PP, IndexMacros, TopLevelDecls, URISchemes);
FSymbols.update(Path,
llvm::make_unique<SymbolSlab>(std::move(Contents.first)),
llvm::make_unique<RefSlab>(std::move(Contents.second)));
Index: clangd/ClangdServer.cpp
===================================================================
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -90,13 +90,14 @@
void onPreambleAST(PathRef Path, ASTContext &Ctx,
std::shared_ptr<clang::Preprocessor> PP) override {
- This->PreambleIdx.update(Path, &Ctx, std::move(PP));
+ This->PreambleIdx.update(Path, &Ctx, std::move(PP),
+ /*IndexMacros=*/true);
}
void onMainAST(PathRef Path, ParsedAST &AST) override {
- This->MainFileIdx.update(Path, &AST.getASTContext(),
- AST.getPreprocessorPtr(),
- AST.getLocalTopLevelDecls());
+ This->MainFileIdx.update(
+ Path, &AST.getASTContext(), AST.getPreprocessorPtr(),
+ /*IndexMacros=*/false, AST.getLocalTopLevelDecls());
}
};
return llvm::make_unique<CB>(this);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
