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

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.





================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4852
+          if (!CD->isTrivial() && !D->getTLSKind())
+            NeedsGlobalCtor = true;
+        }
----------------
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?




================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5009
+  if (isStaticInit(D, getLangOpts()) && NeedsGlobalCtor && NeedsGlobalDtor) {
+    EmitCXXCtorInit(D, GV, true, 201, llvm::StringLiteral("ctor"), false);
+    EmitCXXCtorInit(D, GV, false, 65535, llvm::StringLiteral("dtor"), true);
----------------
efriedma wrote:
> I think you want to use priority 201 whether or not there's a destructor.
Is that what you mean?


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

https://reviews.llvm.org/D137107