[PATCH] D137107: Allow MS extension: support of constexpr with __declspec(dllimport).

[Eli Friedman via Phabricator via cfe-commits]

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