[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

2018-12-11 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai abandoned this revision. smeenai added a comment. https://reviews.llvm.org/D55543 does the same thing Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44619/new/ https://reviews.llvm.org/D44619 ___ cfe-commits m

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

2018-04-03 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai planned changes to this revision. smeenai added a comment. This doesn't pass all tests right now, and I'll also need to change the test in accordance with the review comments. Repository: rC Clang https://reviews.llvm.org/D44619 ___ cfe-

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

2018-04-03 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Finally got a chance to dig into some of the failures this patch is causing. Here's an example crasher (run with `clang -cc1 -triple i686-windows -emit-llvm`): struct Q { Q(int); ~Q(); }; struct Z {}; struct A { A(Q); }; struct B : Z, A { usi

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

2018-03-20 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments. Comment at: lib/CodeGen/CGClass.cpp:2183 ApplyInlineDebugLocation DebugScope(*this, GD); + RunCleanupsScope CleanupScope(*this); Please add a test to ensure that we still destroy function parameters in the right order and at t

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

2018-03-20 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Thanks Reid. I still need to look into why this is causing some existing tests to crash, but I'll also adjust the test. Repository: rC Clang https://reviews.llvm.org/D44619 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

2018-03-19 Thread Reid Kleckner via Phabricator via cfe-commits
rnk accepted this revision. rnk added a comment. This revision is now accepted and ready to land. Code looks good, but we should make the test more robust and self-documenting. Comment at: test/CodeGenCXX/inheriting-constructor-cleanup.cpp:22 +// CHECK-LABEL: define void @_Z1fv

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

2018-03-18 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai added a comment. Argh, this fixes my test case, but causes failures in some other ones. Back to the drawing board, I guess. Repository: rC Clang https://reviews.llvm.org/D44619 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D44619: [CodeGen] Add cleanup scope to EmitInlinedInheritingCXXConstructorCall

2018-03-18 Thread Shoaib Meenai via Phabricator via cfe-commits
smeenai created this revision. smeenai added reviewers: rnk, rsmith. EmitInlinedInheritingCXXConstructorCall may result in a CallBaseDtor cleanup being pushed. That cleanup would then be popped when the CGF's CurCodeDecl no longer points to the method which triggered the cleanup, leading to a fail