[PATCH] D54344: [Clang][CodeGen][CXX]: Workaround __attribute((no_destroy)) crash/incorrect code generation.

Erik Pilkington via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Nov 11 17:11:36 PST 2018


erik.pilkington added a comment.

In https://reviews.llvm.org/D54344#1294960, @kristina wrote:

> In https://reviews.llvm.org/D54344#1294942, @erik.pilkington wrote:
>
> > LGTM too, with one small fix in test. Thanks for working on this!
>
>
> Well, I figured since the assertion being tripped was (before investigation) the only reliable way to notice the bug it makes sense for it to stay in, main concern being that should anything regress and assertions are off, the code generation is essentially undefined. Though a good argument for taking it out would be the fact that currently it's the only test that verifies IR generated with the attribute (last time I checked), but I would also imagine most people running tests would have assertion enabled (or debug) builds.


clang/test/CodeGenCXX/{always,no}_destroy.cpp also test the generated IR. I would prefer always running it, its an interesting code path that might break in other ways, so even if we can't test as thoroughly without assertions, its still worth it to run.

> Though I'm happy to revise the test to remove the assertion requirement, deferring to your judgement with regards to above.
> 
> Aside from that, are you happy for me to land this after the revision?

Yep, land away! Thanks again for fixing this.


Repository:
  rC Clang

https://reviews.llvm.org/D54344





More information about the cfe-commits mailing list