hokein updated this revision to Diff 173842.
hokein marked an inline comment as done.
hokein added a comment.
update the patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53427
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Merge.cpp
clangd/index/Serialization.cpp
clangd/index/SymbolCollector.cpp
clangd/index/YAMLSerialization.cpp
clangd/index/dex/Dex.cpp
unittests/clangd/BackgroundIndexTests.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/DexTests.cpp
unittests/clangd/FileIndexTests.cpp
unittests/clangd/IndexTests.cpp
unittests/clangd/SerializationTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -53,8 +53,10 @@
return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
}
MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
-MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
-MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
+MATCHER_P(DeclURI, P, "") {
+ return StringRef(arg.CanonicalDeclaration.FileURI) == P;
+}
+MATCHER_P(DefURI, P, "") { return StringRef(arg.Definition.FileURI) == P; }
MATCHER_P(IncludeHeader, P, "") {
return (arg.IncludeHeaders.size() == 1) &&
(arg.IncludeHeaders.begin()->IncludeHeader == P);
Index: unittests/clangd/SerializationTests.cpp
===================================================================
--- unittests/clangd/SerializationTests.cpp
+++ unittests/clangd/SerializationTests.cpp
@@ -111,7 +111,7 @@
EXPECT_EQ(Sym1.Signature, "");
EXPECT_EQ(Sym1.Documentation, "Foo doc");
EXPECT_EQ(Sym1.ReturnType, "int");
- EXPECT_EQ(Sym1.CanonicalDeclaration.FileURI, "file:///path/foo.h");
+ EXPECT_EQ(StringRef(Sym1.CanonicalDeclaration.FileURI), "file:///path/foo.h");
EXPECT_EQ(Sym1.Origin, SymbolOrigin::Static);
EXPECT_TRUE(Sym1.Flags & Symbol::IndexedForCodeCompletion);
EXPECT_FALSE(Sym1.Flags & Symbol::Deprecated);
@@ -122,7 +122,8 @@
EXPECT_THAT(Sym2, QName("clang::Foo2"));
EXPECT_EQ(Sym2.Signature, "-sig");
EXPECT_EQ(Sym2.ReturnType, "");
- EXPECT_EQ(Sym2.CanonicalDeclaration.FileURI, "file:///path/bar.h");
+ EXPECT_EQ(llvm::StringRef(Sym2.CanonicalDeclaration.FileURI),
+ "file:///path/bar.h");
EXPECT_FALSE(Sym2.Flags & Symbol::IndexedForCodeCompletion);
EXPECT_TRUE(Sym2.Flags & Symbol::Deprecated);
@@ -134,7 +135,7 @@
testing::SizeIs(1))));
auto Ref1 = ParsedYAML->Refs->begin()->second.front();
EXPECT_EQ(Ref1.Kind, RefKind::Reference);
- EXPECT_EQ(Ref1.Location.FileURI, "file:///path/foo.cc");
+ EXPECT_EQ(StringRef(Ref1.Location.FileURI), "file:///path/foo.cc");
}
std::vector<std::string> YAMLFromSymbols(const SymbolSlab &Slab) {
Index: unittests/clangd/IndexTests.cpp
===================================================================
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -36,7 +36,7 @@
std::make_tuple(Range.start.line, Range.start.character,
Range.end.line, Range.end.character);
}
-MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+MATCHER_P(FileURI, F, "") { return StringRef(arg.Location.FileURI) == F; }
TEST(SymbolLocation, Position) {
using Position = SymbolLocation::Position;
@@ -222,7 +222,7 @@
Symbol M = mergeSymbol(L, R);
EXPECT_EQ(M.Name, "Foo");
- EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:///left.h");
+ EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:///left.h");
EXPECT_EQ(M.References, 3u);
EXPECT_EQ(M.Signature, "()");
EXPECT_EQ(M.CompletionSnippetSuffix, "{$1:0}");
@@ -241,14 +241,14 @@
R.Name = "right";
Symbol M = mergeSymbol(L, R);
- EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/left.h");
- EXPECT_EQ(M.Definition.FileURI, "");
+ EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/left.h");
+ EXPECT_EQ(StringRef(M.Definition.FileURI), "");
EXPECT_EQ(M.Name, "left");
R.Definition.FileURI = "file:/right.cpp"; // Now right will be favored.
M = mergeSymbol(L, R);
- EXPECT_EQ(M.CanonicalDeclaration.FileURI, "file:/right.h");
- EXPECT_EQ(M.Definition.FileURI, "file:/right.cpp");
+ EXPECT_EQ(StringRef(M.CanonicalDeclaration.FileURI), "file:/right.h");
+ EXPECT_EQ(StringRef(M.Definition.FileURI), "file:/right.cpp");
EXPECT_EQ(M.Name, "right");
}
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -31,19 +31,21 @@
using testing::IsEmpty;
using testing::Pair;
using testing::UnorderedElementsAre;
+using namespace llvm;
MATCHER_P(RefRange, Range, "") {
return std::make_tuple(arg.Location.Start.line(), arg.Location.Start.column(),
arg.Location.End.line(), arg.Location.End.column()) ==
std::make_tuple(Range.start.line, Range.start.character,
Range.end.line, Range.end.character);
}
-MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
-MATCHER_P(DeclURI, U, "") { return arg.CanonicalDeclaration.FileURI == U; }
-MATCHER_P(DefURI, U, "") { return arg.Definition.FileURI == U; }
+MATCHER_P(FileURI, F, "") { return StringRef(arg.Location.FileURI) == F; }
+MATCHER_P(DeclURI, U, "") {
+ return StringRef(arg.CanonicalDeclaration.FileURI) == U;
+}
+MATCHER_P(DefURI, U, "") { return StringRef(arg.Definition.FileURI) == U; }
MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
-using namespace llvm;
namespace clang {
namespace clangd {
namespace {
@@ -66,7 +68,7 @@
return llvm::make_unique<SymbolSlab>(std::move(Slab).build());
}
-std::unique_ptr<RefSlab> refSlab(const SymbolID &ID, StringRef Path) {
+std::unique_ptr<RefSlab> refSlab(const SymbolID &ID, const char *Path) {
RefSlab::Builder Slab;
Ref R;
R.Location.FileURI = Path;
Index: unittests/clangd/DexTests.cpp
===================================================================
--- unittests/clangd/DexTests.cpp
+++ unittests/clangd/DexTests.cpp
@@ -666,7 +666,7 @@
TEST(DexTests, Refs) {
DenseMap<SymbolID, std::vector<Ref>> Refs;
- auto AddRef = [&](const Symbol& Sym, StringRef Filename, RefKind Kind) {
+ auto AddRef = [&](const Symbol &Sym, const char *Filename, RefKind Kind) {
auto& SymbolRefs = Refs[Sym.ID];
SymbolRefs.emplace_back();
SymbolRefs.back().Kind = Kind;
Index: unittests/clangd/CodeCompleteTests.cpp
===================================================================
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -579,7 +579,7 @@
ClangdServer Server(CDB, FS, DiagConsumer, ClangdServer::optsForTest());
auto BarURI = URI::createFile(BarHeader).toString();
Symbol Sym = cls("ns::X");
- Sym.CanonicalDeclaration.FileURI = BarURI;
+ Sym.CanonicalDeclaration.FileURI = BarURI.c_str();
Sym.IncludeHeaders.emplace_back(BarURI, 1);
// Shoten include path based on search dirctory and insert.
auto Results = completions(Server,
@@ -610,8 +610,8 @@
Symbol SymY = cls("ns::Y");
std::string BarHeader = testPath("bar.h");
auto BarURI = URI::createFile(BarHeader).toString();
- SymX.CanonicalDeclaration.FileURI = BarURI;
- SymY.CanonicalDeclaration.FileURI = BarURI;
+ SymX.CanonicalDeclaration.FileURI = BarURI.c_str();
+ SymY.CanonicalDeclaration.FileURI = BarURI.c_str();
SymX.IncludeHeaders.emplace_back("<bar>", 1);
SymY.IncludeHeaders.emplace_back("<bar>", 1);
// Shoten include path based on search dirctory and insert.
@@ -1248,7 +1248,7 @@
// Differences in header-to-insert suppress bundling.
std::string DeclFile = URI::createFile(testPath("foo")).toString();
- NoArgsGFunc.CanonicalDeclaration.FileURI = DeclFile;
+ NoArgsGFunc.CanonicalDeclaration.FileURI = DeclFile.c_str();
NoArgsGFunc.IncludeHeaders.emplace_back("<foo>", 1);
EXPECT_THAT(
completions(Context + "int y = GFunc^", {NoArgsGFunc}, Opts).Completions,
@@ -1955,7 +1955,7 @@
TEST(CompletionTest, InsertTheMostPopularHeader) {
std::string DeclFile = URI::createFile(testPath("foo")).toString();
Symbol sym = func("Func");
- sym.CanonicalDeclaration.FileURI = DeclFile;
+ sym.CanonicalDeclaration.FileURI = DeclFile.c_str();
sym.IncludeHeaders.emplace_back("\"foo.h\"", 2);
sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000);
@@ -1977,7 +1977,7 @@
std::string DeclFile = URI::createFile(testPath("foo")).toString();
Symbol sym = func("Func");
- sym.CanonicalDeclaration.FileURI = DeclFile;
+ sym.CanonicalDeclaration.FileURI = DeclFile.c_str();
sym.IncludeHeaders.emplace_back("\"foo.h\"", 2);
sym.IncludeHeaders.emplace_back("\"bar.h\"", 1000);
Index: unittests/clangd/BackgroundIndexTests.cpp
===================================================================
--- unittests/clangd/BackgroundIndexTests.cpp
+++ unittests/clangd/BackgroundIndexTests.cpp
@@ -9,14 +9,16 @@
using testing::Not;
using testing::UnorderedElementsAre;
+using namespace llvm;
namespace clang {
namespace clangd {
MATCHER_P(Named, N, "") { return arg.Name == N; }
-MATCHER(Declared, "") { return !arg.CanonicalDeclaration.FileURI.empty(); }
-MATCHER(Defined, "") { return !arg.Definition.FileURI.empty(); }
-
-MATCHER_P(FileURI, F, "") { return arg.Location.FileURI == F; }
+MATCHER(Declared, "") {
+ return !StringRef(arg.CanonicalDeclaration.FileURI).empty();
+}
+MATCHER(Defined, "") { return !StringRef(arg.Definition.FileURI).empty(); }
+MATCHER_P(FileURI, F, "") { return StringRef(arg.Location.FileURI) == F; }
testing::Matcher<const RefSlab &>
RefsAre(std::vector<testing::Matcher<Ref>> Matchers) {
return ElementsAre(testing::Pair(_, UnorderedElementsAreArray(Matchers)));
Index: clangd/index/dex/Dex.cpp
===================================================================
--- clangd/index/dex/Dex.cpp
+++ clangd/index/dex/Dex.cpp
@@ -52,7 +52,7 @@
std::vector<Token> Result = generateIdentifierTrigrams(Sym.Name);
Result.emplace_back(Token::Kind::Scope, Sym.Scope);
// Skip token generation for symbols with unknown declaration location.
- if (!Sym.CanonicalDeclaration.FileURI.empty())
+ if (!StringRef(Sym.CanonicalDeclaration.FileURI).empty())
for (const auto &ProximityURI :
generateProximityURIs(Sym.CanonicalDeclaration.FileURI))
Result.emplace_back(Token::Kind::ProximityURI, ProximityURI);
Index: clangd/index/YAMLSerialization.cpp
===================================================================
--- clangd/index/YAMLSerialization.cpp
+++ clangd/index/YAMLSerialization.cpp
@@ -20,8 +20,10 @@
#include "llvm/ADT/Optional.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/Allocator.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/StringSaver.h"
#include "llvm/Support/YAMLTraits.h"
#include "llvm/Support/raw_ostream.h"
#include <cstdint>
@@ -128,9 +130,26 @@
YPosition P;
};
+struct NormalizedFileURI {
+ NormalizedFileURI(IO &) {}
+ NormalizedFileURI(IO &, const char *FileURI) { URI = FileURI; }
+
+ const char *denormalize(IO &IO) {
+ assert(IO.getContext() &&
+ "Expecting an UniqueStringSaver to allocate data");
+ return static_cast<llvm::UniqueStringSaver *>(IO.getContext())
+ ->save(URI)
+ .data();
+ }
+
+ std::string URI;
+};
+
template <> struct MappingTraits<SymbolLocation> {
static void mapping(IO &IO, SymbolLocation &Value) {
- IO.mapRequired("FileURI", Value.FileURI);
+ MappingNormalization<NormalizedFileURI, const char *> NFile(IO,
+ Value.FileURI);
+ IO.mapRequired("FileURI", NFile->URI);
MappingNormalization<NormalizedPosition, SymbolLocation::Position> NStart(
IO, Value.Start);
IO.mapRequired("Start", NStart->P);
@@ -292,7 +311,9 @@
Expected<IndexFileIn> readYAML(StringRef Data) {
SymbolSlab::Builder Symbols;
RefSlab::Builder Refs;
- yaml::Input Yin(Data);
+ BumpPtrAllocator Arena; // store the underlying data of Position::FileURI.
+ UniqueStringSaver Strings(Arena);
+ yaml::Input Yin(Data, &Strings);
do {
VariantEntry Variant;
Yin >> Variant;
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -225,7 +225,7 @@
return None;
FileURIStorage = std::move(*U);
SymbolLocation Result;
- Result.FileURI = FileURIStorage;
+ Result.FileURI = FileURIStorage.c_str();
auto Range = getTokenRange(TokLoc, SM, LangOpts);
Result.Start = Range.first;
Result.End = Range.second;
@@ -524,7 +524,7 @@
Ref R;
R.Location.Start = Range.first;
R.Location.End = Range.second;
- R.Location.FileURI = *FileURI;
+ R.Location.FileURI = FileURI->c_str();
R.Kind = toRefKind(LocAndRole.second);
Refs.insert(*ID, R);
}
Index: clangd/index/Serialization.cpp
===================================================================
--- clangd/index/Serialization.cpp
+++ clangd/index/Serialization.cpp
@@ -239,7 +239,7 @@
SymbolLocation readLocation(Reader &Data, ArrayRef<StringRef> Strings) {
SymbolLocation Loc;
- Loc.FileURI = Data.consumeString(Strings);
+ Loc.FileURI = Data.consumeString(Strings).data();
for (auto *Endpoint : {&Loc.Start, &Loc.End}) {
Endpoint->setLine(Data.consumeVar());
Endpoint->setColumn(Data.consumeVar());
@@ -409,8 +409,11 @@
if (Data.Refs) {
for (const auto &Sym : *Data.Refs) {
Refs.emplace_back(Sym);
- for (auto &Ref : Refs.back().second)
- Strings.intern(Ref.Location.FileURI);
+ for (auto &Ref : Refs.back().second) {
+ StringRef File = Ref.Location.FileURI;
+ Strings.intern(File);
+ Ref.Location.FileURI = File.data();
+ }
}
}
Index: clangd/index/Merge.cpp
===================================================================
--- clangd/index/Merge.cpp
+++ clangd/index/Merge.cpp
@@ -113,8 +113,9 @@
bool PreferR = R.Definition && !L.Definition;
// Merge include headers only if both have definitions or both have no
// definition; otherwise, only accumulate references of common includes.
+ assert(L.Definition.FileURI && R.Definition.FileURI);
bool MergeIncludes =
- L.Definition.FileURI.empty() == R.Definition.FileURI.empty();
+ !std::strlen(L.Definition.FileURI) == !std::strlen(R.Definition.FileURI);
Symbol S = PreferR ? R : L; // The target symbol we're merging into.
const Symbol &O = PreferR ? L : R; // The "other" less-preferred symbol.
Index: clangd/index/Index.h
===================================================================
--- clangd/index/Index.h
+++ clangd/index/Index.h
@@ -56,14 +56,19 @@
uint32_t Column : 12; // 0-based
};
- // The URI of the source file where a symbol occurs.
- llvm::StringRef FileURI;
-
/// The symbol range, using half-open range [Start, End).
Position Start;
Position End;
- explicit operator bool() const { return !FileURI.empty(); }
+ explicit operator bool() const { return !StringRef(FileURI).empty(); }
+
+ // The URI of the source file where a symbol occurs.
+ // The string must be null-terminated.
+ //
+ // We avoid using llvm::StringRef here to save memory.
+ // WARNING: unless you know what you are doing, it is recommended to use it
+ // via llvm::StringRef.
+ const char *FileURI = "";
};
inline bool operator==(const SymbolLocation::Position &L,
const SymbolLocation::Position &R) {
@@ -76,12 +81,16 @@
std::make_tuple(R.line(), R.column());
}
inline bool operator==(const SymbolLocation &L, const SymbolLocation &R) {
- return std::tie(L.FileURI, L.Start, L.End) ==
- std::tie(R.FileURI, R.Start, R.End);
+ assert(L.FileURI && R.FileURI);
+ int Cmp = std::strcmp(L.FileURI, R.FileURI);
+ return Cmp == 0 && std::tie(L.Start, L.End) == std::tie(R.Start, R.End);
}
inline bool operator<(const SymbolLocation &L, const SymbolLocation &R) {
- return std::tie(L.FileURI, L.Start, L.End) <
- std::tie(R.FileURI, R.Start, R.End);
+ assert(L.FileURI && R.FileURI);
+ int Cmp = std::strcmp(L.FileURI, R.FileURI);
+ if (Cmp != 0)
+ return Cmp < 0;
+ return std::tie(L.Start, L.End) < std::tie(R.Start, R.End);
}
llvm::raw_ostream &operator<<(llvm::raw_ostream &, const SymbolLocation &);
@@ -288,12 +297,18 @@
template <typename Callback> void visitStrings(Symbol &S, const Callback &CB) {
CB(S.Name);
CB(S.Scope);
- CB(S.CanonicalDeclaration.FileURI);
- CB(S.Definition.FileURI);
CB(S.Signature);
CB(S.CompletionSnippetSuffix);
CB(S.Documentation);
CB(S.ReturnType);
+ auto RawCharPointerCB = [&CB](const char *&P) {
+ llvm::StringRef S(P);
+ CB(S);
+ P = S.data();
+ };
+ RawCharPointerCB(S.CanonicalDeclaration.FileURI);
+ RawCharPointerCB(S.Definition.FileURI);
+
for (auto &Include : S.IncludeHeaders)
CB(Include.IncludeHeader);
}
Index: clangd/index/Index.cpp
===================================================================
--- clangd/index/Index.cpp
+++ clangd/index/Index.cpp
@@ -165,7 +165,8 @@
void RefSlab::Builder::insert(const SymbolID &ID, const Ref &S) {
auto &M = Refs[ID];
M.push_back(S);
- M.back().Location.FileURI = UniqueStrings.save(M.back().Location.FileURI);
+ M.back().Location.FileURI =
+ UniqueStrings.save(M.back().Location.FileURI).data();
}
RefSlab RefSlab::Builder::build() && {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
