[clang] 3242934 - Lazily deserialize default member initializers.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 5 15:46:24 PDT 2023
Author: Richard Smith
Date: 2023-04-05T15:46:13-07:00
New Revision: 32429341910d0a48336215be13c7c9140dd26da4
URL: https://github.com/llvm/llvm-project/commit/32429341910d0a48336215be13c7c9140dd26da4
DIFF: https://github.com/llvm/llvm-project/commit/32429341910d0a48336215be13c7c9140dd26da4.diff
LOG: Lazily deserialize default member initializers.
This is important to break deserialization cycles, where a lambda in a
default member initializer can refer to the field as its context
declaration, and the initializer of the field can refer back to the
lambda.
This is a follow-up to bc73ef0031b5, which applied the same fix to
variable declarations for the same reason.
Added:
Modified:
clang/include/clang/AST/Decl.h
clang/lib/AST/Decl.cpp
clang/lib/Serialization/ASTCommon.cpp
clang/lib/Serialization/ASTReaderDecl.cpp
clang/lib/Serialization/ASTWriterDecl.cpp
clang/test/Modules/merge-lambdas.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/AST/Decl.h b/clang/include/clang/AST/Decl.h
index fde957320ab4b..650baf56414c7 100644
--- a/clang/include/clang/AST/Decl.h
+++ b/clang/include/clang/AST/Decl.h
@@ -2943,11 +2943,7 @@ class FunctionDecl : public DeclaratorDecl,
/// Represents a member of a struct/union/class.
class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
- unsigned BitField : 1;
- unsigned Mutable : 1;
- mutable unsigned CachedFieldIndex : 30;
-
- /// The kinds of value we can store in InitializerOrBitWidth.
+ /// The kinds of value we can store in StorageKind.
///
/// Note that this is compatible with InClassInitStyle except for
/// ISK_CapturedVLAType.
@@ -2970,10 +2966,15 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
ISK_CapturedVLAType,
};
+ unsigned BitField : 1;
+ unsigned Mutable : 1;
+ unsigned StorageKind : 2;
+ mutable unsigned CachedFieldIndex : 28;
+
/// If this is a bitfield with a default member initializer, this
/// structure is used to represent the two expressions.
- struct InitAndBitWidth {
- Expr *Init;
+ struct InitAndBitWidthStorage {
+ LazyDeclStmtPtr Init;
Expr *BitWidth;
};
@@ -2986,16 +2987,25 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
/// and attached.
// FIXME: Tail-allocate this to reduce the size of FieldDecl in the
// overwhelmingly common case that we have none of these things.
- llvm::PointerIntPair<void *, 2, InitStorageKind> InitStorage;
+ union {
+ // Active member if ISK is not ISK_CapturedVLAType and BitField is false.
+ LazyDeclStmtPtr Init;
+ // Active member if ISK is ISK_NoInit and BitField is true.
+ Expr *BitWidth;
+ // Active member if ISK is ISK_InClass*Init and BitField is true.
+ InitAndBitWidthStorage *InitAndBitWidth;
+ // Active member if ISK is ISK_CapturedVLAType.
+ const VariableArrayType *CapturedVLAType;
+ };
protected:
FieldDecl(Kind DK, DeclContext *DC, SourceLocation StartLoc,
- SourceLocation IdLoc, IdentifierInfo *Id,
- QualType T, TypeSourceInfo *TInfo, Expr *BW, bool Mutable,
+ SourceLocation IdLoc, IdentifierInfo *Id, QualType T,
+ TypeSourceInfo *TInfo, Expr *BW, bool Mutable,
InClassInitStyle InitStyle)
- : DeclaratorDecl(DK, DC, IdLoc, Id, T, TInfo, StartLoc),
- BitField(false), Mutable(Mutable), CachedFieldIndex(0),
- InitStorage(nullptr, (InitStorageKind) InitStyle) {
+ : DeclaratorDecl(DK, DC, IdLoc, Id, T, TInfo, StartLoc), BitField(false),
+ Mutable(Mutable), StorageKind((InitStorageKind)InitStyle),
+ CachedFieldIndex(0), Init() {
if (BW)
setBitWidth(BW);
}
@@ -3034,10 +3044,7 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
Expr *getBitWidth() const {
if (!BitField)
return nullptr;
- void *Ptr = InitStorage.getPointer();
- if (getInClassInitStyle())
- return static_cast<InitAndBitWidth*>(Ptr)->BitWidth;
- return static_cast<Expr*>(Ptr);
+ return hasInClassInitializer() ? InitAndBitWidth->BitWidth : BitWidth;
}
unsigned getBitWidthValue(const ASTContext &Ctx) const;
@@ -3048,11 +3055,11 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
assert(!hasCapturedVLAType() && !BitField &&
"bit width or captured type already set");
assert(Width && "no bit width specified");
- InitStorage.setPointer(
- InitStorage.getInt()
- ? new (getASTContext())
- InitAndBitWidth{getInClassInitializer(), Width}
- : static_cast<void*>(Width));
+ if (hasInClassInitializer())
+ InitAndBitWidth =
+ new (getASTContext()) InitAndBitWidthStorage{Init, Width};
+ else
+ BitWidth = Width;
BitField = true;
}
@@ -3060,7 +3067,11 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
// Note: used by some clients (i.e., do not remove it).
void removeBitWidth() {
assert(isBitField() && "no bitfield width to remove");
- InitStorage.setPointer(getInClassInitializer());
+ if (hasInClassInitializer()) {
+ // Read the old initializer before we change the active union member.
+ auto ExistingInit = InitAndBitWidth->Init;
+ Init = ExistingInit;
+ }
BitField = false;
}
@@ -3080,9 +3091,8 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
/// Get the kind of (C++11) default member initializer that this field has.
InClassInitStyle getInClassInitStyle() const {
- InitStorageKind storageKind = InitStorage.getInt();
- return (storageKind == ISK_CapturedVLAType
- ? ICIS_NoInit : (InClassInitStyle) storageKind);
+ return (StorageKind == ISK_CapturedVLAType ? ICIS_NoInit
+ : (InClassInitStyle)StorageKind);
}
/// Determine whether this member has a C++11 default member initializer.
@@ -3090,44 +3100,44 @@ class FieldDecl : public DeclaratorDecl, public Mergeable<FieldDecl> {
return getInClassInitStyle() != ICIS_NoInit;
}
+ /// Determine whether getInClassInitializer() would return a non-null pointer
+ /// without deserializing the initializer.
+ bool hasNonNullInClassInitializer() const {
+ return hasInClassInitializer() && (BitField ? InitAndBitWidth->Init : Init);
+ }
+
/// Get the C++11 default member initializer for this member, or null if one
/// has not been set. If a valid declaration has a default member initializer,
/// but this returns null, then we have not parsed and attached it yet.
- Expr *getInClassInitializer() const {
- if (!hasInClassInitializer())
- return nullptr;
- void *Ptr = InitStorage.getPointer();
- if (BitField)
- return static_cast<InitAndBitWidth*>(Ptr)->Init;
- return static_cast<Expr*>(Ptr);
- }
+ Expr *getInClassInitializer() const;
/// Set the C++11 in-class initializer for this member.
- void setInClassInitializer(Expr *Init) {
- assert(hasInClassInitializer() && !getInClassInitializer());
- if (BitField)
- static_cast<InitAndBitWidth*>(InitStorage.getPointer())->Init = Init;
- else
- InitStorage.setPointer(Init);
- }
+ void setInClassInitializer(Expr *NewInit);
+
+private:
+ void setLazyInClassInitializer(LazyDeclStmtPtr NewInit);
+public:
/// Remove the C++11 in-class initializer from this member.
void removeInClassInitializer() {
assert(hasInClassInitializer() && "no initializer to remove");
- InitStorage.setPointerAndInt(getBitWidth(), ISK_NoInit);
+ StorageKind = ISK_NoInit;
+ if (BitField) {
+ // Read the bit width before we change the active union member.
+ Expr *ExistingBitWidth = InitAndBitWidth->BitWidth;
+ BitWidth = ExistingBitWidth;
+ }
}
/// Determine whether this member captures the variable length array
/// type.
bool hasCapturedVLAType() const {
- return InitStorage.getInt() == ISK_CapturedVLAType;
+ return StorageKind == ISK_CapturedVLAType;
}
/// Get the captured variable length array type.
const VariableArrayType *getCapturedVLAType() const {
- return hasCapturedVLAType() ? static_cast<const VariableArrayType *>(
- InitStorage.getPointer())
- : nullptr;
+ return hasCapturedVLAType() ? CapturedVLAType : nullptr;
}
/// Set the captured variable length array type for this field.
diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp
index de600e3b8aeda..cbee553715f5e 100644
--- a/clang/lib/AST/Decl.cpp
+++ b/clang/lib/AST/Decl.cpp
@@ -4345,6 +4345,28 @@ bool FieldDecl::isAnonymousStructOrUnion() const {
return false;
}
+Expr *FieldDecl::getInClassInitializer() const {
+ if (!hasInClassInitializer())
+ return nullptr;
+
+ LazyDeclStmtPtr InitPtr = BitField ? InitAndBitWidth->Init : Init;
+ return cast_or_null<Expr>(
+ InitPtr.isOffset() ? InitPtr.get(getASTContext().getExternalSource())
+ : InitPtr.get(nullptr));
+}
+
+void FieldDecl::setInClassInitializer(Expr *NewInit) {
+ setLazyInClassInitializer(LazyDeclStmtPtr(NewInit));
+}
+
+void FieldDecl::setLazyInClassInitializer(LazyDeclStmtPtr NewInit) {
+ assert(hasInClassInitializer() && !getInClassInitializer());
+ if (BitField)
+ InitAndBitWidth->Init = NewInit;
+ else
+ Init = NewInit;
+}
+
unsigned FieldDecl::getBitWidthValue(const ASTContext &Ctx) const {
assert(isBitField() && "not a bitfield");
return getBitWidth()->EvaluateKnownConstInt(Ctx).getZExtValue();
@@ -4405,6 +4427,8 @@ unsigned FieldDecl::getFieldIndex() const {
for (auto *Field : RD->fields()) {
Field->getCanonicalDecl()->CachedFieldIndex = Index + 1;
+ assert(Field->getCanonicalDecl()->CachedFieldIndex == Index + 1 &&
+ "overflow in field numbering");
++Index;
}
@@ -4424,11 +4448,11 @@ SourceRange FieldDecl::getSourceRange() const {
void FieldDecl::setCapturedVLAType(const VariableArrayType *VLAType) {
assert((getParent()->isLambda() || getParent()->isCapturedRecord()) &&
"capturing type in non-lambda or captured record.");
- assert(InitStorage.getInt() == ISK_NoInit &&
- InitStorage.getPointer() == nullptr &&
- "bit width, initializer or captured type already set");
- InitStorage.setPointerAndInt(const_cast<VariableArrayType *>(VLAType),
- ISK_CapturedVLAType);
+ assert(StorageKind == ISK_NoInit && !BitField &&
+ "bit-field or field with default member initializer cannot capture "
+ "VLA type");
+ StorageKind = ISK_CapturedVLAType;
+ CapturedVLAType = VLAType;
}
//===----------------------------------------------------------------------===//
diff --git a/clang/lib/Serialization/ASTCommon.cpp b/clang/lib/Serialization/ASTCommon.cpp
index 252cdbb96384a..72e5821074809 100644
--- a/clang/lib/Serialization/ASTCommon.cpp
+++ b/clang/lib/Serialization/ASTCommon.cpp
@@ -484,8 +484,7 @@ bool serialization::needsAnonymousDeclarationNumber(const NamedDecl *D) {
}
// Otherwise, we only care about anonymous class members / block-scope decls.
- // FIXME: We need to handle lambdas and blocks within inline / templated
- // variables too.
+ // FIXME: We need to handle blocks within inline / templated variables too.
if (D->getDeclName())
return false;
if (!isa<RecordDecl, ObjCInterfaceDecl>(D->getLexicalDeclContext()))
diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp
index d9cd97bbb7fe7..ee100824d65d1 100644
--- a/clang/lib/Serialization/ASTReaderDecl.cpp
+++ b/clang/lib/Serialization/ASTReaderDecl.cpp
@@ -566,13 +566,14 @@ void ASTDeclReader::Visit(Decl *D) {
ID->TypeForDecl = Reader.GetType(DeferredTypeID).getTypePtrOrNull();
} else if (auto *FD = dyn_cast<FunctionDecl>(D)) {
// FunctionDecl's body was written last after all other Stmts/Exprs.
- // We only read it if FD doesn't already have a body (e.g., from another
- // module).
- // FIXME: Can we diagnose ODR violations somehow?
if (Record.readInt())
ReadFunctionDefinition(FD);
} else if (auto *VD = dyn_cast<VarDecl>(D)) {
ReadVarDeclInit(VD);
+ } else if (auto *FD = dyn_cast<FieldDecl>(D)) {
+ if (FD->hasInClassInitializer() && Record.readInt()) {
+ FD->setLazyInClassInitializer(LazyDeclStmtPtr(GetCurrentCursorOffset()));
+ }
}
}
@@ -1501,15 +1502,13 @@ void ASTDeclReader::VisitFieldDecl(FieldDecl *FD) {
VisitDeclaratorDecl(FD);
FD->Mutable = Record.readInt();
- if (auto ISK = static_cast<FieldDecl::InitStorageKind>(Record.readInt())) {
- FD->InitStorage.setInt(ISK);
- FD->InitStorage.setPointer(ISK == FieldDecl::ISK_CapturedVLAType
- ? Record.readType().getAsOpaquePtr()
- : Record.readExpr());
- }
-
- if (auto *BW = Record.readExpr())
- FD->setBitWidth(BW);
+ unsigned Bits = Record.readInt();
+ FD->StorageKind = Bits >> 1;
+ if (FD->StorageKind == FieldDecl::ISK_CapturedVLAType)
+ FD->CapturedVLAType =
+ cast<VariableArrayType>(Record.readType().getTypePtr());
+ else if (Bits & 1)
+ FD->setBitWidth(Record.readExpr());
if (!FD->getDeclName()) {
if (auto *Tmpl = readDeclAs<FieldDecl>())
@@ -4464,7 +4463,7 @@ void ASTDeclReader::UpdateDecl(Decl *D,
// Only apply the update if the field still has an uninstantiated
// default member initializer.
- if (FD->hasInClassInitializer() && !FD->getInClassInitializer()) {
+ if (FD->hasInClassInitializer() && !FD->hasNonNullInClassInitializer()) {
if (DefaultInit)
FD->setInClassInitializer(DefaultInit);
else
diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp
index 7540836841983..abb6bb747bfa9 100644
--- a/clang/lib/Serialization/ASTWriterDecl.cpp
+++ b/clang/lib/Serialization/ASTWriterDecl.cpp
@@ -300,6 +300,20 @@ void ASTDeclWriter::Visit(Decl *D) {
Record.AddVarDeclInit(VD);
}
+ // And similarly for FieldDecls. We already serialized whether there is a
+ // default member initializer.
+ if (auto *FD = dyn_cast<FieldDecl>(D)) {
+ if (FD->hasInClassInitializer()) {
+ if (Expr *Init = FD->getInClassInitializer()) {
+ Record.push_back(1);
+ Record.AddStmt(Init);
+ } else {
+ Record.push_back(0);
+ // Initializer has not been instantiated yet.
+ }
+ }
+ }
+
// If this declaration is also a DeclContext, write blocks for the
// declarations that lexically stored inside its context and those
// declarations that are visible from its context.
@@ -967,14 +981,11 @@ void ASTDeclWriter::VisitFieldDecl(FieldDecl *D) {
VisitDeclaratorDecl(D);
Record.push_back(D->isMutable());
- FieldDecl::InitStorageKind ISK = D->InitStorage.getInt();
- Record.push_back(ISK);
- if (ISK == FieldDecl::ISK_CapturedVLAType)
+ Record.push_back((D->StorageKind << 1) | D->BitField);
+ if (D->StorageKind == FieldDecl::ISK_CapturedVLAType)
Record.AddTypeRef(QualType(D->getCapturedVLAType(), 0));
- else if (ISK)
- Record.AddStmt(D->getInClassInitializer());
-
- Record.AddStmt(D->getBitWidth());
+ else if (D->BitField)
+ Record.AddStmt(D->getBitWidth());
if (!D->getDeclName())
Record.AddDeclRef(Context.getInstantiatedFromUnnamedFieldDecl(D));
@@ -2044,7 +2055,7 @@ void ASTWriter::WriteDeclAbbrevs() {
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // TSIType
// FieldDecl
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // isMutable
- Abv->Add(BitCodeAbbrevOp(0)); // InitStyle
+ Abv->Add(BitCodeAbbrevOp(0)); // StorageKind
// Type Source Info
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));
Abv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // TypeLoc
diff --git a/clang/test/Modules/merge-lambdas.cpp b/clang/test/Modules/merge-lambdas.cpp
index da10ec1199853..113a1fbac638c 100644
--- a/clang/test/Modules/merge-lambdas.cpp
+++ b/clang/test/Modules/merge-lambdas.cpp
@@ -17,6 +17,23 @@ module B {}
inline auto x1() { return f<int>(); }
inline auto z() { return []{}; }
inline auto x2() { return z(); }
+
+struct Function {
+ template<typename T>
+ Function(T t) : p(new T((T&&)t)) {}
+
+ void *p;
+};
+
+struct Outer {
+ struct Inner {
+ Inner() {}
+ Function f = []{};
+ };
+ Outer(Inner = Inner());
+};
+
+inline void use_nested_1() { Outer o; }
#pragma clang module end
#pragma clang module endbuild
@@ -30,6 +47,23 @@ inline auto z() { return []{}; }
inline auto y2() { return z(); }
inline auto q() { return []{}; }
inline auto y3() { return q(); }
+
+struct Function {
+ template<typename T>
+ Function(T t) : p(new T((T&&)t)) {}
+
+ void *p;
+};
+
+struct Outer {
+ struct Inner {
+ Inner() {}
+ Function f = []{};
+ };
+ Outer(Inner = Inner());
+};
+
+inline void use_nested_2() { Outer o; }
#pragma clang module end
#pragma clang module endbuild
@@ -49,3 +83,8 @@ using V = decltype(y3);
#pragma clang module import A
void (*p)() = f<int>();
+
+void use_nested() {
+ use_nested_1();
+ use_nested_2();
+}
More information about the cfe-commits
mailing list