zahiraam added inline comments.
================
Comment at: clang/lib/Sema/SemaDecl.cpp:13896
+ Diag(var->getLocation(), diag::err_constexpr_var_requires_const_init)
+ << var << Init->getSourceRange();
+ }
----------------
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.
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
