[PATCH] D154189: [clang][Interp] Implement zero-init of record types
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6d79f985b53e: [clang][Interp] Implement zero-init of record types (authored by tbaeder). Changed prior to commit: https://reviews.llvm.org/D154189?vs=545367=555836#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Descriptor.cpp clang/lib/AST/Interp/Descriptor.h clang/test/AST/Interp/records.cpp Index: clang/test/AST/Interp/records.cpp === --- clang/test/AST/Interp/records.cpp +++ clang/test/AST/Interp/records.cpp @@ -911,3 +911,115 @@ } static_assert(foo(F()) == 0, ""); } + +namespace ZeroInit { + struct F { +int a; + }; + + namespace Simple { +struct A { + char a; + bool b; + int c[4]; + float d; +}; +constexpr int foo(A x) { + return x.a + static_cast(x.b) + x.c[0] + x.c[3] + static_cast(x.d); +} +static_assert(foo(A()) == 0, ""); + } + + namespace Inheritance { +struct F2 : F { + float f; +}; + +constexpr int foo(F2 f) { + return (int)f.f + f.a; +} +static_assert(foo(F2()) == 0, ""); + } + + namespace BitFields { +struct F { + unsigned a : 6; +}; +constexpr int foo(F f) { + return f.a; +} +static_assert(foo(F()) == 0, ""); + } + + namespace Nested { +struct F2 { + float f; + char c; +}; + +struct F { + F2 f2; + int i; +}; + +constexpr int foo(F f) { + return f.i + f.f2.f + f.f2.c; +} +static_assert(foo(F()) == 0, ""); + } + + namespace CompositeArrays { +struct F2 { + float f; + char c; +}; + +struct F { + F2 f2[2]; + int i; +}; + +constexpr int foo(F f) { + return f.i + f.f2[0].f + f.f2[0].c + f.f2[1].f + f.f2[1].c; +} +static_assert(foo(F()) == 0, ""); + } + + /// FIXME: This needs support for unions on the new interpreter. + /// We diagnose an uninitialized object in c++14. +#if __cplusplus > 201402L + namespace Unions { +struct F { + union { +int a; +char c[4]; +float f; + } U; + int i; +}; + +constexpr int foo(F f) { + return f.i + f.U.f; // ref-note {{read of member 'f' of union with active member 'a'}} +} +static_assert(foo(F()) == 0, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} + } +#endif + +#if __cplusplus >= 202002L + namespace Failure { +struct S { + int a; + F f{12}; +}; +constexpr int foo(S x) { + return x.a; // expected-note {{read of uninitialized object}} \ + // ref-note {{read of uninitialized object}} +} +static_assert(foo(S()) == 0, ""); // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} + }; +#endif +} Index: clang/lib/AST/Interp/Descriptor.h === --- clang/lib/AST/Interp/Descriptor.h +++ clang/lib/AST/Interp/Descriptor.h @@ -138,6 +138,7 @@ bool IsTemporary, bool IsMutable); QualType getType() const; + QualType getElemQualType() const; SourceLocation getLocation() const; const Decl *asDecl() const { return Source.dyn_cast(); } Index: clang/lib/AST/Interp/Descriptor.cpp === --- clang/lib/AST/Interp/Descriptor.cpp +++ clang/lib/AST/Interp/Descriptor.cpp @@ -285,6 +285,12 @@ llvm_unreachable("Invalid descriptor type"); } +QualType Descriptor::getElemQualType() const { + assert(isArray()); + const auto *AT = cast(getType()); + return AT->getElementType(); +} + SourceLocation Descriptor::getLocation() const { if (auto *D = Source.dyn_cast()) return D->getLocation(); Index: clang/lib/AST/Interp/ByteCodeExprGen.h === --- clang/lib/AST/Interp/ByteCodeExprGen.h +++ clang/lib/AST/Interp/ByteCodeExprGen.h @@ -213,6 +213,7 @@ /// Emits a zero initializer. bool visitZeroInitializer(QualType QT, const Expr *E); + bool visitZeroRecordInitializer(const Record *R, const Expr *E); enum class DerefKind { /// Value is read and pushed to stack. Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -1302,7
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder updated this revision to Diff 545367. tbaeder marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Descriptor.cpp clang/lib/AST/Interp/Descriptor.h clang/test/AST/Interp/records.cpp Index: clang/test/AST/Interp/records.cpp === --- clang/test/AST/Interp/records.cpp +++ clang/test/AST/Interp/records.cpp @@ -931,3 +931,116 @@ O o1(0); } #endif + + +namespace ZeroInit { + struct F { +int a; + }; + + namespace Simple { +struct A { + char a; + bool b; + int c[4]; + float d; +}; +constexpr int foo(A x) { + return x.a + static_cast(x.b) + x.c[0] + x.c[3] + static_cast(x.d); +} +static_assert(foo(A()) == 0, ""); + } + + namespace Inheritance { +struct F2 : F { + float f; +}; + +constexpr int foo(F2 f) { + return (int)f.f + f.a; +} +static_assert(foo(F2()) == 0, ""); + } + + namespace BitFields { +struct F { + unsigned a : 6; +}; +constexpr int foo(F f) { + return f.a; +} +static_assert(foo(F()) == 0, ""); + } + + namespace Nested { +struct F2 { + float f; + char c; +}; + +struct F { + F2 f2; + int i; +}; + +constexpr int foo(F f) { + return f.i + f.f2.f + f.f2.c; +} +static_assert(foo(F()) == 0, ""); + } + + namespace CompositeArrays { +struct F2 { + float f; + char c; +}; + +struct F { + F2 f2[2]; + int i; +}; + +constexpr int foo(F f) { + return f.i + f.f2[0].f + f.f2[0].c + f.f2[1].f + f.f2[1].c; +} +static_assert(foo(F()) == 0, ""); + } + + /// FIXME: This needs support for unions on the new interpreter. + /// We diagnose an uninitialized object in c++14. +#if __cplusplus > 201402L + namespace Unions { +struct F { + union { +int a; +char c[4]; +float f; + } U; + int i; +}; + +constexpr int foo(F f) { + return f.i + f.U.f; // ref-note {{read of member 'f' of union with active member 'a'}} +} +static_assert(foo(F()) == 0, ""); // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} + } +#endif + +#if __cplusplus >= 202002L + namespace Failure { +struct S { + int a; + F f{12}; +}; +constexpr int foo(S x) { + return x.a; // expected-note {{read of uninitialized object}} \ + // ref-note {{read of uninitialized object}} +} +static_assert(foo(S()) == 0, ""); // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} + }; +#endif +} Index: clang/lib/AST/Interp/Descriptor.h === --- clang/lib/AST/Interp/Descriptor.h +++ clang/lib/AST/Interp/Descriptor.h @@ -140,6 +140,7 @@ bool IsTemporary, bool IsMutable); QualType getType() const; + QualType getElemQualType() const; SourceLocation getLocation() const; const Decl *asDecl() const { return Source.dyn_cast(); } Index: clang/lib/AST/Interp/Descriptor.cpp === --- clang/lib/AST/Interp/Descriptor.cpp +++ clang/lib/AST/Interp/Descriptor.cpp @@ -289,6 +289,12 @@ llvm_unreachable("Invalid descriptor type"); } +QualType Descriptor::getElemQualType() const { + assert(isArray()); + const auto *AT = cast(getType()); + return AT->getElementType(); +} + SourceLocation Descriptor::getLocation() const { if (auto *D = Source.dyn_cast()) return D->getLocation(); Index: clang/lib/AST/Interp/ByteCodeExprGen.h === --- clang/lib/AST/Interp/ByteCodeExprGen.h +++ clang/lib/AST/Interp/ByteCodeExprGen.h @@ -222,6 +222,7 @@ /// Emits a zero initializer. bool visitZeroInitializer(QualType QT, const Expr *E); + bool visitZeroRecordInitializer(const Record *R, const Expr *E); enum class DerefKind { /// Value is read and pushed to stack. Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -1394,7 +1394,15 @@ assert(!classify(T)); if (T->isRecordType()) { -const Function *Func = getFunction(E->getConstructor()); +const CXXConstructorDecl *Ctor = E->getConstructor(); + +// Trivial zero initialization. +if (E->requiresZeroInitialization() && Ctor->isTrivial()) { +
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1240 + // Fields + for (const Record::Field : R->fields()) { +const Descriptor *D = Field.Desc; tbaeder wrote: > aaron.ballman wrote: > > It looks like you're not initializing base classes or padding bits: > > http://eel.is/c++draft/dcl.init#general-6.2 -- we could use test coverage > > for both of those cases (don't forget to also test bit-fields). You should > > also have a test for zero init of unions. > Unions and bitfields are generally not supported yet, and I'm not sure what > you mean by padding bits - they don't exist at this stage. In storage > however, they are always zero since we memset that to 0. > Unions and bitfields are generally not supported yet, Let's add the test coverage and mark failures with FIXME comments; otherwise we risk forgetting to add the test coverage later. > and I'm not sure what you mean by padding bits - they don't exist at this > stage. In storage however, they are always zero since we memset that to 0. So long as we're validating that they're properly set to zero; that's the important part. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder updated this revision to Diff 545146. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Descriptor.cpp clang/lib/AST/Interp/Descriptor.h clang/test/AST/Interp/records.cpp Index: clang/test/AST/Interp/records.cpp === --- clang/test/AST/Interp/records.cpp +++ clang/test/AST/Interp/records.cpp @@ -931,3 +931,63 @@ O o1(0); } #endif + + +namespace ZeroInit { + struct F { +int a; + }; + + namespace Simple { +struct A { + char a; + bool b; + int c[4]; + float d; +}; +constexpr int foo(A x) { + return x.a + static_cast(x.b) + x.c[0] + x.c[3] + static_cast(x.d); +} +static_assert(foo(A()) == 0, ""); + } + + namespace Inheritance { +struct F2 : F { + float f; +}; + +constexpr int foo(F2 f) { + return (int)f.f + f.a; +} +static_assert(foo(F2()) == 0, ""); + } + + namespace BitFields { +struct F { + unsigned a : 6; +}; +constexpr int foo(F f) { + return f.a; +} +static_assert(foo(F()) == 0, ""); + } + + +#if __cplusplus >= 202002L + namespace Failure { +struct S { + int a; + F f{12}; +}; +constexpr int foo(S x) { + return x.a; // expected-note {{read of uninitialized object}} \ + // ref-note {{read of uninitialized object}} +} +static_assert(foo(S()) == 0, ""); // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} + }; +#endif +} + Index: clang/lib/AST/Interp/Descriptor.h === --- clang/lib/AST/Interp/Descriptor.h +++ clang/lib/AST/Interp/Descriptor.h @@ -140,6 +140,7 @@ bool IsTemporary, bool IsMutable); QualType getType() const; + QualType getElemQualType() const; SourceLocation getLocation() const; const Decl *asDecl() const { return Source.dyn_cast(); } Index: clang/lib/AST/Interp/Descriptor.cpp === --- clang/lib/AST/Interp/Descriptor.cpp +++ clang/lib/AST/Interp/Descriptor.cpp @@ -289,6 +289,12 @@ llvm_unreachable("Invalid descriptor type"); } +QualType Descriptor::getElemQualType() const { + assert(isArray()); + const auto *AT = cast(getType()); + return AT->getElementType(); +} + SourceLocation Descriptor::getLocation() const { if (auto *D = Source.dyn_cast()) return D->getLocation(); Index: clang/lib/AST/Interp/ByteCodeExprGen.h === --- clang/lib/AST/Interp/ByteCodeExprGen.h +++ clang/lib/AST/Interp/ByteCodeExprGen.h @@ -222,6 +222,7 @@ /// Emits a zero initializer. bool visitZeroInitializer(QualType QT, const Expr *E); + bool visitZeroRecordInitializer(const Record *R, const Expr *E); enum class DerefKind { /// Value is read and pushed to stack. Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -1394,7 +1394,15 @@ assert(!classify(T)); if (T->isRecordType()) { -const Function *Func = getFunction(E->getConstructor()); +const CXXConstructorDecl *Ctor = E->getConstructor(); + +// Trivial zero initialization. +if (E->requiresZeroInitialization() && Ctor->isTrivial()) { + const Record *R = getRecord(E->getType()); + return this->visitZeroRecordInitializer(R, E); +} + +const Function *Func = getFunction(Ctor); if (!Func) return false; @@ -1698,6 +1706,62 @@ llvm_unreachable("unknown primitive type"); } +template +bool ByteCodeExprGen::visitZeroRecordInitializer(const Record *R, + const Expr *E) { + assert(E); + assert(R); + // Fields + for (const Record::Field : R->fields()) { +const Descriptor *D = Field.Desc; +if (D->isPrimitive()) { + QualType QT = D->getType(); + PrimType T = classifyPrim(D->getType()); + if (!this->visitZeroInitializer(QT, E)) +return false; + if (!this->emitInitField(T, Field.Offset, E)) +return false; + continue; +} + +// TODO: Add GetPtrFieldPop and get rid of this dup. +if (!this->emitDupPtr(E)) + return false; +if (!this->emitGetPtrField(Field.Offset, E)) + return false; + +if (D->isPrimitiveArray()) { + QualType ET = D->getElemQualType(); + PrimType T = classifyPrim(ET); + for (uint32_t I = 0, N = D->getNumElems(); I
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder added inline comments. Comment at: clang/test/AST/Interp/records.cpp:939 + }; +#endif + aaron.ballman wrote: > I think we should have more test coverage. I mentioned a few above, but other > things to test would be inner classes (and anonymous members), like: > ``` > struct S { > struct { > int x, y; > } v; > union { > float a; > int b; > }; > }; > ``` > and member pointers (because those aren't simple pointers): > ``` > struct A { > void func(); > }; > struct S { > void (A::*ptr)(); > }; > ``` member pointers are also not handled yet. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1240 + // Fields + for (const Record::Field : R->fields()) { +const Descriptor *D = Field.Desc; aaron.ballman wrote: > It looks like you're not initializing base classes or padding bits: > http://eel.is/c++draft/dcl.init#general-6.2 -- we could use test coverage for > both of those cases (don't forget to also test bit-fields). You should also > have a test for zero init of unions. Unions and bitfields are generally not supported yet, and I'm not sure what you mean by padding bits - they don't exist at this stage. In storage however, they are always zero since we memset that to 0. Comment at: clang/lib/AST/Interp/Descriptor.cpp:281-289 +QualType Descriptor::getElemQualType() const { + assert(isArray()); + QualType T = getType(); + + const auto *CAT = cast(T); + return CAT->getElementType(); + aaron.ballman wrote: > This looks pretty dangerous -- is it safe to assume the type is a constant > array, or should this be a `dyn_cast` and the spurious `return` is for the > `else` branch? I only need an `ArrayType` anyway, so should be fine. Comment at: clang/test/AST/Interp/records.cpp:931 +constexpr int foo(S x) { + return x.a; // expected-note {{read of object outside its lifetime}} \ + // ref-note {{read of uninitialized object}} aaron.ballman wrote: > Why is `x.a` outside of its lifetime? That was just a wrong diagnostic that has since been fixed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder updated this revision to Diff 545131. tbaeder marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Descriptor.cpp clang/lib/AST/Interp/Descriptor.h clang/test/AST/Interp/records.cpp Index: clang/test/AST/Interp/records.cpp === --- clang/test/AST/Interp/records.cpp +++ clang/test/AST/Interp/records.cpp @@ -931,3 +931,63 @@ O o1(0); } #endif + + +namespace ZeroInit { + struct F { +int a; + }; + + namespace Simple { +struct A { + char a; + bool b; + int c[4]; + float d; +}; +constexpr int foo(A x) { + return x.a + static_cast(x.b) + x.c[0] + x.c[3] + static_cast(x.d); +} +static_assert(foo(A()) == 0, ""); + } + + namespace Inheritance { +struct F2 : F { + float f; +}; + +constexpr int foo(F2 f) { + return (int)f.f + f.a; +} +static_assert(foo(F2()) == 0, ""); + } + + namespace BitFields { +struct F { + unsigned a : 6; +}; +constexpr int foo(F f) { + return f.a; +} +static_assert(foo(F()) == 0, ""); + } + + +#if __cplusplus >= 202002L + namespace Failure { +struct S { + int a; + F f{12}; +}; +constexpr int foo(S x) { + return x.a; // expected-note {{read of uninitialized object}} \ + // ref-note {{read of uninitialized object}} +} +static_assert(foo(S()) == 0, ""); // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} + }; +#endif +} + Index: clang/lib/AST/Interp/Descriptor.h === --- clang/lib/AST/Interp/Descriptor.h +++ clang/lib/AST/Interp/Descriptor.h @@ -140,6 +140,7 @@ bool IsTemporary, bool IsMutable); QualType getType() const; + QualType getElemQualType() const; SourceLocation getLocation() const; const Decl *asDecl() const { return Source.dyn_cast(); } Index: clang/lib/AST/Interp/Descriptor.cpp === --- clang/lib/AST/Interp/Descriptor.cpp +++ clang/lib/AST/Interp/Descriptor.cpp @@ -289,6 +289,12 @@ llvm_unreachable("Invalid descriptor type"); } +QualType Descriptor::getElemQualType() const { + assert(isArray()); + const auto *AT = cast(getType()); + return AT->getElementType(); +} + SourceLocation Descriptor::getLocation() const { if (auto *D = Source.dyn_cast()) return D->getLocation(); Index: clang/lib/AST/Interp/ByteCodeExprGen.h === --- clang/lib/AST/Interp/ByteCodeExprGen.h +++ clang/lib/AST/Interp/ByteCodeExprGen.h @@ -222,6 +222,7 @@ /// Emits a zero initializer. bool visitZeroInitializer(QualType QT, const Expr *E); + bool visitZeroRecordInitializer(const Record *R, const Expr *E); enum class DerefKind { /// Value is read and pushed to stack. Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -1394,7 +1394,15 @@ assert(!classify(T)); if (T->isRecordType()) { -const Function *Func = getFunction(E->getConstructor()); +const CXXConstructorDecl *Ctor = E->getConstructor(); + +// Trivial zero initialization. +if (E->requiresZeroInitialization() && Ctor->isTrivial()) { + const Record *R = getRecord(E->getType()); + return this->visitZeroRecordInitializer(R, E); +} + +const Function *Func = getFunction(Ctor); if (!Func) return false; @@ -1467,7 +1475,7 @@ return true; } - return false; + return DiscardResult ? this->emitPopPtr(E) : true; } template @@ -1698,6 +1706,62 @@ llvm_unreachable("unknown primitive type"); } +template +bool ByteCodeExprGen::visitZeroRecordInitializer(const Record *R, + const Expr *E) { + assert(E); + assert(R); + // Fields + for (const Record::Field : R->fields()) { +const Descriptor *D = Field.Desc; +if (D->isPrimitive()) { + QualType QT = D->getType(); + PrimType T = classifyPrim(D->getType()); + if (!this->visitZeroInitializer(QT, E)) +return false; + if (!this->emitInitField(T, Field.Offset, E)) +return false; + continue; +} + +// TODO: Add GetPtrFieldPop and get rid of this dup. +if (!this->emitDupPtr(E)) + return false; +if (!this->emitGetPtrField(Field.Offset, E)) + return
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1240 + // Fields + for (const Record::Field : R->fields()) { +const Descriptor *D = Field.Desc; It looks like you're not initializing base classes or padding bits: http://eel.is/c++draft/dcl.init#general-6.2 -- we could use test coverage for both of those cases (don't forget to also test bit-fields). You should also have a test for zero init of unions. Comment at: clang/lib/AST/Interp/Descriptor.cpp:281-289 +QualType Descriptor::getElemQualType() const { + assert(isArray()); + QualType T = getType(); + + const auto *CAT = cast(T); + return CAT->getElementType(); + This looks pretty dangerous -- is it safe to assume the type is a constant array, or should this be a `dyn_cast` and the spurious `return` is for the `else` branch? Comment at: clang/test/AST/Interp/records.cpp:931 +constexpr int foo(S x) { + return x.a; // expected-note {{read of object outside its lifetime}} \ + // ref-note {{read of uninitialized object}} Why is `x.a` outside of its lifetime? Comment at: clang/test/AST/Interp/records.cpp:939 + }; +#endif + I think we should have more test coverage. I mentioned a few above, but other things to test would be inner classes (and anonymous members), like: ``` struct S { struct { int x, y; } v; union { float a; int b; }; }; ``` and member pointers (because those aren't simple pointers): ``` struct A { void func(); }; struct S { void (A::*ptr)(); }; ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1115 + // all field and bases explicitly. + if (E->requiresZeroInitialization() && Ctor->isTrivial()) { +assert(E->getType()->isRecordType()); I briefly talked about this with @aaron.ballman on IRC. The current interpreter properly zero-initialized all the fields of a struct, but the un-does the initialization again (and initializes them to a `APValue::Indeterminate` in `HandleConstructorCall::SkipToField()`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154189/new/ https://reviews.llvm.org/D154189 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D154189: [clang][Interp] Implement zero-init of record types
tbaeder created this revision. tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik. Herald added a project: All. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Unify `CXXTemporaryObjectExpr` and `CXXConstructExpr` code and zero-initialize the object if requested. Hints on how to test this properly without creating some weird intermediate object are welcome of course. :) Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D154189 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/Descriptor.cpp clang/lib/AST/Interp/Descriptor.h clang/test/AST/Interp/records.cpp Index: clang/test/AST/Interp/records.cpp === --- clang/test/AST/Interp/records.cpp +++ clang/test/AST/Interp/records.cpp @@ -899,3 +899,45 @@ return bp.first && bp.second; } static_assert(BPand(BoolPair{true, false}) == false, ""); + + +namespace ZeroInit { + struct F { +int a; + }; + + namespace Simple { +struct A { + char a; + bool b; + int c[4]; + float d; +}; +constexpr int foo(A x) { + return x.a + static_cast(x.b) + x.c[0] + x.c[3] + static_cast(x.d); +} +#if __cplusplus >= 201703L +static_assert(foo(A()) == 0, ""); +#endif + } + +#if __cplusplus >= 202002L + namespace Failure { +struct S { + int a; + F f{12}; +}; +constexpr int foo(S x) { + return x.a; // expected-note {{read of object outside its lifetime}} \ + // ref-note {{read of uninitialized object}} +} +static_assert(foo(S()) == 0, ""); // expected-error {{not an integral constant expression}} \ + // expected-note {{in call to}} \ + // ref-error {{not an integral constant expression}} \ + // ref-note {{in call to}} + }; +#endif + + +} + Index: clang/lib/AST/Interp/Descriptor.h === --- clang/lib/AST/Interp/Descriptor.h +++ clang/lib/AST/Interp/Descriptor.h @@ -137,6 +137,7 @@ bool IsTemporary, bool IsMutable); QualType getType() const; + QualType getElemQualType() const; SourceLocation getLocation() const; const Decl *asDecl() const { return Source.dyn_cast(); } Index: clang/lib/AST/Interp/Descriptor.cpp === --- clang/lib/AST/Interp/Descriptor.cpp +++ clang/lib/AST/Interp/Descriptor.cpp @@ -278,6 +278,16 @@ llvm_unreachable("Invalid descriptor type"); } +QualType Descriptor::getElemQualType() const { + assert(isArray()); + QualType T = getType(); + + const auto *CAT = cast(T); + return CAT->getElementType(); + + return T; +} + SourceLocation Descriptor::getLocation() const { if (auto *D = Source.dyn_cast()) return D->getLocation(); Index: clang/lib/AST/Interp/ByteCodeExprGen.h === --- clang/lib/AST/Interp/ByteCodeExprGen.h +++ clang/lib/AST/Interp/ByteCodeExprGen.h @@ -92,7 +92,6 @@ bool VisitExprWithCleanups(const ExprWithCleanups *E); bool VisitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E); bool VisitCXXBindTemporaryExpr(const CXXBindTemporaryExpr *E); - bool VisitCXXTemporaryObjectExpr(const CXXTemporaryObjectExpr *E); bool VisitCompoundLiteralExpr(const CompoundLiteralExpr *E); bool VisitTypeTraitExpr(const TypeTraitExpr *E); bool VisitLambdaExpr(const LambdaExpr *E); @@ -210,6 +209,7 @@ /// Emits a zero initializer. bool visitZeroInitializer(QualType QT, const Expr *E); + bool visitZeroRecordInitializer(const Record *R, const Expr *E); enum class DerefKind { /// Value is read and pushed to stack. Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -1019,24 +1019,6 @@ return this->visit(E->getSubExpr()); } -template -bool ByteCodeExprGen::VisitCXXTemporaryObjectExpr( -const CXXTemporaryObjectExpr *E) { - if (std::optional LocalIndex = - allocateLocal(E, /*IsExtended=*/false)) { -if (!this->emitGetPtrLocal(*LocalIndex, E)) - return false; - -if (!this->visitInitializer(E)) - return false; - -if (DiscardResult) - return this->emitPopPtr(E); -return true; - } - return false; -} - template bool ByteCodeExprGen::VisitCompoundLiteralExpr( const CompoundLiteralExpr *E) { @@ -1119,19 +1101,30 @@ template bool ByteCodeExprGen::VisitCXXConstructExpr( const CXXConstructExpr *E) { - if (std::optional GI = allocateLocal(E, /*IsExtended=*/false)) { -if (!this->emitGetPtrLocal(*GI, E)) - return false; + std::optional