efriedma added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprConstant.cpp:1926
+ DiagnosticsEngine &Diags = CGM.getContext().getDiagnostics();
+ Diags.Report(diag::warn_for_global_ctor_for_dllimport) << D;
+ return nullptr;
----------------
I think this will trigger in cases we don't actually want it to; we only want
to warn when we actually generate a global constructor, not when we end up
falling back to generated code within a function. (For example, we sometimes
constant-evaluate local variables.)
================
Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5006
+ if (NeedsGlobalCtor || NeedsGlobalDtor)
+ DelayedCXXInitPosition[D] = ~0U;
+ } else {
----------------
zahiraam wrote:
> efriedma wrote:
> > 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.
> I have left the condition as it was to make sure no cases are left. What
> other cases are you thinking of?
>
EmitCXXGlobalVarDeclInitFunc has special cases for CUDA, OpenMP, thread-local
variables, the InitSeg attribute, and inline variables.
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
