efriedma added inline comments.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:13896
+ Diag(var->getLocation(), diag::err_constexpr_var_requires_const_init)
+ << var << Init->getSourceRange();
+ }
----------------
rnk wrote:
> zahiraam wrote:
> > efriedma wrote:
> > > zahiraam wrote:
> > > > efriedma wrote:
> > > > > zahiraam wrote:
> > > > > > efriedma wrote:
> > > > > > > I don't understand why this diagnostic is necessary.
> > > > > > Now that the DLLImport variable can be a constant, HasConstInit is
> > > > > > returning true (it was before returning false) and when the
> > > > > > variable is global a diagnostic should be reported.
> > > > > >
> > > > > > This:
> > > > > > extern int _declspec(dllimport) val;
> > > > > > constexpr int& val_ref = val;
> > > > > >
> > > > > > should report a diagnostic, but this:
> > > > > >
> > > > > > int foo() {
> > > > > > extern int _declspec(dllimport) val;
> > > > > > constexpr int& val_ref = val;
> > > > > > }
> > > > > >
> > > > > > shouldn't report a diagnostic.
> > > > > MSVC doesn't report a diagnostic for either of your examples?
> > > > So, these shouldn't fail anymore?
> > > > https://github.com/llvm/llvm-project/blob/main/clang/test/SemaCXX/PR19955.cpp#L5
> > > > https://github.com/llvm/llvm-project/blob/main/clang/test/SemaCXX/PR19955.cpp#L8
> > > > https://github.com/llvm/llvm-project/blob/main/clang/test/SemaCXX/dllimport-constexpr.cpp#L44
> > > Right, they shouldn't fail.
> > >
> > > My perspective is that since we're able to give those the expected
> > > semantics, we should just do that. The only reason that "dllimport" is
> > > relevant at all is that there isn't a relocation for it, but our plan is
> > > to cover that up with a runtime constructor. So the user shouldn't need
> > > to know there's trickery behind the scenes unless they disassemble the
> > > binary.
> > Got it! Thanks. Will remove the diagnostic here then.
> I worry that some of our users really do care quite a lot about constant
> initialization. I think emitting a special runtime initializer may be our
> best option (mingw does it through other means), but this is likely to be
> disruptive. Chrome has code which runs before CRT initialization to do some
> early sandboxing. ASan also competes to initialize shadow memory as early as
> possible (search compiler-rt for `.CRT`).
>
> I worry that these users will think, "well, I've used [[constinit]] and
> constexpr, so this global should *definitely* be initialized during startup,
> and I won't have to worry about init order fiasco problems!", and then they
> will be surprised to find that this is not the case. Should we care? What can
> we do about this, realistically? Leaving behind this portability pothole of
> "dlimport symbols just aren't constexpr" seems worse. Our current behavior
> may be unduly influenced by my particular values: I think constant
> initialization is really important, I really care a lot about whether things
> are relocated or initialized at runtime.
>
> I don't see any good solutions, so please carry on, but if you have any good
> ideas, please share. :)
For the obscure cases where the user actually cares about that sort of thing,
we can make -Wglobal-constructors trigger, I guess? I'm not sure what else we
could do that doesn't break the feature.
------
It might be worth trying to reach out to Microsoft to figure out a common
design for the runtime initialization thing. MSVC appears to emit broken code
in certain cases at the moment. For example, in the following, "z" is
correctly initialized at runtime, but "z2" emits a broken direct reference.
```
_declspec(dllimport) int x;
constexpr int *y = &x;
int *z = &x;
int *z2 = y;
```
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
