[clang] 15f0272 - [clang][Interp] Improve support for virtual bases
Timm Bäder via cfe-commits
cfe-commits at lists.llvm.org
Fri Apr 26 02:12:48 PDT 2024
Author: Timm Bäder
Date: 2024-04-26T11:10:45+02:00
New Revision: 15f02723d49be9a828fbf072966a225babd60457
URL: https://github.com/llvm/llvm-project/commit/15f02723d49be9a828fbf072966a225babd60457
DIFF: https://github.com/llvm/llvm-project/commit/15f02723d49be9a828fbf072966a225babd60457.diff
LOG: [clang][Interp] Improve support for virtual bases
Fix initializing virtual bases. We only consider their base size,
not their virtual size because they get flattened into the Record
hierarchy when we create them. Fix that and also virtual
derived-to-base casts.
Added:
Modified:
clang/lib/AST/Interp/ByteCodeExprGen.cpp
clang/lib/AST/Interp/ByteCodeStmtGen.cpp
clang/lib/AST/Interp/Descriptor.cpp
clang/lib/AST/Interp/Interp.h
clang/test/AST/Interp/cxx23.cpp
clang/test/AST/Interp/records.cpp
Removed:
################################################################################
diff --git a/clang/lib/AST/Interp/ByteCodeExprGen.cpp b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
index bbd2771d37124c..588ffa55c11e61 100644
--- a/clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -110,10 +110,29 @@ bool ByteCodeExprGen<Emitter>::VisitCastExpr(const CastExpr *CE) {
if (!this->visit(SubExpr))
return false;
- unsigned DerivedOffset =
- collectBaseOffset(CE->getType(), SubExpr->getType());
+ const auto extractRecordDecl = [](QualType Ty) -> const CXXRecordDecl * {
+ if (const auto *PT = dyn_cast<PointerType>(Ty))
+ return PT->getPointeeType()->getAsCXXRecordDecl();
+ return Ty->getAsCXXRecordDecl();
+ };
- return this->emitGetPtrBasePop(DerivedOffset, CE);
+ // FIXME: We can express a series of non-virtual casts as a single
+ // GetPtrBasePop op.
+ QualType CurType = SubExpr->getType();
+ for (const CXXBaseSpecifier *B : CE->path()) {
+ if (B->isVirtual()) {
+ if (!this->emitGetPtrVirtBasePop(extractRecordDecl(B->getType()), CE))
+ return false;
+ CurType = B->getType();
+ } else {
+ unsigned DerivedOffset = collectBaseOffset(B->getType(), CurType);
+ if (!this->emitGetPtrBasePop(DerivedOffset, CE))
+ return false;
+ CurType = B->getType();
+ }
+ }
+
+ return true;
}
case CK_BaseToDerived: {
diff --git a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
index 36dab6252ece67..9b8e64f1138559 100644
--- a/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
+++ b/clang/lib/AST/Interp/ByteCodeStmtGen.cpp
@@ -189,14 +189,24 @@ bool ByteCodeStmtGen<Emitter>::visitFunc(const FunctionDecl *F) {
if (!emitFieldInitializer(F, F->Offset, InitExpr))
return false;
} else if (const Type *Base = Init->getBaseClass()) {
- // Base class initializer.
- // Get This Base and call initializer on it.
const auto *BaseDecl = Base->getAsCXXRecordDecl();
assert(BaseDecl);
- const Record::Base *B = R->getBase(BaseDecl);
- assert(B);
- if (!this->emitGetPtrThisBase(B->Offset, InitExpr))
- return false;
+
+ if (Init->isBaseVirtual()) {
+ const Record::Base *B = R->getVirtualBase(BaseDecl);
+ assert(B);
+ if (!this->emitGetPtrThisVirtBase(BaseDecl, InitExpr))
+ return false;
+
+ } else {
+ // Base class initializer.
+ // Get This Base and call initializer on it.
+ const Record::Base *B = R->getBase(BaseDecl);
+ assert(B);
+ if (!this->emitGetPtrThisBase(B->Offset, InitExpr))
+ return false;
+ }
+
if (!this->visitInitializer(InitExpr))
return false;
if (!this->emitFinishInitPop(InitExpr))
diff --git a/clang/lib/AST/Interp/Descriptor.cpp b/clang/lib/AST/Interp/Descriptor.cpp
index a4ccc0236d292c..954c58c8cb3716 100644
--- a/clang/lib/AST/Interp/Descriptor.cpp
+++ b/clang/lib/AST/Interp/Descriptor.cpp
@@ -136,28 +136,66 @@ static void moveArrayDesc(Block *B, const std::byte *Src, std::byte *Dst,
}
}
+static void initField(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
+ bool IsActive, const Descriptor *D,
+ unsigned FieldOffset) {
+ bool IsUnion = false; // FIXME
+ auto *Desc = reinterpret_cast<InlineDescriptor *>(Ptr + FieldOffset) - 1;
+ Desc->Offset = FieldOffset;
+ Desc->Desc = D;
+ Desc->IsInitialized = D->IsArray;
+ Desc->IsBase = false;
+ Desc->IsActive = IsActive && !IsUnion;
+ Desc->IsConst = IsConst || D->IsConst;
+ Desc->IsFieldMutable = IsMutable || D->IsMutable;
+
+ if (auto Fn = D->CtorFn)
+ Fn(B, Ptr + FieldOffset, Desc->IsConst, Desc->IsFieldMutable,
+ Desc->IsActive, D);
+}
+
+static void initBase(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
+ bool IsActive, const Descriptor *D, unsigned FieldOffset,
+ bool IsVirtualBase) {
+ assert(D);
+ assert(D->ElemRecord);
+
+ bool IsUnion = D->ElemRecord->isUnion();
+ auto *Desc = reinterpret_cast<InlineDescriptor *>(Ptr + FieldOffset) - 1;
+ Desc->Offset = FieldOffset;
+ Desc->Desc = D;
+ Desc->IsInitialized = D->IsArray;
+ Desc->IsBase = true;
+ Desc->IsActive = IsActive && !IsUnion;
+ Desc->IsConst = IsConst || D->IsConst;
+ Desc->IsFieldMutable = IsMutable || D->IsMutable;
+
+ for (const auto &V : D->ElemRecord->bases())
+ initBase(B, Ptr + FieldOffset, IsConst, IsMutable, IsActive, V.Desc,
+ V.Offset, false);
+ for (const auto &F : D->ElemRecord->fields())
+ initField(B, Ptr + FieldOffset, IsConst, IsMutable, IsActive, F.Desc,
+ F.Offset);
+
+ // If this is initializing a virtual base, we do NOT want to consider its
+ // virtual bases, those are already flattened into the parent record when
+ // creating it.
+ if (IsVirtualBase)
+ return;
+
+ for (const auto &V : D->ElemRecord->virtual_bases())
+ initBase(B, Ptr + FieldOffset, IsConst, IsMutable, IsActive, V.Desc,
+ V.Offset, true);
+}
+
static void ctorRecord(Block *B, std::byte *Ptr, bool IsConst, bool IsMutable,
bool IsActive, const Descriptor *D) {
- const bool IsUnion = D->ElemRecord->isUnion();
- auto CtorSub = [=](unsigned SubOff, const Descriptor *F, bool IsBase) {
- auto *Desc = reinterpret_cast<InlineDescriptor *>(Ptr + SubOff) - 1;
- Desc->Offset = SubOff;
- Desc->Desc = F;
- Desc->IsInitialized = F->IsArray && !IsBase;
- Desc->IsBase = IsBase;
- Desc->IsActive = IsActive && !IsUnion;
- Desc->IsConst = IsConst || F->IsConst;
- Desc->IsFieldMutable = IsMutable || F->IsMutable;
- if (auto Fn = F->CtorFn)
- Fn(B, Ptr + SubOff, Desc->IsConst, Desc->IsFieldMutable, Desc->IsActive,
- F);
- };
- for (const auto &B : D->ElemRecord->bases())
- CtorSub(B.Offset, B.Desc, /*isBase=*/true);
+ for (const auto &V : D->ElemRecord->bases())
+ initBase(B, Ptr, IsConst, IsMutable, IsActive, V.Desc, V.Offset, false);
for (const auto &F : D->ElemRecord->fields())
- CtorSub(F.Offset, F.Desc, /*isBase=*/false);
+ initField(B, Ptr, IsConst, IsMutable, IsActive, F.Desc, F.Offset);
for (const auto &V : D->ElemRecord->virtual_bases())
- CtorSub(V.Offset, V.Desc, /*isBase=*/true);
+ initBase(B, Ptr, IsConst, IsMutable, IsActive, V.Desc, V.Offset, true);
}
static void dtorRecord(Block *B, std::byte *Ptr, const Descriptor *D) {
diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 0e9f287cd2218f..9da0286deada17 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1366,6 +1366,9 @@ inline bool GetPtrVirtBasePop(InterpState &S, CodePtr OpPC,
const Pointer &Ptr = S.Stk.pop<Pointer>();
if (!CheckNull(S, OpPC, Ptr, CSK_Base))
return false;
+ if (Ptr.isDummy()) // FIXME: Once we have type info for dummy pointers, this
+ // needs to go.
+ return false;
return VirtBaseHelper(S, OpPC, D, Ptr);
}
diff --git a/clang/test/AST/Interp/cxx23.cpp b/clang/test/AST/Interp/cxx23.cpp
index f0325eef6d87cf..13cc9f43febc74 100644
--- a/clang/test/AST/Interp/cxx23.cpp
+++ b/clang/test/AST/Interp/cxx23.cpp
@@ -141,3 +141,19 @@ struct check_ice {
};
};
static_assert(check_ice<42>::x == 42);
+
+
+namespace VirtualBases {
+ namespace One {
+ struct U { int n; };
+ struct V : U { int n; };
+ struct A : virtual V { int n; };
+ struct Aa { int n; };
+ struct B : virtual A, Aa {};
+ struct C : virtual A, Aa {};
+ struct D : B, C {};
+
+ /// Calls the constructor of D.
+ D d;
+ }
+}
diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index 3e52354a4a1067..9307b9c090c5dd 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -1330,3 +1330,82 @@ namespace UnnamedBitFields {
static_assert(a.f == 1.0, "");
static_assert(a.c == 'a', "");
}
+
+/// FIXME: This still doesn't work in the new interpreter because
+/// we lack type information for dummy pointers.
+namespace VirtualBases {
+ /// This used to crash.
+ namespace One {
+ class A {
+ protected:
+ int x;
+ };
+ class B : public virtual A {
+ public:
+ int getX() { return x; } // ref-note {{declared here}}
+ };
+
+ class DV : virtual public B{};
+
+ void foo() {
+ DV b;
+ int a[b.getX()]; // both-warning {{variable length arrays}} \
+ // ref-note {{non-constexpr function 'getX' cannot be used}}
+ }
+ }
+
+ namespace Two {
+ struct U { int n; };
+ struct A : virtual U { int n; };
+ struct B : A {};
+ B a;
+ static_assert((U*)(A*)(&a) == (U*)(&a), "");
+
+ struct C : virtual A {};
+ struct D : B, C {};
+ D d;
+ constexpr B *p = &d;
+ constexpr C *q = &d;
+ static_assert((A*)p == (A*)q, ""); // both-error {{failed}}
+ }
+
+ namespace Three {
+ struct U { int n; };
+ struct V : U { int n; };
+ struct A : virtual V { int n; };
+ struct Aa { int n; };
+ struct B : virtual A, Aa {};
+
+ struct C : virtual A, Aa {};
+
+ struct D : B, C {};
+
+ D d;
+
+ constexpr B *p = &d;
+ constexpr C *q = &d;
+
+ static_assert((void*)p != (void*)q, "");
+ static_assert((A*)p == (A*)q, "");
+ static_assert((Aa*)p != (Aa*)q, "");
+
+ constexpr V *v = p;
+ constexpr V *w = q;
+ constexpr V *x = (A*)p;
+ static_assert(v == w, "");
+ static_assert(v == x, "");
+
+ static_assert((U*)&d == p, "");
+ static_assert((U*)&d == q, "");
+ static_assert((U*)&d == v, "");
+ static_assert((U*)&d == w, "");
+ static_assert((U*)&d == x, "");
+
+ struct X {};
+ struct Y1 : virtual X {};
+ struct Y2 : X {};
+ struct Z : Y1, Y2 {};
+ Z z;
+ static_assert((X*)(Y1*)&z != (X*)(Y2*)&z, "");
+ }
+}
More information about the cfe-commits
mailing list