[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

[Eli Friedman via Phabricator via cfe-commits]

efriedma added inline comments.


================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4852
+          if (!CD->isTrivial() && !D->getTLSKind())
+            NeedsGlobalCtor = true;
+        }
----------------
zahiraam wrote:
> efriedma wrote:
> > I have no idea what this code is supposed to do.
> I have removed from this the thread_local variables for being processed the same way. Maybe they should be included, not sure?
> 
> For the rest is to differentiate between these cases. I found the 2nd test case while doing some unit testing.
> 
>   struct B {
>   constexpr B() {}
>   ~B() {};
>   };
>   constinit B b;
> 
> 
> and
> 
>   struct A {
>     int s;
>     ~A();
>   };
> 
>   struct D : A {};
>   D d1 = {};
> 
> I think the second test case is not supposed to EmitDeclInit in EmitCXXGlobalVarDeclInit right? But since now tryEmitForInitializer is returning an Initializer, then NeedsGlobalCtor = true; 
> 
> Actually, when I removed this code, I have 2 LIT tests failing with the same crash. 
> WDYT?
> 
> 
I'm not sure how thread-local var init works on Windows off the top of my head; I'd need to check.

For the given testcases, neither one of those cases should be calling EmitDeclInit; we use constant initialization for both cases, so the global init func should just be registering the destructors.  We shouldn't need to examine the initializer beyond checking whether tryEmitForInitializer succeeds.

Not sure what's crashing from your description.


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

https://reviews.llvm.org/D137107