Skip to content

Commit 888b53c

Browse files
akyrtziakyrtzi
authored
Merge pull request #23 from akyrtzi/fix-unit-filtering
[Database] For ReadTransaction::getProviderFileCodeReferences() make sure to filter the units properly
2 parents 9126c26 + 65d77ac commit 888b53c

File tree

4 files changed

+69
-35
lines changed

4 files changed

+69
-35
lines changed

‎include/IndexStoreDB/Database/ReadTransaction.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,14 @@ class INDEXSTOREDB_EXPORT ReadTransaction {
4444
StringRef getModuleName(IDCode moduleName);
4545
bool getProviderFileReferences(IDCode provider,
4646
llvm::function_ref<bool(TimestampedPath path)> receiver);
47+
/// `unitFilter` returns `true` if the unit should be included, `false` if it should be ignored.
4748
bool getProviderFileCodeReferences(IDCode provider,
48-
llvm::function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver);
49+
function_ref<bool(IDCode unitCode)> unitFilter,
50+
function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver);
4951
/// Returns all provider-file associations. Intended for debugging purposes.
50-
bool foreachProviderAndFileCodeReference(llvm::function_ref<bool(IDCode provider, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver);
52+
/// `unitFilter` returns `true` if the unit should be included, `false` if it should be ignored.
53+
bool foreachProviderAndFileCodeReference(function_ref<bool(IDCode unitCode)> unitFilter,
54+
function_ref<bool(IDCode provider, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver);
5155

5256
/// Returns USR codes in batches.
5357
bool foreachUSROfGlobalSymbolKind(SymbolKind symKind, llvm::function_ref<bool(ArrayRef<IDCode> usrCodes)> receiver);

‎lib/Database/ReadTransaction.cpp

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,8 @@ StringRef ReadTransaction::Implementation::getModuleName(IDCode moduleNameCode)
121121

122122
bool ReadTransaction::Implementation::getProviderFileReferences(IDCode provider,
123123
llvm::function_ref<bool(TimestampedPath path)> receiver) {
124-
return getProviderFileCodeReferences(provider, [&](IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem) -> bool {
124+
auto unitFilter = [](IDCode unitCode)->bool { return true; };
125+
return getProviderFileCodeReferences(provider, unitFilter, [&](IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem) -> bool {
125126
std::string pathString;
126127
llvm::raw_string_ostream OS(pathString);
127128
if (!getFullFilePathFromCode(pathCode, OS)) {
@@ -138,9 +139,11 @@ bool ReadTransaction::Implementation::getProviderFileReferences(IDCode provider,
138139
});
139140
}
140141

142+
/// `unitFilter` returns `true` if the unit should be included, `false` if it should be ignored.
141143
static bool passFileReferencesForProviderCursor(lmdb::val &key,
142144
lmdb::val &value,
143145
lmdb::cursor &cursor,
146+
function_ref<bool(IDCode unitCode)> unitFilter,
144147
llvm::function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
145148
// Entries are sorted by file code and there can be multiple same file entries
146149
// from different units. We want to pass each file only once with its most recent
@@ -159,32 +162,46 @@ static bool passFileReferencesForProviderCursor(lmdb::val &key,
159162
const auto &entry = *(TimestampedFileForProviderData*)value.data();
160163
llvm::sys::TimePoint<> modTime = llvm::sys::TimePoint<>(std::chrono::nanoseconds(entry.NanoTime));
161164
if (!currFileCode) {
162-
currFileCode = entry.FileCode;
163-
currUnitCode = entry.UnitCode;
164-
currModTime = modTime;
165-
currModuleNameCode = entry.ModuleNameCode;
166-
currIsSystem = entry.IsSystem;
165+
if (unitFilter(entry.UnitCode)) {
166+
currFileCode = entry.FileCode;
167+
currUnitCode = entry.UnitCode;
168+
currModTime = modTime;
169+
currModuleNameCode = entry.ModuleNameCode;
170+
currIsSystem = entry.IsSystem;
171+
}
167172
} else if (currFileCode.getValue() == entry.FileCode) {
168173
if (currModTime < modTime) {
169-
currModTime = modTime;
170-
currUnitCode = entry.UnitCode;
174+
if (unitFilter(entry.UnitCode)) {
175+
currModTime = modTime;
176+
currUnitCode = entry.UnitCode;
177+
}
171178
}
172179
} else {
173180
if (!passCurrFile())
174181
return false;
175-
currFileCode = entry.FileCode;
176-
currUnitCode = entry.UnitCode;
177-
currModTime = modTime;
178-
currModuleNameCode = entry.ModuleNameCode;
179-
currIsSystem = entry.IsSystem;
182+
if (unitFilter(entry.UnitCode)) {
183+
currFileCode = entry.FileCode;
184+
currUnitCode = entry.UnitCode;
185+
currModTime = modTime;
186+
currModuleNameCode = entry.ModuleNameCode;
187+
currIsSystem = entry.IsSystem;
188+
} else {
189+
currFileCode.reset();
190+
currUnitCode.reset();
191+
}
180192
}
181193
} while (cursor.get(key, value, MDB_NEXT_DUP));
182194

183-
return passCurrFile();
195+
if (currFileCode) {
196+
return passCurrFile();
197+
} else {
198+
return true;
199+
}
184200
}
185201

186202
bool ReadTransaction::Implementation::getProviderFileCodeReferences(IDCode provider,
187-
llvm::function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
203+
function_ref<bool(IDCode unitCode)> unitFilter,
204+
function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
188205
auto &db = DBase->impl();
189206
auto &dbiFilesByProvider = db.getDBITimestampedFilesByProvider();
190207
auto cursor = lmdb::cursor::open(Txn, dbiFilesByProvider);
@@ -195,10 +212,12 @@ bool ReadTransaction::Implementation::getProviderFileCodeReferences(IDCode provi
195212
if (!found)
196213
return true;
197214

198-
return passFileReferencesForProviderCursor(key, value, cursor, std::move(receiver));
215+
return passFileReferencesForProviderCursor(key, value, cursor, std::move(unitFilter), std::move(receiver));
199216
}
200217

201-
bool ReadTransaction::Implementation::foreachProviderAndFileCodeReference(llvm::function_ref<bool(IDCode provider, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
218+
bool ReadTransaction::Implementation::foreachProviderAndFileCodeReference(
219+
function_ref<bool(IDCode unitCode)> unitFilter,
220+
function_ref<bool(IDCode provider, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
202221
auto &db = DBase->impl();
203222
auto &dbiFilesByProvider = db.getDBITimestampedFilesByProvider();
204223
auto cursor = lmdb::cursor::open(Txn, dbiFilesByProvider);
@@ -207,7 +226,7 @@ bool ReadTransaction::Implementation::foreachProviderAndFileCodeReference(llvm::
207226
lmdb::val value{};
208227
while (cursor.get(key, value, MDB_NEXT_NODUP)) {
209228
IDCode providerCode = *(IDCode*)key.data();
210-
bool cont = passFileReferencesForProviderCursor(key, value, cursor, [&](IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem) -> bool {
229+
bool cont = passFileReferencesForProviderCursor(key, value, cursor, unitFilter, [&](IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem) -> bool {
211230
return receiver(providerCode, pathCode, unitCode, modTime, moduleNameCode, isSystem);
212231
});
213232
if (!cont)
@@ -635,12 +654,15 @@ bool ReadTransaction::getProviderFileReferences(IDCode provider,
635654
}
636655

637656
bool ReadTransaction::getProviderFileCodeReferences(IDCode provider,
638-
llvm::function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
639-
return Impl->getProviderFileCodeReferences(provider, std::move(receiver));
657+
function_ref<bool(IDCode unitCode)> unitFilter,
658+
function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
659+
return Impl->getProviderFileCodeReferences(provider, std::move(unitFilter), std::move(receiver));
640660
}
641661

642-
bool ReadTransaction::foreachProviderAndFileCodeReference(llvm::function_ref<bool(IDCode provider, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
643-
return Impl->foreachProviderAndFileCodeReference(std::move(receiver));
662+
bool ReadTransaction::foreachProviderAndFileCodeReference(
663+
function_ref<bool(IDCode unitCode)> unitFilter,
664+
function_ref<bool(IDCode provider, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver) {
665+
return Impl->foreachProviderAndFileCodeReference(std::move(unitFilter), std::move(receiver));
644666
}
645667

646668
bool ReadTransaction::foreachUSROfGlobalSymbolKind(SymbolKind symKind, llvm::function_ref<bool(ArrayRef<IDCode> usrCodes)> receiver) {

‎lib/Database/ReadTransactionImpl.h

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,13 @@ class ReadTransaction::Implementation {
4949
StringRef getModuleName(IDCode moduleName);
5050
bool getProviderFileReferences(IDCode provider,
5151
llvm::function_ref<bool(TimestampedPath path)> receiver);
52+
/// `unitFilter` returns `true` if the unit should be included, `false` if it should be ignored.
5253
bool getProviderFileCodeReferences(IDCode provider,
53-
llvm::function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver);
54-
bool foreachProviderAndFileCodeReference(llvm::function_ref<bool(IDCode provider, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver);
54+
function_ref<bool(IDCode unitCode)> unitFilter,
55+
function_ref<bool(IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver);
56+
/// `unitFilter` returns `true` if the unit should be included, `false` if it should be ignored.
57+
bool foreachProviderAndFileCodeReference(function_ref<bool(IDCode unitCode)> unitFilter,
58+
function_ref<bool(IDCode provider, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem)> receiver);
5559

5660
bool foreachUSROfGlobalSymbolKind(SymbolKind symKind, llvm::function_ref<bool(ArrayRef<IDCode> usrCodes)> receiver);
5761
bool foreachUSROfGlobalUnitTestSymbol(llvm::function_ref<bool(ArrayRef<IDCode> usrCodes)> receiver);

‎lib/Index/SymbolIndex.cpp

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class SymbolIndexImpl {
9191
std::vector<SymbolDataProviderRef> lookupProvidersForUSR(StringRef USR, SymbolRoleSet roles, SymbolRoleSet relatedRoles);
9292
std::vector<std::pair<SymbolDataProviderRef, bool>> findCanonicalProvidersForUSR(IDCode usrCode);
9393
SymbolDataProviderRef createVisibleProviderForCode(IDCode providerCode, ReadTransaction &reader);
94-
SymbolDataProviderRef createProviderForCode(IDCode providerCode, ReadTransaction &reader, function_ref<bool(ReadTransaction &, const UnitInfo &)> unitFilter);
94+
SymbolDataProviderRef createProviderForCode(IDCode providerCode, ReadTransaction &reader, function_ref<bool(const UnitInfo &)> unitFilter);
9595
};
9696

9797
} // anonymous namespace
@@ -151,7 +151,8 @@ void SymbolIndexImpl::printStats(raw_ostream &OS) {
151151
void SymbolIndexImpl::dumpProviderFileAssociations(raw_ostream &OS) {
152152
ReadTransaction reader(DBase);
153153
Optional<IDCode> prevProvCode;
154-
reader.foreachProviderAndFileCodeReference([&](IDCode providerCode, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem) -> bool {
154+
auto unitFilter = [](IDCode unitCode)->bool { return true; };
155+
reader.foreachProviderAndFileCodeReference(unitFilter, [&](IDCode providerCode, IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem) -> bool {
155156
if (!prevProvCode.hasValue() || prevProvCode.getValue() != providerCode) {
156157
OS << reader.getProviderName(providerCode) << '\n';
157158
prevProvCode = providerCode;
@@ -166,12 +167,12 @@ void SymbolIndexImpl::dumpProviderFileAssociations(raw_ostream &OS) {
166167
}
167168

168169
SymbolDataProviderRef SymbolIndexImpl::createVisibleProviderForCode(IDCode providerCode, ReadTransaction &reader) {
169-
return createProviderForCode(providerCode, reader, [&](ReadTransaction &reader, const UnitInfo &unitInfo) -> bool {
170+
return createProviderForCode(providerCode, reader, [&](const UnitInfo &unitInfo) -> bool {
170171
return VisibilityChecker->isUnitVisible(unitInfo, reader);
171172
});
172173
}
173174

174-
SymbolDataProviderRef SymbolIndexImpl::createProviderForCode(IDCode providerCode, ReadTransaction &reader, function_ref<bool(ReadTransaction &, const UnitInfo &)> unitFilter) {
175+
SymbolDataProviderRef SymbolIndexImpl::createProviderForCode(IDCode providerCode, ReadTransaction &reader, function_ref<bool(const UnitInfo &)> unitFilter) {
175176
StringRef recordName = reader.getProviderName(providerCode);
176177
if (recordName.empty()) {
177178
++NumMissingProvidersLookedUp;
@@ -180,12 +181,15 @@ SymbolDataProviderRef SymbolIndexImpl::createProviderForCode(IDCode providerCode
180181

181182
Optional<SymbolProviderKind> providerKind;
182183
SmallVector<FileAndTarget, 8> fileRefs;
183-
reader.getProviderFileCodeReferences(providerCode, [&](IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem) -> bool {
184+
auto unitCodeFilter = [&reader, unitFilter](IDCode unitCode) -> bool {
184185
auto unitInfo = reader.getUnitInfo(unitCode);
185186
if (unitInfo.isInvalid())
186-
return true;
187-
if (!unitFilter(reader, unitInfo))
188-
return true;
187+
return false;
188+
return unitFilter(reader.getUnitInfo(unitCode));
189+
};
190+
reader.getProviderFileCodeReferences(providerCode, unitCodeFilter, [&](IDCode pathCode, IDCode unitCode, llvm::sys::TimePoint<> modTime, IDCode moduleNameCode, bool isSystem) -> bool {
191+
auto unitInfo = reader.getUnitInfo(unitCode);
192+
assert(!unitInfo.isInvalid());
189193

190194
if (!providerKind.hasValue()) {
191195
providerKind = unitInfo.SymProviderKind;
@@ -483,7 +487,7 @@ bool SymbolIndexImpl::foreachUnitTestSymbolReferencedByOutputPaths(ArrayRef<Cano
483487
outFileCodes.insert(reader.getFilePathCode(path));
484488
}
485489
for (IDCode providerCode : providerCodes) {
486-
auto provider = createProviderForCode(providerCode, reader, [&](ReadTransaction &reader, const UnitInfo &unitInfo) -> bool {
490+
auto provider = createProviderForCode(providerCode, reader, [&](const UnitInfo &unitInfo) -> bool {
487491
return outFileCodes.count(unitInfo.OutFileCode);
488492
});
489493
if (provider) {

0 commit comments

Comments
 (0)