teemperor created this revision.
As discussed in https://reviews.llvm.org/D30793, we have some unsafe calls to
`isConsumerInterestedIn()`. This patch implements Richard's suggestion (from
the inline comment) that we should track if we just deserialized an
declaration. If we just deserialized, we can skip the unsafe call because we
know it's interesting. If we didn't just deserialize the declaration, calling
`isConsumerInterestedIn()` should be safe.
https://reviews.llvm.org/D32499
Files:
include/clang/Serialization/ASTReader.h
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTReaderDecl.cpp
Index: lib/Serialization/ASTReaderDecl.cpp
===
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -3612,7 +3612,8 @@
assert(Record.getIdx() == Record.size());
// Load any relevant update records.
- PendingUpdateRecords.push_back(std::make_pair(ID, D));
+ PendingUpdateRecords.push_back(
+ PendingUpdateRecord(ID, D, /*JustLoaded=*/true));
// Load the categories after recursive loading is finished.
if (ObjCInterfaceDecl *Class = dyn_cast(D))
@@ -3657,20 +3658,24 @@
}
}
-void ASTReader::loadDeclUpdateRecords(serialization::DeclID ID, Decl *D) {
+void ASTReader::loadDeclUpdateRecords(PendingUpdateRecord ) {
// The declaration may have been modified by files later in the chain.
// If this is the case, read the record containing the updates from each file
// and pass it to ASTDeclReader to make the modifications.
+ serialization::GlobalDeclID ID = Record.ID;
+ Decl *D = Record.D;
ProcessingUpdatesRAIIObj ProcessingUpdates(*this);
DeclUpdateOffsetsMap::iterator UpdI = DeclUpdateOffsets.find(ID);
if (UpdI != DeclUpdateOffsets.end()) {
auto UpdateOffsets = std::move(UpdI->second);
DeclUpdateOffsets.erase(UpdI);
-// FIXME: This call to isConsumerInterestedIn is not safe because
-// we could be deserializing declarations at the moment. We should
-// delay calling this in the same way as done in D30793.
-bool WasInteresting = isConsumerInterestedIn(Context, D, false);
+// Check if this decl was interesting to the consumer. If we just loaded
+// the declaration, then we know it was interesting and we skip the call
+// to isConsumerInterestedIn because it is unsafe to call in the
+// current ASTReader state.
+bool WasInteresting =
+Record.JustLoaded || isConsumerInterestedIn(Context, D, false);
for (auto : UpdateOffsets) {
ModuleFile *F = FileAndOffset.first;
uint64_t Offset = FileAndOffset.second;
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -2757,7 +2757,8 @@
// If we've already loaded the decl, perform the updates when we finish
// loading this block.
if (Decl *D = GetExistingDecl(ID))
-PendingUpdateRecords.push_back(std::make_pair(ID, D));
+PendingUpdateRecords.push_back(
+PendingUpdateRecord(ID, D, /*JustLoaded=*/false));
break;
}
@@ -3087,7 +3088,8 @@
// If we've already loaded the decl, perform the updates when we finish
// loading this block.
if (Decl *D = GetExistingDecl(ID))
- PendingUpdateRecords.push_back(std::make_pair(ID, D));
+ PendingUpdateRecords.push_back(
+ PendingUpdateRecord(ID, D, /*JustLoaded=*/false));
}
break;
}
@@ -8940,7 +8942,7 @@
while (!PendingUpdateRecords.empty()) {
auto Update = PendingUpdateRecords.pop_back_val();
ReadingKindTracker ReadingKind(Read_Decl, *this);
- loadDeclUpdateRecords(Update.first, Update.second);
+ loadDeclUpdateRecords(Update);
}
}
Index: include/clang/Serialization/ASTReader.h
===
--- include/clang/Serialization/ASTReader.h
+++ include/clang/Serialization/ASTReader.h
@@ -478,10 +478,18 @@
/// in the chain.
DeclUpdateOffsetsMap DeclUpdateOffsets;
+ struct PendingUpdateRecord {
+Decl *D;
+serialization::GlobalDeclID ID;
+// Whether the declaration was just deserialized.
+bool JustLoaded;
+PendingUpdateRecord(serialization::GlobalDeclID ID, Decl *D,
+bool JustLoaded)
+: D(D), ID(ID), JustLoaded(JustLoaded) {}
+ };
/// \brief Declaration updates for already-loaded declarations that we need
/// to apply once we finish processing an import.
- llvm::SmallVector, 16>
- PendingUpdateRecords;
+ llvm::SmallVector PendingUpdateRecords;
enum class PendingFakeDefinitionKind { NotFake, Fake, FakeLoaded };
@@ -1279,7 +1287,7 @@
RecordLocation DeclCursorForID(serialization::DeclID ID,
SourceLocation
