[PATCH] D136012: [clang][Interp] Fix record members of reference type
This revision was automatically updated to reflect the committed changes. tbaeder marked an inline comment as done. Closed by commit rG0ccff030f3b4: [clang][Interp] Fix record members of reference type (authored by tbaeder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136012/new/ https://reviews.llvm.org/D136012 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeStmtGen.cpp clang/lib/AST/Interp/Interp.h clang/test/AST/Interp/references.cpp Index: clang/test/AST/Interp/references.cpp === --- clang/test/AST/Interp/references.cpp +++ clang/test/AST/Interp/references.cpp @@ -88,3 +88,29 @@ return j; } static_assert(RefToMemberExpr() == 11, ""); + +struct Ref { + int +}; + +constexpr int RecordWithRef() { + int m = 100; + Ref r{m}; + m = 200; + return r.a; +} +static_assert(RecordWithRef() == 200, ""); + + +struct Ref2 { + int + constexpr Ref2(int ) : a(a) {} +}; + +constexpr int RecordWithRef2() { + int m = 100; + Ref2 r(m); + m = 200; + return r.a; +} +static_assert(RecordWithRef2() == 200, ""); Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -470,6 +470,8 @@ return true; } +/// 1) Peeks a pointer on the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetField(InterpState , CodePtr OpPC, uint32_t I) { const Pointer = S.Stk.peek(); @@ -499,6 +501,8 @@ return true; } +/// 1) Pops a pointer from the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetFieldPop(InterpState , CodePtr OpPC, uint32_t I) { const Pointer = S.Stk.pop(); Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp === --- clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -105,7 +105,7 @@ if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); -if (Optional T = this->classify(InitExpr->getType())) { +if (Optional T = this->classify(InitExpr)) { if (!this->emitThis(InitExpr)) return false; Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -343,6 +343,8 @@ const Record *R = getRecord(RD); const Record::Field *F = R->getField(FD); // Leave a pointer to the field on the stack. +if (F->Decl->getType()->isReferenceType()) + return this->emitGetFieldPop(PT_Ptr, F->Offset, E); return this->emitGetPtrField(F->Offset, E); } @@ -809,7 +811,7 @@ if (!this->emitDupPtr(Initializer)) return false; - if (Optional T = classify(Init->getType())) { + if (Optional T = classify(Init)) { if (!this->visit(Init)) return false; Index: clang/test/AST/Interp/references.cpp === --- clang/test/AST/Interp/references.cpp +++ clang/test/AST/Interp/references.cpp @@ -88,3 +88,29 @@ return j; } static_assert(RefToMemberExpr() == 11, ""); + +struct Ref { + int +}; + +constexpr int RecordWithRef() { + int m = 100; + Ref r{m}; + m = 200; + return r.a; +} +static_assert(RecordWithRef() == 200, ""); + + +struct Ref2 { + int + constexpr Ref2(int ) : a(a) {} +}; + +constexpr int RecordWithRef2() { + int m = 100; + Ref2 r(m); + m = 200; + return r.a; +} +static_assert(RecordWithRef2() == 200, ""); Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -470,6 +470,8 @@ return true; } +/// 1) Peeks a pointer on the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetField(InterpState , CodePtr OpPC, uint32_t I) { const Pointer = S.Stk.peek(); @@ -499,6 +501,8 @@ return true; } +/// 1) Pops a pointer from the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetFieldPop(InterpState , CodePtr OpPC, uint32_t I) { const Pointer = S.Stk.pop(); Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp === --- clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -105,7 +105,7 @@ if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); -if (Optional T = this->classify(InitExpr->getType())) { +if (Optional T = this->classify(InitExpr)) { if (!this->emitThis(InitExpr)) return false; Index:
[PATCH] D136012: [clang][Interp] Fix record members of reference type
tbaeder marked an inline comment as done. tbaeder added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:867 - if (Optional T = classify(Init->getType())) { + if (Optional T = classify(Init)) { if (!this->visit(Init)) shafik wrote: > What is the effect here or is this just stylistic? ``` llvm::Optional classify(const Expr *E) const { return E->isGLValue() ? PT_Ptr : classify(E->getType()); } llvm::Optional classify(QualType Ty) const { return Ctx.classify(Ty); } ``` Passing the full `Expr` will return `PT_Ptr` for references. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136012/new/ https://reviews.llvm.org/D136012 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D136012: [clang][Interp] Fix record members of reference type
shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:867 - if (Optional T = classify(Init->getType())) { + if (Optional T = classify(Init)) { if (!this->visit(Init)) What is the effect here or is this just stylistic? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136012/new/ https://reviews.llvm.org/D136012 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D136012: [clang][Interp] Fix record members of reference type
tbaeder updated this revision to Diff 469580. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136012/new/ https://reviews.llvm.org/D136012 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeStmtGen.cpp clang/lib/AST/Interp/Interp.h clang/test/AST/Interp/references.cpp Index: clang/test/AST/Interp/references.cpp === --- clang/test/AST/Interp/references.cpp +++ clang/test/AST/Interp/references.cpp @@ -88,3 +88,29 @@ return j; } static_assert(RefToMemberExpr() == 11, ""); + +struct Ref { + int +}; + +constexpr int RecordWithRef() { + int m = 100; + Ref r{m}; + m = 200; + return r.a; +} +static_assert(RecordWithRef() == 200, ""); + + +struct Ref2 { + int + constexpr Ref2(int ) : a(a) {} +}; + +constexpr int RecordWithRef2() { + int m = 100; + Ref2 r(m); + m = 200; + return r.a; +} +static_assert(RecordWithRef2() == 200, ""); Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -483,6 +483,8 @@ return true; } +/// 1) Peeks a pointer on the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetField(InterpState , CodePtr OpPC, uint32_t I) { const Pointer = S.Stk.peek(); @@ -512,6 +514,8 @@ return true; } +/// 1) Pops a pointer from the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetFieldPop(InterpState , CodePtr OpPC, uint32_t I) { const Pointer = S.Stk.pop(); Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp === --- clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -104,7 +104,7 @@ if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); -if (Optional T = this->classify(InitExpr->getType())) { +if (Optional T = this->classify(InitExpr)) { if (!this->emitThis(InitExpr)) return false; Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -392,6 +392,8 @@ const Record *R = getRecord(RD); const Record::Field *F = R->getField(FD); // Leave a pointer to the field on the stack. +if (F->Decl->getType()->isReferenceType()) + return this->emitGetFieldPop(PT_Ptr, F->Offset, E); return this->emitGetPtrField(F->Offset, E); } @@ -862,7 +864,7 @@ if (!this->emitDupPtr(Initializer)) return false; - if (Optional T = classify(Init->getType())) { + if (Optional T = classify(Init)) { if (!this->visit(Init)) return false; Index: clang/test/AST/Interp/references.cpp === --- clang/test/AST/Interp/references.cpp +++ clang/test/AST/Interp/references.cpp @@ -88,3 +88,29 @@ return j; } static_assert(RefToMemberExpr() == 11, ""); + +struct Ref { + int +}; + +constexpr int RecordWithRef() { + int m = 100; + Ref r{m}; + m = 200; + return r.a; +} +static_assert(RecordWithRef() == 200, ""); + + +struct Ref2 { + int + constexpr Ref2(int ) : a(a) {} +}; + +constexpr int RecordWithRef2() { + int m = 100; + Ref2 r(m); + m = 200; + return r.a; +} +static_assert(RecordWithRef2() == 200, ""); Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -483,6 +483,8 @@ return true; } +/// 1) Peeks a pointer on the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetField(InterpState , CodePtr OpPC, uint32_t I) { const Pointer = S.Stk.peek(); @@ -512,6 +514,8 @@ return true; } +/// 1) Pops a pointer from the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetFieldPop(InterpState , CodePtr OpPC, uint32_t I) { const Pointer = S.Stk.pop(); Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp === --- clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -104,7 +104,7 @@ if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); -if (Optional T = this->classify(InitExpr->getType())) { +if (Optional T = this->classify(InitExpr)) { if (!this->emitThis(InitExpr)) return false; Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -392,6 +392,8 @@
[PATCH] D136012: [clang][Interp] Fix record members of reference type
tbaeder updated this revision to Diff 468000. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136012/new/ https://reviews.llvm.org/D136012 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeStmtGen.cpp clang/lib/AST/Interp/Interp.h clang/test/AST/Interp/references.cpp Index: clang/test/AST/Interp/references.cpp === --- clang/test/AST/Interp/references.cpp +++ clang/test/AST/Interp/references.cpp @@ -88,3 +88,29 @@ return j; } static_assert(RefToMemberExpr() == 11, ""); + +struct Ref { + int +}; + +constexpr int RecordWithRef() { + int m = 100; + Ref r{m}; + m = 200; + return r.a; +} +static_assert(RecordWithRef() == 200, ""); + + +struct Ref2 { + int + constexpr Ref2(int ) : a(a) {} +}; + +constexpr int RecordWithRef2() { + int m = 100; + Ref2 r(m); + m = 200; + return r.a; +} +static_assert(RecordWithRef2() == 200, ""); Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -478,9 +478,11 @@ return true; } +/// 1) Pops a pointer from the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetField(InterpState , CodePtr OpPC, uint32_t I) { - const Pointer = S.Stk.peek(); + const Pointer = S.Stk.pop(); if (!CheckNull(S, OpPC, Obj, CSK_Field)) return false; if (!CheckRange(S, OpPC, Obj, CSK_Field)) Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp === --- clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -104,7 +104,7 @@ if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); -if (Optional T = this->classify(InitExpr->getType())) { +if (Optional T = this->classify(InitExpr)) { if (!this->emitThis(InitExpr)) return false; Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -397,6 +397,8 @@ const Record *R = getRecord(RD); const Record::Field *F = R->getField(FD); // Leave a pointer to the field on the stack. +if (F->Decl->getType()->isReferenceType()) + return this->emitGetField(PT_Ptr, F->Offset, E); return this->emitGetPtrField(F->Offset, E); } @@ -871,7 +873,7 @@ if (!this->emitDupPtr(Initializer)) return false; - if (Optional T = classify(Init->getType())) { + if (Optional T = classify(Init)) { if (!this->visit(Init)) return false; Index: clang/test/AST/Interp/references.cpp === --- clang/test/AST/Interp/references.cpp +++ clang/test/AST/Interp/references.cpp @@ -88,3 +88,29 @@ return j; } static_assert(RefToMemberExpr() == 11, ""); + +struct Ref { + int +}; + +constexpr int RecordWithRef() { + int m = 100; + Ref r{m}; + m = 200; + return r.a; +} +static_assert(RecordWithRef() == 200, ""); + + +struct Ref2 { + int + constexpr Ref2(int ) : a(a) {} +}; + +constexpr int RecordWithRef2() { + int m = 100; + Ref2 r(m); + m = 200; + return r.a; +} +static_assert(RecordWithRef2() == 200, ""); Index: clang/lib/AST/Interp/Interp.h === --- clang/lib/AST/Interp/Interp.h +++ clang/lib/AST/Interp/Interp.h @@ -478,9 +478,11 @@ return true; } +/// 1) Pops a pointer from the stack +/// 2) Pushes the value of the pointer's field on the stack template ::T> bool GetField(InterpState , CodePtr OpPC, uint32_t I) { - const Pointer = S.Stk.peek(); + const Pointer = S.Stk.pop(); if (!CheckNull(S, OpPC, Obj, CSK_Field)) return false; if (!CheckRange(S, OpPC, Obj, CSK_Field)) Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp === --- clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -104,7 +104,7 @@ if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); -if (Optional T = this->classify(InitExpr->getType())) { +if (Optional T = this->classify(InitExpr)) { if (!this->emitThis(InitExpr)) return false; Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -397,6 +397,8 @@ const Record *R = getRecord(RD); const Record::Field *F = R->getField(FD); // Leave a pointer to the field on the stack. +if (F->Decl->getType()->isReferenceType()) + return
[PATCH] D136012: [clang][Interp] Fix record members of reference type
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. When assigning to them, we can't classify the expression type, because that doesn't contain the right information. And when reading from them, we need to do the extra deref, just like we do when reading from a DeclRefExpr. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D136012 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeStmtGen.cpp clang/test/AST/Interp/references.cpp Index: clang/test/AST/Interp/references.cpp === --- clang/test/AST/Interp/references.cpp +++ clang/test/AST/Interp/references.cpp @@ -88,3 +88,29 @@ return j; } static_assert(RefToMemberExpr() == 11, ""); + +struct Ref { + int +}; + +constexpr int RecordWithRef() { + int m = 100; + Ref r{m}; + m = 200; + return r.a; +} +static_assert(RecordWithRef() == 200, ""); + + +struct Ref2 { + int + constexpr Ref2(int ) : a(a) {} +}; + +constexpr int RecordWithRef2() { + int m = 100; + Ref2 r(m); + m = 200; + return r.a; +} +static_assert(RecordWithRef2() == 200, ""); Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp === --- clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -104,7 +104,7 @@ if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); -if (Optional T = this->classify(InitExpr->getType())) { +if (Optional T = this->classify(InitExpr)) { if (!this->emitThis(InitExpr)) return false; Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -397,7 +397,11 @@ const Record *R = getRecord(RD); const Record::Field *F = R->getField(FD); // Leave a pointer to the field on the stack. -return this->emitGetPtrField(F->Offset, E); +if (!this->emitGetPtrField(F->Offset, E)) + return false; +if (F->Decl->getType()->isReferenceType()) + return this->emitLoadPopPtr(E); +return true; } return false; @@ -871,7 +875,7 @@ if (!this->emitDupPtr(Initializer)) return false; - if (Optional T = classify(Init->getType())) { + if (Optional T = classify(Init)) { if (!this->visit(Init)) return false; Index: clang/test/AST/Interp/references.cpp === --- clang/test/AST/Interp/references.cpp +++ clang/test/AST/Interp/references.cpp @@ -88,3 +88,29 @@ return j; } static_assert(RefToMemberExpr() == 11, ""); + +struct Ref { + int +}; + +constexpr int RecordWithRef() { + int m = 100; + Ref r{m}; + m = 200; + return r.a; +} +static_assert(RecordWithRef() == 200, ""); + + +struct Ref2 { + int + constexpr Ref2(int ) : a(a) {} +}; + +constexpr int RecordWithRef2() { + int m = 100; + Ref2 r(m); + m = 200; + return r.a; +} +static_assert(RecordWithRef2() == 200, ""); Index: clang/lib/AST/Interp/ByteCodeStmtGen.cpp === --- clang/lib/AST/Interp/ByteCodeStmtGen.cpp +++ clang/lib/AST/Interp/ByteCodeStmtGen.cpp @@ -104,7 +104,7 @@ if (const FieldDecl *Member = Init->getMember()) { const Record::Field *F = R->getField(Member); -if (Optional T = this->classify(InitExpr->getType())) { +if (Optional T = this->classify(InitExpr)) { if (!this->emitThis(InitExpr)) return false; Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp === --- clang/lib/AST/Interp/ByteCodeExprGen.cpp +++ clang/lib/AST/Interp/ByteCodeExprGen.cpp @@ -397,7 +397,11 @@ const Record *R = getRecord(RD); const Record::Field *F = R->getField(FD); // Leave a pointer to the field on the stack. -return this->emitGetPtrField(F->Offset, E); +if (!this->emitGetPtrField(F->Offset, E)) + return false; +if (F->Decl->getType()->isReferenceType()) + return this->emitLoadPopPtr(E); +return true; } return false; @@ -871,7 +875,7 @@ if (!this->emitDupPtr(Initializer)) return false; - if (Optional T = classify(Init->getType())) { + if (Optional T = classify(Init)) { if (!this->visit(Init)) return false; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits