[PATCH] D134475: [clang-cl] Add support for [[msvc::constexpr]] C++11 attribute

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: clang/lib/AST/ExprConstant.cpp:9591-9594
+  bool IsMSConstexpr = Info.CurrentCall->CanEvalMSConstexpr &&
+                       OperatorNew->hasAttr<MSConstexprAttr>();
   if (OperatorNew->isReservedGlobalPlacementOperator() &&
+      (Info.CurrentCall->isStdFunction() || IsMSConstexpr) && !E->isArray()) {
----------------
RIscRIpt wrote:
> rsmith wrote:
> > Do we really need this change? Was our existing check of whether the caller is in namespace `std` not sufficient for MS' standard library? I'd strongly prefer not to have a documented, user-visible attribute that gives permission to use placement new directly.
> Yes, STL's `operator new` is defined in global namespace in [[ https://gist.github.com/RIscRIpt/9f0991f09f97eafc375fc73ea851a81b#file-vcruntime_new-h-L165 | vcruntime_new.h ]] (and all includes of this file are made from global namespace).
The existing code is checking whether the caller of `operator new` (eg, `std::construct_at`) is in namespace `std`, not whether the `operator new` itself is. Does MSVC's `construct_at` need this? It might if it uses a placement new indirectly via a function in a different namespace, but it seems likely to me that it doesn't.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134475