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

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam marked 3 inline comments as done.
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:
> > > 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


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137107/new/

https://reviews.llvm.org/D137107