[clang] [clang] Fix parenthesized list initialization of arrays not working with `new` (PR #76976)

Alan Zhao via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 4 11:17:56 PST 2024


https://github.com/alanzhao1 updated https://github.com/llvm/llvm-project/pull/76976

>From ee4e3c8634bb876166ee753a4ebcbf3c1699a175 Mon Sep 17 00:00:00 2001
From: Alan Zhao <ayzhao at google.com>
Date: Wed, 3 Jan 2024 12:29:21 -0800
Subject: [PATCH 1/4] [clang] Fix parenthesized list initialization of arrays
 not working with `new`

This bug is caused by parenthesized list initialization not being
implemented in `CodeGenFunction::EmitNewArrayInitializer(...)`.

Parenthesized list initialization of `struct`s  with `operator new`
already works in Clang and is not affected by this bug.

Additionally, fix the test new-delete.cpp as it incorrectly assumes that
using parentheses with operator new to initialize arrays is illegal for
C++ versions >= C++17.

Fixes #68198
---
 clang/docs/ReleaseNotes.rst                |  3 +
 clang/lib/CodeGen/CGExprCXX.cpp            | 17 ++---
 clang/lib/Sema/SemaExprCXX.cpp             |  6 +-
 clang/test/CodeGen/paren-list-agg-init.cpp | 72 ++++++++++++++++++++++
 clang/test/SemaCXX/new-delete.cpp          | 20 +++---
 5 files changed, 98 insertions(+), 20 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c7bf162426a68c..211fd62a1ee85f 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -844,6 +844,9 @@ Bug Fixes to C++ Support
 - Fix crash when parsing nested requirement. Fixes:
   (`#73112 <https://github.com/llvm/llvm-project/issues/73112>`_)
 
+- Clang now allows parenthesized initialization of arrays in `operator new[]`.
+  Fixes: (`#68198 <https://github.com/llvm/llvm-project/issues/68198>`_)
+
 Bug Fixes to AST Handling
 ^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed an import failure of recursive friend class template.
diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 98ae56e2df8818..72c61bfb5ec344 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -1038,11 +1038,13 @@ void CodeGenFunction::EmitNewArrayInitializer(
     return true;
   };
 
+  const InitListExpr *ILE = dyn_cast<InitListExpr>(Init);
+  const CXXParenListInitExpr *CPLIE = dyn_cast<CXXParenListInitExpr>(Init);
   // If the initializer is an initializer list, first do the explicit elements.
-  if (const InitListExpr *ILE = dyn_cast<InitListExpr>(Init)) {
+  if (ILE || CPLIE) {
     // Initializing from a (braced) string literal is a special case; the init
     // list element does not initialize a (single) array element.
-    if (ILE->isStringLiteralInit()) {
+    if (ILE && ILE->isStringLiteralInit()) {
       // Initialize the initial portion of length equal to that of the string
       // literal. The allocation must be for at least this much; we emitted a
       // check for that earlier.
@@ -1073,7 +1075,7 @@ void CodeGenFunction::EmitNewArrayInitializer(
       return;
     }
 
-    InitListElements = ILE->getNumInits();
+    InitListElements = ILE ? ILE->getNumInits() : CPLIE->getInitExprs().size();
 
     // If this is a multi-dimensional array new, we will initialize multiple
     // elements with each init list element.
@@ -1101,7 +1103,8 @@ void CodeGenFunction::EmitNewArrayInitializer(
     }
 
     CharUnits StartAlign = CurPtr.getAlignment();
-    for (unsigned i = 0, e = ILE->getNumInits(); i != e; ++i) {
+    ArrayRef<Expr *> InitExprs = ILE ? ILE->inits() : CPLIE->getInitExprs();
+    for (unsigned i = 0; i < InitExprs.size(); ++i) {
       // Tell the cleanup that it needs to destroy up to this
       // element.  TODO: some of these stores can be trivially
       // observed to be unnecessary.
@@ -1111,8 +1114,8 @@ void CodeGenFunction::EmitNewArrayInitializer(
       // FIXME: If the last initializer is an incomplete initializer list for
       // an array, and we have an array filler, we can fold together the two
       // initialization loops.
-      StoreAnyExprIntoOneUnit(*this, ILE->getInit(i),
-                              ILE->getInit(i)->getType(), CurPtr,
+      Expr *IE = InitExprs[i];
+      StoreAnyExprIntoOneUnit(*this, IE, IE->getType(), CurPtr,
                               AggValueSlot::DoesNotOverlap);
       CurPtr = Address(Builder.CreateInBoundsGEP(
                            CurPtr.getElementType(), CurPtr.getPointer(),
@@ -1122,7 +1125,7 @@ void CodeGenFunction::EmitNewArrayInitializer(
     }
 
     // The remaining elements are filled with the array filler expression.
-    Init = ILE->getArrayFiller();
+    Init = ILE ? ILE->getArrayFiller() : CPLIE->getArrayFiller();
 
     // Extract the initializer for the individual array elements by pulling
     // out the array filler from all the nested initializer lists. This avoids
diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 4ae04358d5df7c..71e420648ce7af 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -1947,11 +1947,11 @@ Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
 }
 
 static bool isLegalArrayNewInitializer(CXXNewInitializationStyle Style,
-                                       Expr *Init) {
+                                       Expr *Init, bool IsCPlusPlus20) {
   if (!Init)
     return true;
   if (ParenListExpr *PLE = dyn_cast<ParenListExpr>(Init))
-    return PLE->getNumExprs() == 0;
+    return IsCPlusPlus20 || PLE->getNumExprs() == 0;
   if (isa<ImplicitValueInitExpr>(Init))
     return true;
   else if (CXXConstructExpr *CCE = dyn_cast<CXXConstructExpr>(Init))
@@ -2403,7 +2403,7 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
   // Array 'new' can't have any initializers except empty parentheses.
   // Initializer lists are also allowed, in C++11. Rely on the parser for the
   // dialect distinction.
-  if (ArraySize && !isLegalArrayNewInitializer(InitStyle, Initializer)) {
+  if (ArraySize && !isLegalArrayNewInitializer(InitStyle, Initializer, getLangOpts().CPlusPlus20)) {
     SourceRange InitRange(Exprs.front()->getBeginLoc(),
                           Exprs.back()->getEndLoc());
     Diag(StartLoc, diag::err_new_array_init_args) << InitRange;
diff --git a/clang/test/CodeGen/paren-list-agg-init.cpp b/clang/test/CodeGen/paren-list-agg-init.cpp
index 0e68beb5c37066..9618b6ddf14558 100644
--- a/clang/test/CodeGen/paren-list-agg-init.cpp
+++ b/clang/test/CodeGen/paren-list-agg-init.cpp
@@ -513,3 +513,75 @@ namespace gh61567 {
     I(0);
   }
 }
+
+namespace gh68198 {
+  // CHECK: define {{.*}} void @{{.*foo25.*}} {
+  // CHECK-NEXT: entry
+  // CHECK-NEXT: [[ARR_8:%.*arr8.*]] = alloca ptr, align 8
+  // CHECK-NEXT: [[CALL_PTR:%.*]] = call noalias noundef nonnull ptr @_Znam(i64 noundef 8)
+  // CHECK-NEXT: store i32 1, ptr [[CALL_PTR]], align 4
+  // CHECK-NEXT: [[ARRAY_EXP_NEXT:%.*]] = getelementptr inbounds i32, ptr [[CALL_PTR]], i64 1
+  // CHECK-NEXT: store i32 2, ptr [[ARRAY_EXP_NEXT]], align 4
+  // CHECK-NEXT: [[ARRAY_EXP_NEXT1:%.*]] = getelementptr inbounds i32, ptr [[ARRAY_EXP_NEXT]], i64 1
+  // CHECK-NEXT: store ptr [[CALL_PTR]], ptr %arr8, align 8
+  // CHECK-NEXT: ret void
+  void foo25() {
+    int* arr8 = new int[](1, 2);
+  }
+
+  // CHECK: define {{.*}} void @{{.*foo26.*}} {
+  // CHECK-NEXT: entry
+  // CHECK-NEXT: [[ARR_9:%.*arr9.*]] = alloca ptr, align 8
+  // CHECK-NEXT: [[CALL_PTR:%.*]] = call noalias noundef nonnull ptr @_Znam(i64 noundef 16) #7
+  // CHECK-NEXT: store i32 1, ptr [[CALL_PTR]], align 4
+  // CHECK-NEXT: [[ARRAY_EXP_NEXT:%.*]] = getelementptr inbounds i32, ptr [[CALL_PTR]], i64 1
+  // CHECK-NEXT: store i32 2, ptr [[ARRAY_EXP_NEXT]], align 4
+  // CHECK-NEXT: [[ARRAY_EXP_NEXT1:%.*]] = getelementptr inbounds i32, ptr [[ARRAY_EXP_NEXT]], i64 1
+  // CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 4 [[ARRAY_EXP_NEXT1]], i8 0, i64 8, i1 false)
+  // CHECK-NEXT: store ptr [[CALL_PTR]], ptr [[ARR_9]], align 8
+  // CHECK-NEXT: ret void
+  void foo26() {
+    int* arr9 = new int[4](1, 2);
+  }
+
+  // CHECK: define {{.*}} void @{{.*foo27.*}} {
+  // CHECK-NEXT: entry
+  // CHECK-NEXT: [[ARR_10:%.*arr10.*]] = alloca ptr, align 8
+  // CHECK-NEXT: [[CALL_PTR]] = call noalias noundef nonnull ptr @_Znam(i64 noundef 16)
+  // CHECK-NEXT: [[ARRAYINIT_BEGIN:%.*]] = getelementptr inbounds [2 x i32], ptr [[CALL]], i64 0, i64 0
+  // CHECK-NEXT: store i32 1, ptr [[ARRAYINIT_BEGIN]], align 4
+  // CHECK-NEXT: [[ARRAYINIT_ELEMENT:%.*]] = getelementptr inbounds i32, ptr [[ARRAYINIT_BEGIN]], i64 1
+  // CHECK-NEXT: store i32 2, ptr [[ARRAYINIT_ELEMENT]], align 4
+  // CHECK-NEXT: [[ARRAY_EXP_NEXT:%.*]] = getelementptr inbounds [2 x i32], ptr %call, i64 1
+  // CHECK-NEXT: [[ARRAYINIT_BEGIN1:%.*]] = getelementptr inbounds [2 x i32], ptr [[ARRAY_EXP_NEXT]], i64 0, i64 0
+  // CHECK-NEXT: store i32 3, ptr [[ARRAYINIT_BEGIN1]], align 4
+  // CHECK-NEXT: [[ARRAYINIT_ELEMENT2:%.*]] = getelementptr inbounds i32, ptr [[ARRAYINIT_BEGIN1]], i64 1
+  // CHECK-NEXT: store i32 4, ptr [[ARRAYINIT_ELEMENT2]], align 4
+  // CHECK-NEXT: [[ARRAY_EXP_NEXT3:%.*]] = getelementptr inbounds [2 x i32], ptr [[ARRAY_EXP_NEXT]], i64 1
+  // CHECK-NEXT: store ptr [[CALL_PTR]], ptr [[ARR_10]], align 8
+  // CHECK-NEXT: ret void
+  void foo27() {
+    void* arr10 = new int[][2]({1, 2}, {3, 4});
+  }
+
+  // CHECK: define {{.*}} void @{{.*foo28.*}} {
+  // CHECK-NEXT: entry
+  // CHECK-NEXT: [[ARR_11:%.*arr11.*]] = alloca ptr, align 8
+  // CHECK-NEXT: [[CALL_PTR]] = call noalias noundef nonnull ptr @_Znam(i64 noundef 32)
+  // CHECK-NEXT: [[ARRAYINIT_BEGIN:%.*]] = getelementptr inbounds [2 x i32], ptr [[CALL]], i64 0, i64 0
+  // CHECK-NEXT: store i32 5, ptr [[ARRAYINIT_BEGIN]], align 4
+  // CHECK-NEXT: [[ARRAYINIT_ELEMENT:%.*]] = getelementptr inbounds i32, ptr [[ARRAYINIT_BEGIN]], i64 1
+  // CHECK-NEXT: store i32 6, ptr [[ARRAYINIT_ELEMENT]], align 4
+  // CHECK-NEXT: [[ARRAY_EXP_NEXT:%.*]] = getelementptr inbounds [2 x i32], ptr %call, i64 1
+  // CHECK-NEXT: [[ARRAYINIT_BEGIN1:%.*]] = getelementptr inbounds [2 x i32], ptr [[ARRAY_EXP_NEXT]], i64 0, i64 0
+  // CHECK-NEXT: store i32 7, ptr [[ARRAYINIT_BEGIN1]], align 4
+  // CHECK-NEXT: [[ARRAYINIT_ELEMENT2:%.*]] = getelementptr inbounds i32, ptr [[ARRAYINIT_BEGIN1]], i64 1
+  // CHECK-NEXT: store i32 8, ptr [[ARRAYINIT_ELEMENT2]], align 4
+  // CHECK-NEXT: [[ARRAY_EXP_NEXT3:%.*]] = getelementptr inbounds [2 x i32], ptr [[ARRAY_EXP_NEXT]], i64 1
+  // CHECK-NEXT: call void @llvm.memset.p0.i64(ptr align 4 [[ARRAY_EXP_NEXT3]], i8 0, i64 16, i1 false)
+  // CHECK-NEXT: store ptr [[CALL_PTR]], ptr [[ARR_11]], align 8
+  // CHECK-NEXT: ret void
+  void foo28() {
+    void* arr11 = new int[4][2]({5, 6}, {7, 8});
+  }
+}
diff --git a/clang/test/SemaCXX/new-delete.cpp b/clang/test/SemaCXX/new-delete.cpp
index 0270e42b7389fe..ec406260b6a391 100644
--- a/clang/test/SemaCXX/new-delete.cpp
+++ b/clang/test/SemaCXX/new-delete.cpp
@@ -1,7 +1,7 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++98
-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++11
-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++14
-// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx17 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null %std_cxx17-
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++11
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++14
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx17,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++17
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx17 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++20
 
 // FIXME Location is (frontend)
 // cxx17-note@*:* {{candidate function not viable: requires 2 arguments, but 3 were provided}}
@@ -340,23 +340,23 @@ namespace PR5918 { // Look for template operator new overloads.
 namespace Test1 {
 
 void f() {
-  (void)new int[10](1, 2); // expected-error {{array 'new' cannot have initialization arguments}}
+  (void)new int[10](1, 2); // precxx20-error {{array 'new' cannot have initialization arguments}}
 
   typedef int T[10];
-  (void)new T(1, 2); // expected-error {{array 'new' cannot have initialization arguments}}
+  (void)new T(1, 2); // precxx20-error {{array 'new' cannot have initialization arguments}}
 }
 
 template<typename T>
 void g(unsigned i) {
-  (void)new T[1](i); // expected-error {{array 'new' cannot have initialization arguments}}
+  (void)new T[1](i); // precxx20-error {{array 'new' cannot have initialization arguments}}
 }
 
 template<typename T>
 void h(unsigned i) {
-  (void)new T(i); // expected-error {{array 'new' cannot have initialization arguments}}
+  (void)new T(i); // precxx20-error {{array 'new' cannot have initialization arguments}}
 }
 template void h<unsigned>(unsigned);
-template void h<unsigned[10]>(unsigned); // expected-note {{in instantiation of function template specialization 'Test1::h<unsigned int[10]>' requested here}}
+template void h<unsigned[10]>(unsigned); // precxx20-note {{in instantiation of function template specialization 'Test1::h<unsigned int[10]>' requested here}}
 
 }
 
@@ -556,7 +556,7 @@ namespace P12023 {
 
   int main()
   {
-    CopyCounter* f = new CopyCounter[10](CopyCounter()); // expected-error {{cannot have initialization arguments}}
+    CopyCounter* f = new CopyCounter[10](CopyCounter()); // precxx20-error {{cannot have initialization arguments}}
       return 0;
   }
 }

>From ccdf700474cba4b77e2e0cf7029d673cca65840f Mon Sep 17 00:00:00 2001
From: Alan Zhao <ayzhao at google.com>
Date: Thu, 4 Jan 2024 09:06:43 -0800
Subject: [PATCH 2/4] fix accidentally deleted line in new-delete.cpp

---
 clang/test/SemaCXX/new-delete.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/clang/test/SemaCXX/new-delete.cpp b/clang/test/SemaCXX/new-delete.cpp
index ec406260b6a391..2db32938f957fb 100644
--- a/clang/test/SemaCXX/new-delete.cpp
+++ b/clang/test/SemaCXX/new-delete.cpp
@@ -1,3 +1,4 @@
+// RUN: %clang_cc1 -fsyntax-only -verify=expected,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++98
 // RUN: %clang_cc1 -fsyntax-only -verify=expected,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++11
 // RUN: %clang_cc1 -fsyntax-only -verify=expected,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++14
 // RUN: %clang_cc1 -fsyntax-only -verify=expected,cxx17,precxx20 %s -triple=i686-pc-linux-gnu -Wno-new-returns-null -std=c++17

>From 1877e21590aa5c16481b78f1a88ccede5dac4194 Mon Sep 17 00:00:00 2001
From: Alan Zhao <ayzhao at google.com>
Date: Thu, 4 Jan 2024 09:08:44 -0800
Subject: [PATCH 3/4] fix clang format issue

---
 clang/lib/Sema/SemaExprCXX.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 71e420648ce7af..90e0dcb35af277 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -2403,7 +2403,8 @@ ExprResult Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
   // Array 'new' can't have any initializers except empty parentheses.
   // Initializer lists are also allowed, in C++11. Rely on the parser for the
   // dialect distinction.
-  if (ArraySize && !isLegalArrayNewInitializer(InitStyle, Initializer, getLangOpts().CPlusPlus20)) {
+  if (ArraySize && !isLegalArrayNewInitializer(InitStyle, Initializer,
+                                               getLangOpts().CPlusPlus20)) {
     SourceRange InitRange(Exprs.front()->getBeginLoc(),
                           Exprs.back()->getEndLoc());
     Diag(StartLoc, diag::err_new_array_init_args) << InitRange;

>From 5610f3018abbf8076c80c8800b89f429cb8d8fb4 Mon Sep 17 00:00:00 2001
From: Alan Zhao <ayzhao at google.com>
Date: Thu, 4 Jan 2024 11:17:42 -0800
Subject: [PATCH 4/4] use range-based for loop

---
 clang/lib/CodeGen/CGExprCXX.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clang/lib/CodeGen/CGExprCXX.cpp b/clang/lib/CodeGen/CGExprCXX.cpp
index 72c61bfb5ec344..dd93bcfc57b93c 100644
--- a/clang/lib/CodeGen/CGExprCXX.cpp
+++ b/clang/lib/CodeGen/CGExprCXX.cpp
@@ -1104,7 +1104,8 @@ void CodeGenFunction::EmitNewArrayInitializer(
 
     CharUnits StartAlign = CurPtr.getAlignment();
     ArrayRef<Expr *> InitExprs = ILE ? ILE->inits() : CPLIE->getInitExprs();
-    for (unsigned i = 0; i < InitExprs.size(); ++i) {
+    unsigned i = 0;
+    for (Expr *IE : InitExprs) {
       // Tell the cleanup that it needs to destroy up to this
       // element.  TODO: some of these stores can be trivially
       // observed to be unnecessary.
@@ -1114,14 +1115,13 @@ void CodeGenFunction::EmitNewArrayInitializer(
       // FIXME: If the last initializer is an incomplete initializer list for
       // an array, and we have an array filler, we can fold together the two
       // initialization loops.
-      Expr *IE = InitExprs[i];
       StoreAnyExprIntoOneUnit(*this, IE, IE->getType(), CurPtr,
                               AggValueSlot::DoesNotOverlap);
       CurPtr = Address(Builder.CreateInBoundsGEP(
                            CurPtr.getElementType(), CurPtr.getPointer(),
                            Builder.getSize(1), "array.exp.next"),
                        CurPtr.getElementType(),
-                       StartAlign.alignmentAtOffset((i + 1) * ElementSize));
+                       StartAlign.alignmentAtOffset((++i) * ElementSize));
     }
 
     // The remaining elements are filled with the array filler expression.



More information about the cfe-commits mailing list