[[msvc::constexpr]] placement new is not supported outside of std namespace

[RIscRIpt]

Initially #71300 replicated semantics of [[msvc::constexpr]] from MSVC, so that it was possible to use [[msvc::constexpr]] return ::new from non-std namespace (constexpr placement new). E.g. https://godbolt.org/z/7eKh5Envz

// RUN: %clang_cc1 -fms-compatibility -fms-compatibility-version=19.33 -std=c++20 -ast-dump %s | FileCheck %s

// CHECK: used operator new
// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} <col:17, col:23>
[[nodiscard]] [[msvc::constexpr]] inline void* __cdecl operator new(decltype(sizeof(void*)), void* p) noexcept { return p; }

// CHECK: used constexpr construct_at
// CHECK: AttributedStmt 0x{{[0-9a-f]+}} <col:46, col:88>
// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} <col:48, col:54>
// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} <col:66, col:88>
constexpr int* construct_at(int* p, int v) { [[msvc::constexpr]] return ::new (p) int(v); }

constexpr bool check_construct_at() { int x; return *construct_at(&x, 42) == 42; }

static_assert(check_construct_at());

However, during the review a concern over changes in PointerExprEvaluator::VisitCXXNewExpr was raised:

   bool IsNothrow = false;
   bool IsPlacement = false;
+  bool IsMSConstexpr = Info.CurrentCall->CanEvalMSConstexpr &&
+                       OperatorNew->hasAttr<MSConstexprAttr>();
  if (OperatorNew->isReservedGlobalPlacementOperator() &&
-     Info.CurrentCall->isStdFunction() && !E->isArray()) {
+     (Info.CurrentCall->isStdFunction() || IsMSConstexpr) && !E->isArray()) {

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.

I thought it was blocking the PR, so to speed up the review, I decided to move decision-making around this feature to this follow-up issue.

Originally posted by @RIscRIpt in #71300 (comment)

[llvmbot]

@llvm/issue-subscribers-clang-frontend

Author: Richard Dzenis (RIscRIpt)

Initially #71300 replicated semantics of `[[msvc::constexpr]]` from MSVC, so that it was possible to use `[[msvc::constexpr]] return ::new` from non-std namespace (constexpr placement new). E.g. https://godbolt.org/z/7eKh5Envz ```cpp // RUN: %clang_cc1 -fms-compatibility -fms-compatibility-version=19.33 -std=c++20 -ast-dump %s | FileCheck %s

// CHECK: used operator new
// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} <col:17, col:23>
[[nodiscard]] [[msvc::constexpr]] inline void* __cdecl operator new(decltype(sizeof(void*)), void* p) noexcept { return p; }

// CHECK: used constexpr construct_at
// CHECK: AttributedStmt 0x{{[0-9a-f]+}} <col:46, col:88>
// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} <col:48, col:54>
// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} <col:66, col:88>
constexpr int* construct_at(int* p, int v) { [[msvc::constexpr]] return ::new (p) int(v); }

constexpr bool check_construct_at() { int x; return *construct_at(&x, 42) == 42; }

static_assert(check_construct_at());

However, during the review a concern over changes in `PointerExprEvaluator::VisitCXXNewExpr` was raised:
```diff
   bool IsNothrow = false;
   bool IsPlacement = false;
+  bool IsMSConstexpr = Info.CurrentCall->CanEvalMSConstexpr &&
+                       OperatorNew->hasAttr<MSConstexprAttr>();
  if (OperatorNew->isReservedGlobalPlacementOperator() &&
-     Info.CurrentCall->isStdFunction() && !E->isArray()) {
+     (Info.CurrentCall->isStdFunction() || IsMSConstexpr) && !E->isArray()) {

> 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.

I thought it was blocking the PR, so to speed up the review, I decided to move decision-making around this feature to this follow-up issue.

Originally posted by @RIscRIpt in #71300 (comment)

[cor3ntin]

Is that still necessary now that we support placement new in constexpr?

[RIscRIpt]

Is that still necessary now that we support placement new in constexpr?

I've quickly tried in https://godbolt.org/z/9981fczEo if their clang version is up-to-date, then yes, we still need this, if we want to match MSVC behavior.

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: Richard Dzenis (RIscRIpt)

Initially #71300 replicated semantics of `[[msvc::constexpr]]` from MSVC, so that it was possible to use `[[msvc::constexpr]] return ::new` from non-std namespace (constexpr placement new). E.g. https://godbolt.org/z/7eKh5Envz ```cpp // RUN: %clang_cc1 -fms-compatibility -fms-compatibility-version=19.33 -std=c++20 -ast-dump %s | FileCheck %s

// CHECK: used operator new
// CHECK: MSConstexprAttr 0x{{[0-9a-f]+}} <col:17, col:23>
[[nodiscard]] [[msvc::constexpr]] inline void* __cdecl operator new(decltype(sizeof(void*)), void* p) noexcept { return p; }

// CHECK: used constexpr construct_at
// CHECK: AttributedStmt 0x{{[0-9a-f]+}} <col:46, col:88>
// CHECK-NEXT: MSConstexprAttr 0x{{[0-9a-f]+}} <col:48, col:54>
// CHECK-NEXT: ReturnStmt 0x{{[0-9a-f]+}} <col:66, col:88>
constexpr int* construct_at(int* p, int v) { [[msvc::constexpr]] return ::new (p) int(v); }

constexpr bool check_construct_at() { int x; return *construct_at(&x, 42) == 42; }

static_assert(check_construct_at());

However, during the review a concern over changes in `PointerExprEvaluator::VisitCXXNewExpr` was raised:
```diff
   bool IsNothrow = false;
   bool IsPlacement = false;
+  bool IsMSConstexpr = Info.CurrentCall->CanEvalMSConstexpr &&
+                       OperatorNew->hasAttr<MSConstexprAttr>();
  if (OperatorNew->isReservedGlobalPlacementOperator() &&
-     Info.CurrentCall->isStdFunction() && !E->isArray()) {
+     (Info.CurrentCall->isStdFunction() || IsMSConstexpr) && !E->isArray()) {

> 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.

I thought it was blocking the PR, so to speed up the review, I decided to move decision-making around this feature to this follow-up issue.

Originally posted by @RIscRIpt in #71300 (comment)

[cor3ntin]

This should be easy to change as described in the original issue description now that we support placement new in c++26 mode

[RIscRIpt]

This should be easy to change as described in the original issue description now that we support placement new in c++26 mode

Right. Since you've marked this issue as a good first issue https://github.com/llvm/llvm-project/contribute , I'll allow a few weeks for anybody interested to pick it up; if no volunteers, I'll submit a PR later.