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