[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/CGDeclCXX.cpp:572
     PrioritizedCXXGlobalInits.push_back(std::make_pair(Key, Fn));
+  } else if (D->hasConstantInitialization() && !(D->hasAttr<ConstInitAttr>())) {
+    OrderGlobalInitsOrStermFinalizers Key(201,
----------------
zahiraam wrote:
> efriedma wrote:
> > zahiraam wrote:
> > > efriedma wrote:
> > > > How is ConstInitAttr relevant here?
> > > This change made (without the !(D->hasAttr<ConstInitAttr>()) made the LIT behavior of aix-static-init.cpp. The IR generated for 
> > > namespace test3 {
> > >   struct Test3 {
> > >     constexpr Test3() {};
> > >     ~Test3() {};
> > >   };
> > > 
> > >   constinit Test3 t;
> > > } // namespace test3
> > > 
> > > was different. I would have thought that the change we made for constexpr wouldn't affter constinit? 
> > I think the significant bit there isn't the use of constinit; it's the non-trivial destructor.  I think the priority modification should only affect constructors, not destructors.  (Not sure how to make that work, at first glance.)
> Let's see if this is an acceptable solution.
To fake constant initialization, we need to initialize the variable before anything else runs.  But the rearranged prioritization isn't supposed to affect the destructor.  From [basic.start.term]: "If an object is initialized statically, the object is destroyed in the same order as if the object was dynamically initialized."

What you're doing here isn't exactly implementing that.  What you're doing here is delaying both the initialization and the destruction if the variable has a non-trivial destructor.  We need to separate the two to get the behavior we want.


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

https://reviews.llvm.org/D137107