efriedma added inline comments.

================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5006
+    if (NeedsGlobalCtor || NeedsGlobalDtor)
+      DelayedCXXInitPosition[D] = ~0U;
+  } else {
----------------
zahiraam wrote:
> efriedma wrote:
> > zahiraam wrote:
> > > Do you agree this should be done only when one of those flags is on?
> > Yes, that's fine; I wasn't really paying close attention to the exact code. 
> >  Just wanted to make the point about the structure of the if statements, 
> > and code was the easiest way to explain it.
> > 
> > Maybe the outer if statement should actually be `if (isStaticInit(D, 
> > getLangOpts()) && NeedsGlobalCtor) {`.
> > 
> > On a related note, we might want to avoid the name "ctor", in case that 
> > accidentally conflicts with some user code; an "__"-prefixed name would be 
> > appropriate.
> >> Maybe the outer if statement should actually be if (isStaticInit(D, 
> >> getLangOpts()) && NeedsGlobalCtor) {
> Not sure about that! There are cases where (isStaticInit(D, getLangOpts())) = 
> true and NeedsGlobalCtor=false, but NeedsGlobalDtor=true. In which case a 
> __dtor needs to be emitted, no?
> 
> Writing the condition as you are proposing would actually not get me into the 
> body to emit the __dtor. Is that what we want?
EmitCXXGlobalVarDeclInitFunc should be able to handle that case.

Looking again, I'm a little concerned that in the isStaticInit() case, we're 
skipping a bunch of the logic in EmitCXXGlobalVarDeclInitFunc. EmitCXXCtorInit 
handles the basic cases correctly, but there are a lot of special cases in 
EmitCXXGlobalVarDeclInitFunc.


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

https://reviews.llvm.org/D137107

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to