efriedma added a comment.
Looking at this again, I just thought of something: in C mode, we probably
don't want to generate global constructors, so we might want to continue to
print an error in that case.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:13896
+ Diag(var->getLocation(), diag::err_constexpr_var_requires_const_init)
+ << var << Init->getSourceRange();
+ }
----------------
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.
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
