[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2023-08-01 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. The problem is back, but has been fixed upstream by https://reviews.llvm.org/rG61c7a9140becb19c5b1bc644e54452c6f782f5d5. I managed to reduce a test case triggering the assert touched in this revision, see https://reviews.llvm.org/D156806 Repository: rG LLVM Github

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2023-07-03 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld abandoned this revision. Hahnfeld added a comment. apparently not needed anymore downstream... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137787/new/ https://reviews.llvm.org/D137787 ___ cfe-

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-14 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In D137787#3924857 , @v.g.vassilev wrote: > - Is the failure also not reproducible with clang9 (on which is based cling)? > - Is the failure also not reproducible with clang9 built on top of the > downstream patches? The failu

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-14 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a reviewer: rsmith. v.g.vassilev added a subscriber: rsmith. v.g.vassilev added a comment. Usually @rsmith has something up in the sleeve in these situations ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137787/new/ https://re

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-14 Thread Vassil Vassilev via Phabricator via cfe-commits
v.g.vassilev added a comment. In D137787#3924727 , @Hahnfeld wrote: > Yes, I fully agree that having a test is desirable, I just didn't manage so > far. Maybe it takes somebody with deep AST knowledge to explain under what > circumstances `DtorDecl->get

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-14 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. Yes, I fully agree that having a test is desirable, I just didn't manage so far. Maybe it takes somebody with deep AST knowledge to explain under what circumstances `DtorDecl->getParent()` is *not* the canonical `Decl`. Maybe this could help crafting a test case, even

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-14 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D137787#3924341 , @Hahnfeld wrote: >> Sorry, I don't understand well. Could you rewrite the reproducer in the >> style I wrote? And in what cases it works fine? > > Okay, let me try to restate: I can reproduce the problem in

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-14 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. > Sorry, I don't understand well. Could you rewrite the reproducer in the style > I wrote? And in what cases it works fine? Okay, let me try to restate: I can reproduce the problem in ROOT/Cling which is an *interpreter* built on top of LLVM and CLang. The moment I put

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D137787#3923540 , @Hahnfeld wrote: >> Is it the reproducer? > > No, as I wrote: > >> Sadly this works fine in standalone Clang... Sorry, I don't understand well. Could you rewrite the reproducer in the style I wrote? And in

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-13 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. > Is it the reproducer? No, as I wrote: > Sadly this works fine in standalone Clang... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137787/new/ https://reviews.llvm.org/D137787 _

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-13 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. In D137787#3921673 , @Hahnfeld wrote: > I've trimmed the failing code down to > > #include > #include > #include > > template > struct SO { > void a() { > struct SI { > std::vector v; > }

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-11 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. I've trimmed the failing code down to #include #include #include template struct SO { void a() { struct SI { std::vector v; }; SI s; SI m(std::move(s)); } void g() { std::vector v{"a"}; } }; in

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-10 Thread Chuanqi Xu via Phabricator via cfe-commits
ChuanqiXu added a comment. I think it might be necessary/better to add a test for this. Otherwise, it looks not good to change this. > This fixes the assertion for a downstream case in ROOT/Cling with the > involvement of modules. If anyone has ideas how to test this, please let me > know...

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-10 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. This fixes the assertion for a downstream case in ROOT/Cling with the involvement of modules. If anyone has ideas how to test this, please let me know... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137787/new/ https://

[PATCH] D137787: [CodeGen] Relax assertion on generating destructor call

2022-11-10 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld created this revision. Hahnfeld added reviewers: v.g.vassilev, mantognini, ChuanqiXu, Bigcheese. Herald added a project: All. Hahnfeld requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. If the `Decl` pointers are not identical, `decla