[PATCH] D145369: Emit const globals with constexpr destructor as constant LLVM values

[Hans Wennborg via Phabricator via cfe-commits]

hans marked an inline comment as done.
hans added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4344
+             (Record->hasTrivialDestructor() ||
+              Record->hasConstexprDestructor());
   }
----------------
efriedma wrote:
> For the purposes of CodeGen, checking `Record->hasConstexprDestructor()` isn't really helpful.  Even if `Record->hasConstexprDestructor()` is true, a destructor can still have side-effects.  Since the callers need to handle that possibility anyway, you might as well just skip the `Record->hasTrivialDestructor() || Record->hasConstexprDestructor()` check.
Maybe what we want is to also have an `ExcludeDtor` param that callers which have checked that no dtor call is needed could set?


================
Comment at: clang/test/CodeGenCXX/static-init.cpp:181
+#if __cplusplus >= 202002L
+// A const object with constexpr destructor can be emitted as a constant.
+namespace test5 {
----------------
efriedma wrote:
> I don't see how this works.  For a static local variable, in general, we register the destructor with __cxa_atexit, and the destructor can have arbitrary side-effects.  For example:
> 
> ```
> extern void foo();
> struct A {
>   constexpr A() : x(1) {}
>   constexpr ~A() {if (x) foo();}
>   int x;
> };
> const int *f() {
>   static const A a{};
>   return &a.x;
> }
> ```
Hmm, I guess I assumed the destructor being constexpr meant it wouldn't have side effects, which of course isn't necessarily true.. thanks for the example.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D145369/new/

https://reviews.llvm.org/D145369