Skip to content

[clang][bytecode] Fix diagnosing replaceable global allocator functions #126717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2025

Conversation

tbaederr
Copy link
Contributor

Don't return true here in InvalidNewDeleteExpr just because we are in C++26 mode. This invalid there as well.

Testcase reduced from libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.pass.cpp

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 11, 2025
@llvmbot
Copy link
Member

llvmbot commented Feb 11, 2025

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

Don't return true here in InvalidNewDeleteExpr just because we are in C++26 mode. This invalid there as well.

Testcase reduced from libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.pass.cpp


Full diff: https://github.com/llvm/llvm-project/pull/126717.diff

2 Files Affected:

  • (modified) clang/lib/AST/ByteCode/Interp.cpp (+18-14)
  • (modified) clang/test/AST/ByteCode/cxx26.cpp (+21-2)
diff --git a/clang/lib/AST/ByteCode/Interp.cpp b/clang/lib/AST/ByteCode/Interp.cpp
index bf48139f57c0f09..c80be094856b086 100644
--- a/clang/lib/AST/ByteCode/Interp.cpp
+++ b/clang/lib/AST/ByteCode/Interp.cpp
@@ -1564,34 +1564,38 @@ bool CheckNewTypeMismatch(InterpState &S, CodePtr OpPC, const Expr *E,
 bool InvalidNewDeleteExpr(InterpState &S, CodePtr OpPC, const Expr *E) {
   assert(E);
 
-  if (S.getLangOpts().CPlusPlus26)
-    return true;
-
-  const auto &Loc = S.Current->getSource(OpPC);
-
   if (const auto *NewExpr = dyn_cast<CXXNewExpr>(E)) {
     const FunctionDecl *OperatorNew = NewExpr->getOperatorNew();
 
-    if (!S.getLangOpts().CPlusPlus26 && NewExpr->getNumPlacementArgs() > 0) {
+    if (NewExpr->getNumPlacementArgs() > 0) {
       // This is allowed pre-C++26, but only an std function.
-      if (S.Current->isStdFunction())
+      if (S.getLangOpts().CPlusPlus26 || S.Current->isStdFunction())
         return true;
-      S.FFDiag(Loc, diag::note_constexpr_new_placement)
+      S.FFDiag(S.Current->getSource(OpPC), diag::note_constexpr_new_placement)
           << /*C++26 feature*/ 1 << E->getSourceRange();
-    } else if (NewExpr->getNumPlacementArgs() == 1 &&
-               !OperatorNew->isReservedGlobalPlacementOperator()) {
-      S.FFDiag(Loc, diag::note_constexpr_new_placement)
-          << /*Unsupported*/ 0 << E->getSourceRange();
     } else if (!OperatorNew->isReplaceableGlobalAllocationFunction()) {
-      S.FFDiag(Loc, diag::note_constexpr_new_non_replaceable)
+      S.FFDiag(S.Current->getSource(OpPC),
+               diag::note_constexpr_new_non_replaceable)
           << isa<CXXMethodDecl>(OperatorNew) << OperatorNew;
+      return false;
+    } else if (!S.getLangOpts().CPlusPlus26 &&
+               NewExpr->getNumPlacementArgs() == 1 &&
+               !OperatorNew->isReservedGlobalPlacementOperator()) {
+      if (!S.getLangOpts().CPlusPlus26) {
+        S.FFDiag(S.Current->getSource(OpPC), diag::note_constexpr_new_placement)
+            << /*Unsupported*/ 0 << E->getSourceRange();
+        return false;
+      }
+      return true;
     }
   } else {
     const auto *DeleteExpr = cast<CXXDeleteExpr>(E);
     const FunctionDecl *OperatorDelete = DeleteExpr->getOperatorDelete();
     if (!OperatorDelete->isReplaceableGlobalAllocationFunction()) {
-      S.FFDiag(Loc, diag::note_constexpr_new_non_replaceable)
+      S.FFDiag(S.Current->getSource(OpPC),
+               diag::note_constexpr_new_non_replaceable)
           << isa<CXXMethodDecl>(OperatorDelete) << OperatorDelete;
+      return false;
     }
   }
 
diff --git a/clang/test/AST/ByteCode/cxx26.cpp b/clang/test/AST/ByteCode/cxx26.cpp
index 0b0e2b21e8201e7..2ac3b21695ddd80 100644
--- a/clang/test/AST/ByteCode/cxx26.cpp
+++ b/clang/test/AST/ByteCode/cxx26.cpp
@@ -1,10 +1,29 @@
 // RUN: %clang_cc1 -std=c++26 -fsyntax-only -fcxx-exceptions -verify=ref,both %s
 // RUN: %clang_cc1 -std=c++26 -fsyntax-only -fcxx-exceptions -verify=expected,both %s -fexperimental-new-constant-interpreter
 
-// both-no-diagnostics
-
 namespace VoidCast {
   constexpr void* p = nullptr;
   constexpr int* q = static_cast<int*>(p);
   static_assert(q == nullptr);
 }
+
+namespace ReplaceableAlloc {
+  struct F {
+    static void* operator new(unsigned long n) {
+      return nullptr; // both-warning {{should not return a null pointer}}
+    }
+  };
+
+  constexpr F *createF() {
+    return new F(); // both-note {{call to class-specific 'operator new'}}
+  }
+
+  constexpr bool foo() {
+    F *f = createF(); // both-note {{in call to}}
+
+    delete f;
+    return true;
+  }
+  static_assert(foo()); // both-error {{not an integral constant expression}} \
+                        // both-note {{in call to}}
+}

@tbaederr tbaederr force-pushed the replaceable-global-alloc branch from 8a271fb to 4d6c6ae Compare February 11, 2025 13:07
Don't return true here in InvalidNewDeleteExpr just because we are in
C++26 mode. This invalid there as well.

Testcase reduced from libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.pass.cpp
@tbaederr tbaederr force-pushed the replaceable-global-alloc branch from 4d6c6ae to 111f301 Compare February 11, 2025 14:57
@tbaederr tbaederr merged commit 9387fd9 into llvm:main Feb 11, 2025
8 checks passed
Icohedron pushed a commit to Icohedron/llvm-project that referenced this pull request Feb 11, 2025
…ns (llvm#126717)

Don't return true here in InvalidNewDeleteExpr just because we are in
C++26 mode. This invalid there as well.

Testcase reduced from
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.pass.cpp
joaosaffran pushed a commit to joaosaffran/llvm-project that referenced this pull request Feb 14, 2025
…ns (llvm#126717)

Don't return true here in InvalidNewDeleteExpr just because we are in
C++26 mode. This invalid there as well.

Testcase reduced from
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.pass.cpp
sivan-shani pushed a commit to sivan-shani/llvm-project that referenced this pull request Feb 24, 2025
…ns (llvm#126717)

Don't return true here in InvalidNewDeleteExpr just because we are in
C++26 mode. This invalid there as well.

Testcase reduced from
libcxx/test/std/utilities/smartptr/unique.ptr/unique.ptr.create/make_unique_for_overwrite.pass.cpp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants