Skip to content

Commit e31ab98

Browse files
committed
[Modules] Record side effect info in EvaluatedStmt (llvm#146468)
All deserialized VarDecl initializers are EvaluatedStmt, but not all EvaluatedStmt initializers are from a PCH. Calling `VarDecl::hasInitWithSideEffects` can trigger constant evaluation, but it's hard to know ahead of time whether that will trigger deserialization - even if the initializer is fully deserialized, it may contain a call to a constructor whose body is not deserialized. By caching the result of `VarDecl::hasInitWithSideEffects` and populating that cache during deserialization we can guarantee that calling it won't trigger deserialization regardless of the state of the initializer. This also reduces memory usage by removing the `InitSideEffectVars` set in `ASTReader`. rdar://154717930 (cherry picked from commit 2910c24)
1 parent 739191e commit e31ab98

File tree

11 files changed

+77
-63
lines changed

11 files changed

+77
-63
lines changed

clang/include/clang/AST/Decl.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -874,13 +874,17 @@ struct EvaluatedStmt {
874874
bool HasICEInit : 1;
875875
bool CheckedForICEInit : 1;
876876

877+
bool HasSideEffects : 1;
878+
bool CheckedForSideEffects : 1;
879+
877880
LazyDeclStmtPtr Value;
878881
APValue Evaluated;
879882

880883
EvaluatedStmt()
881884
: WasEvaluated(false), IsEvaluating(false),
882885
HasConstantInitialization(false), HasConstantDestruction(false),
883-
HasICEInit(false), CheckedForICEInit(false) {}
886+
HasICEInit(false), CheckedForICEInit(false), HasSideEffects(false),
887+
CheckedForSideEffects(false) {}
884888
};
885889

886890
/// Represents a variable declaration or definition.
@@ -1339,9 +1343,11 @@ class VarDecl : public DeclaratorDecl, public Redeclarable<VarDecl> {
13391343
return const_cast<VarDecl *>(this)->getInitializingDeclaration();
13401344
}
13411345

1342-
/// Checks whether this declaration has an initializer with side effects,
1343-
/// without triggering deserialization if the initializer is not yet
1344-
/// deserialized.
1346+
/// Checks whether this declaration has an initializer with side effects.
1347+
/// The result is cached. If the result hasn't been computed this can trigger
1348+
/// deserialization and constant evaluation. By running this during
1349+
/// serialization and serializing the result all clients can safely call this
1350+
/// without triggering further deserialization.
13451351
bool hasInitWithSideEffects() const;
13461352

13471353
/// Determine whether this variable's value might be usable in a

clang/include/clang/AST/ExternalASTSource.h

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -174,10 +174,6 @@ class ExternalASTSource : public RefCountedBase<ExternalASTSource> {
174174

175175
virtual ExtKind hasExternalDefinitions(const Decl *D);
176176

177-
virtual bool hasInitializerWithSideEffects(const VarDecl *VD) const {
178-
return false;
179-
}
180-
181177
/// Finds all declarations lexically contained within the given
182178
/// DeclContext, after applying an optional filter predicate.
183179
///
@@ -412,17 +408,6 @@ struct LazyOffsetPtr {
412408
return GetPtr();
413409
}
414410

415-
/// Retrieve the pointer to the AST node that this lazy pointer points to,
416-
/// if it can be done without triggering deserialization.
417-
///
418-
/// \returns a pointer to the AST node, or null if not yet deserialized.
419-
T *getWithoutDeserializing() const {
420-
if (isOffset()) {
421-
return nullptr;
422-
}
423-
return GetPtr();
424-
}
425-
426411
/// Retrieve the address of the AST node pointer. Deserializes the pointee if
427412
/// necessary.
428413
T **getAddressOfPointer(ExternalASTSource *Source) const {

clang/include/clang/Sema/MultiplexExternalSemaSource.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,6 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
9292

9393
ExtKind hasExternalDefinitions(const Decl *D) override;
9494

95-
bool hasInitializerWithSideEffects(const VarDecl *VD) const override;
96-
9795
/// Find all declarations with the given name in the
9896
/// given context.
9997
bool FindExternalVisibleDeclsByName(const DeclContext *DC,

clang/include/clang/Serialization/ASTReader.h

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1348,12 +1348,6 @@ class ASTReader
13481348
const StringRef &operator*() && = delete;
13491349
};
13501350

1351-
/// VarDecls with initializers containing side effects must be emitted,
1352-
/// but DeclMustBeEmitted is not allowed to deserialize the intializer.
1353-
/// FIXME: Lower memory usage by removing VarDecls once the initializer
1354-
/// is deserialized.
1355-
llvm::SmallPtrSet<Decl *, 16> InitSideEffectVars;
1356-
13571351
public:
13581352
/// Get the buffer for resolving paths.
13591353
SmallString<0> &getPathBuf() { return PathBuf; }
@@ -2269,8 +2263,6 @@ class ASTReader
22692263

22702264
ExtKind hasExternalDefinitions(const Decl *D) override;
22712265

2272-
bool hasInitializerWithSideEffects(const VarDecl *VD) const override;
2273-
22742266
/// Retrieve a selector from the given module with its local ID
22752267
/// number.
22762268
Selector getLocalSelector(ModuleFile &M, unsigned LocalID);

clang/lib/AST/Decl.cpp

Lines changed: 10 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -2444,24 +2444,16 @@ bool VarDecl::hasInitWithSideEffects() const {
24442444
if (!hasInit())
24452445
return false;
24462446

2447-
// Check if we can get the initializer without deserializing
2448-
const Expr *E = nullptr;
2449-
if (auto *S = dyn_cast<Stmt *>(Init)) {
2450-
E = cast<Expr>(S);
2451-
} else {
2452-
E = cast_or_null<Expr>(getEvaluatedStmt()->Value.getWithoutDeserializing());
2453-
}
2454-
2455-
if (E)
2456-
return E->HasSideEffects(getASTContext()) &&
2457-
// We can get a value-dependent initializer during error recovery.
2458-
(E->isValueDependent() || !evaluateValue());
2459-
2460-
assert(getEvaluatedStmt()->Value.isOffset());
2461-
// ASTReader tracks this without having to deserialize the initializer
2462-
if (auto Source = getASTContext().getExternalSource())
2463-
return Source->hasInitializerWithSideEffects(this);
2464-
return false;
2447+
EvaluatedStmt *ES = ensureEvaluatedStmt();
2448+
if (!ES->CheckedForSideEffects) {
2449+
const Expr *E = getInit();
2450+
ES->HasSideEffects =
2451+
E->HasSideEffects(getASTContext()) &&
2452+
// We can get a value-dependent initializer during error recovery.
2453+
(E->isValueDependent() || !evaluateValue());
2454+
ES->CheckedForSideEffects = true;
2455+
}
2456+
return ES->HasSideEffects;
24652457
}
24662458

24672459
bool VarDecl::isOutOfLine() const {

clang/lib/Sema/MultiplexExternalSemaSource.cpp

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -107,14 +107,6 @@ MultiplexExternalSemaSource::hasExternalDefinitions(const Decl *D) {
107107
return EK_ReplyHazy;
108108
}
109109

110-
bool MultiplexExternalSemaSource::hasInitializerWithSideEffects(
111-
const VarDecl *VD) const {
112-
for (const auto &S : Sources)
113-
if (S->hasInitializerWithSideEffects(VD))
114-
return true;
115-
return false;
116-
}
117-
118110
bool MultiplexExternalSemaSource::
119111
FindExternalVisibleDeclsByName(const DeclContext *DC, DeclarationName Name) {
120112
bool AnyDeclsFound = false;

clang/lib/Serialization/ASTReader.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9268,10 +9268,6 @@ ExternalASTSource::ExtKind ASTReader::hasExternalDefinitions(const Decl *FD) {
92689268
return I->second ? EK_Never : EK_Always;
92699269
}
92709270

9271-
bool ASTReader::hasInitializerWithSideEffects(const VarDecl *VD) const {
9272-
return InitSideEffectVars.count(VD);
9273-
}
9274-
92759271
Selector ASTReader::getLocalSelector(ModuleFile &M, unsigned LocalID) {
92769272
return DecodeSelector(getGlobalSelectorID(M, LocalID));
92779273
}

clang/lib/Serialization/ASTReaderDecl.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1622,9 +1622,6 @@ ASTDeclReader::RedeclarableResult ASTDeclReader::VisitVarDeclImpl(VarDecl *VD) {
16221622
VD->NonParmVarDeclBits.PreviousDeclInSameBlockScope =
16231623
VarDeclBits.getNextBit();
16241624

1625-
if (VarDeclBits.getNextBit())
1626-
Reader.InitSideEffectVars.insert(VD);
1627-
16281625
VD->NonParmVarDeclBits.EscapingByref = VarDeclBits.getNextBit();
16291626
HasDeducedType = VarDeclBits.getNextBit();
16301627
VD->NonParmVarDeclBits.ImplicitParamKind =
@@ -1695,6 +1692,8 @@ void ASTDeclReader::ReadVarDeclInit(VarDecl *VD) {
16951692
Eval->HasConstantInitialization = (Val & 2) != 0;
16961693
Eval->HasConstantDestruction = (Val & 4) != 0;
16971694
Eval->WasEvaluated = (Val & 8) != 0;
1695+
Eval->HasSideEffects = (Val & 16) != 0;
1696+
Eval->CheckedForSideEffects = true;
16981697
if (Eval->WasEvaluated) {
16991698
Eval->Evaluated = Record.readAPValue();
17001699
if (Eval->Evaluated.needsCleanup())

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6834,6 +6834,10 @@ void ASTRecordWriter::AddVarDeclInit(const VarDecl *VD) {
68346834

68356835
uint64_t Val = 1;
68366836
if (EvaluatedStmt *ES = VD->getEvaluatedStmt()) {
6837+
// This may trigger evaluation, so run it first
6838+
if (VD->hasInitWithSideEffects())
6839+
Val |= 16;
6840+
assert(ES->CheckedForSideEffects);
68376841
Val |= (ES->HasConstantInitialization ? 2 : 0);
68386842
Val |= (ES->HasConstantDestruction ? 4 : 0);
68396843
APValue *Evaluated = VD->getEvaluatedValue();

clang/lib/Serialization/ASTWriterDecl.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1158,7 +1158,6 @@ void ASTDeclWriter::VisitVarDecl(VarDecl *D) {
11581158
VarDeclBits.addBit(D->isConstexpr());
11591159
VarDeclBits.addBit(D->isInitCapture());
11601160
VarDeclBits.addBit(D->isPreviousDeclInSameBlockScope());
1161-
VarDeclBits.addBit(D->hasInitWithSideEffects());
11621161

11631162
VarDeclBits.addBit(D->isEscapingByref());
11641163
HasDeducedType = D->getType()->getContainedDeducedType();

0 commit comments

Comments
 (0)