[PATCH] D135025: [clang][Interp] Support base class constructors

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG9cb4e90e7260: [clang][Interp] Support base class constructors (authored by tbaeder). Changed prior to commit:

[PATCH] D135025: [clang][Interp] Support base class constructors

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. Okay, this is incremental progress, so LGTM! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder marked an inline comment as done. tbaeder added a comment. In D135025#3862031 , @aaron.ballman wrote: > Do you think we should address some of the FIXMEs with base class ctor > functionality as part of this patch, given that the failures all

[PATCH] D135025: [clang][Interp] Support base class constructors

aaron.ballman added a comment. Do you think we should address some of the FIXMEs with base class ctor functionality as part of this patch, given that the failures all seem to be related to that functionality? Or are these failures due to other missing functionality that happened to be exposed

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder marked an inline comment as done. tbaeder added inline comments. Comment at: clang/test/AST/Interp/records.cpp:209 + static_assert(d.getA() == 20); + static_assert(d.getB() == 30); +}; aaron.ballman wrote: > I'd appreciate some more failure test cases,

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder updated this revision to Diff 467699. tbaeder marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h

[PATCH] D135025: [clang][Interp] Support base class constructors

aaron.ballman added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:258 + /// or nullptr if no such decl exists. + const CXXRecordDecl * getRecordDecl(const Expr *E) const { +QualType T = E->getType(); Comment at:

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder added a comment. Anything still missing here? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder updated this revision to Diff 465271. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/ByteCodeStmtGen.cpp

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder added inline comments. Comment at: clang/test/AST/Interp/records.cpp:165 + +class A : public Base { +public: tbaeder wrote: > erichkeane wrote: > > tbaeder wrote: > > > erichkeane wrote: > > > > shafik wrote: > > > > > How about also testing `private`

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder marked an inline comment as done. tbaeder added inline comments. Comment at: clang/test/AST/Interp/records.cpp:165 + +class A : public Base { +public: erichkeane wrote: > tbaeder wrote: > > erichkeane wrote: > > > shafik wrote: > > > > How about also

[PATCH] D135025: [clang][Interp] Support base class constructors

erichkeane added inline comments. Comment at: clang/test/AST/Interp/records.cpp:165 + +class A : public Base { +public: tbaeder wrote: > erichkeane wrote: > > shafik wrote: > > > How about also testing `private` and `virtual` as well as multiple bases. > > I

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder marked an inline comment as done. tbaeder added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:263 +QualType T = E->getType(); +if (const auto *PT = dyn_cast(T)) + T = PT->getPointeeType(); erichkeane wrote: > We are

[PATCH] D135025: [clang][Interp] Support base class constructors

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:263 +QualType T = E->getType(); +if (const auto *PT = dyn_cast(T)) + T = PT->getPointeeType(); We are de-pointering here, why not de-referencing? If we are OK

[PATCH] D135025: [clang][Interp] Support base class constructors

shafik added a comment. This looks good to me but I would like one other set of eyes on it CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder updated this revision to Diff 464915. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeExprGen.h clang/lib/AST/Interp/ByteCodeStmtGen.cpp

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder updated this revision to Diff 464913. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeStmtGen.cpp clang/test/AST/Interp/records.cpp Index:

[PATCH] D135025: [clang][Interp] Support base class constructors

shafik added a comment. I realize it is not directly related but I don't see it in the test suite (maybe I missed it) but also delegating constructors, default member initializaers Vs mem-init list (mem-init list has priority) and mem-init list items out of order of initialization.

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder updated this revision to Diff 464618. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeStmtGen.cpp clang/test/AST/Interp/records.cpp Index:

[PATCH] D135025: [clang][Interp] Support base class constructors

tbaeder updated this revision to Diff 464549. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 Files: clang/lib/AST/Interp/ByteCodeExprGen.cpp clang/lib/AST/Interp/ByteCodeStmtGen.cpp clang/test/AST/Interp/records.cpp Index:

[PATCH] D135025: [clang][Interp] Support base class constructors

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. I think this is pretty simple since we get the