[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-23 Thread Erich Keane via cfe-commits 

erichkeane wrote:

> @mahtohappy can we please revert this in the meantime while you look for a 
> fix here. Feel free to land it again with the fix.

He's likely outside of work hours, but feel free to submit a revert.

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-23 Thread Pranav Kant via cfe-commits 

pranavk wrote:

@mahtohappy can we please revert this in the meantime while you look for a fix 
here. Feel free to land it again with the fix. 

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-23 Thread via cfe-commits 

mahtohappy wrote:

Hi @ilovepi Sure. Looking at this.

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-23 Thread Paul Kirth via cfe-commits 

ilovepi wrote:

I think we're seeing some build failures after this patch, and it isn't clear 
to me that this is a bug in the source, so I'd appreciate it if you could take 
a look.

```
FAILED: 
obj/src/media/audio/tools/signal_generator/signal_generator.signal_generator.cc.o
 
../../prebuilt/third_party/clang/custom/bin/clang++ -MD -MF 
obj/src/media/audio/tools/signal_generator/signal_generator.signal_generator.cc.o.d
 -D_LIBCPP_DISABLE_DEPRECATION_WARNINGS 
-D_LIBCPP_DISABLE_VISIBILITY_ANNOTATIONS -D_LIBCPP_REMOVE_TRANSITIVE_INCLUDES 
-D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS=1 -DZX_ASSERT_LEVEL=2 -I../.. -Igen 
-Ifidling/gen/sdk/fidl/fuchsia.media/fuchsia.media/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.hardware.audio/fuchsia.hardware.audio/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.hardware.audio.signalprocessing/fuchsia.hardware.audio.signalprocessing/hlcpp
 -I../../sdk -Igen/sdk -I../../sdk/lib/fidl_base/include 
-I../../src/zircon/lib/zircon/include 
-Ifidling/gen/zircon/vdso/zx/zither/legacy_syscall_cdecl 
-I../../sdk/lib/fit/include -I../../sdk/lib/stdcompat/include 
-I../../sdk/lib/fit-promise/include -I../../sdk/lib/fidl/include 
-I../../zircon/system/ulib/zx/include -I../../zircon/system/ulib/async/include 
-I../../zircon/system/ulib/async-default/include 
-Ifidling/gen/sdk/fidl/fuchsia.images/fuchsia.images/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.sysmem/fuchsia.sysmem/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.io/fuchsia.io/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.unknown/fuchsia.unknown/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.sysmem2/fuchsia.sysmem2/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.images2/fuchsia.images2/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.math/fuchsia.math/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.media.audio/fuchsia.media.audio/hlcpp 
-Ifidling/gen/sdk/fidl/fuchsia.ultrasound/fuchsia.ultrasound/hlcpp 
-I../../sdk/lib/fidl/cpp/wire/include -I../../zircon/system/ulib/sync/include 
-I../../sdk/lib/fdio/include -I../../zircon/system/public 
-I../../zircon/system/ulib/fbl/include 
-I../../third_party/googletest/src/googletest/include 
-I../../zircon/system/ulib/affine/include 
-I../../zircon/system/ulib/zircon-internal/include -I../../third_party/re2/src 
-I../../zircon/system/ulib/async-loop/include 
-I../../zircon/system/ulib/fzl/include -fcolor-diagnostics 
-fcrash-diagnostics-dir=clang-crashreports -fcrash-diagnostics=all 
-gen-reproducer=error -ffp-contract=off --sysroot=gen/zircon/public/sysroot/cpp 
--target=x86_64-unknown-fuchsia -ffuchsia-api-level=4294967295 -march=x86-64-v2 
-mtune=generic -mbranches-within-32B-boundaries -ffile-compilation-dir=. 
-no-canonical-prefixes -fno-omit-frame-pointer -momit-leaf-frame-pointer 
-fdata-sections -ffunction-sections -O0 -Xclang -debug-info-kind=constructor 
-g3 -grecord-gcc-switches -gdwarf-5 -gz=zstd -Wall -Wextra -Wconversion 
-Wextra-semi -Wimplicit-fallthrough -Wnewline-eof -Wstrict-prototypes 
-Wwrite-strings -Wno-sign-conversion -Wno-unused-parameter 
-Wnonportable-system-include-path -Wno-missing-field-initializers 
-Wno-extra-qualification -Wno-cast-function-type-strict 
-Wno-cast-function-type-mismatch -Wno-unknown-warning-option 
-Wno-deprecated-pragma -fvisibility=hidden -Werror -Wa,--fatal-warnings 
-ftrivial-auto-var-init=pattern -Wthread-safety -Wno-unknown-warning-option 
-Wno-thread-safety-reference-return -Wno-undef -fvisibility-inlines-hidden 
-std=c++17 -fno-exceptions -fno-rtti -ftemplate-backtrace-limit=0 -c 
../../src/media/audio/tools/signal_generator/signal_generator.cc -o 
obj/src/media/audio/tools/signal_generator/signal_generator.signal_generator.cc.o
In file included from 
../../src/media/audio/tools/signal_generator/signal_generator.cc:4:
In file included from 
../../src/media/audio/tools/signal_generator/signal_generator.h:7:
In file included from 
fidling/gen/sdk/fidl/fuchsia.media/fuchsia.media/hlcpp/fuchsia/media/cpp/fidl.h:6:
In file included from ../../sdk/lib/fidl/cpp/internal/header.h:11:
In file included from ../../sdk/lib/fit/include/lib/fit/function.h:9:
In file included from 
../../prebuilt/third_party/clang/custom/bin/../include/c++/v1/memory:937:
In file included from 
../../prebuilt/third_party/clang/custom/bin/../include/c++/v1/__memory/shared_ptr.h:32:
../../prebuilt/third_party/clang/custom/bin/../include/c++/v1/__memory/unique_ptr.h:627:10:
 error: no matching conversion for functional-style cast from 'double *' to 
'unique_ptr'
  627 |   return unique_ptr<_Tp>(new _Up[__n]());
  |  ^~~
../../src/media/audio/tools/signal_generator/signal_generator.cc:803:25: note: 
in instantiation of function template specialization 
'std::make_unique' requested here
  803 |   input_history_ = std::make_unique(num_channels_);
  | ^
../../prebuilt/third_party/clang/custom/bin/../include/c++/v1/__memory/unique_ptr.h:125:59:
 note: candidate constructor (the implicit copy constructor) not viable: no 
known conversion from 'double *' to 'const unique_ptr' for 1st

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-23 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane closed 
https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-23 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane approved this pull request.


https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-23 Thread via cfe-commits 

mahtohappy wrote:

Hi @erichkeane Please merge this change. 

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-21 Thread via cfe-commits 

mahtohappy wrote:

Hi @cor3ntin Please merge this. 

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-21 Thread via cfe-commits 

https://github.com/mahtohappy updated 
https://github.com/llvm/llvm-project/pull/89036

>From 56c2b4f70735a6ef3b80cb8461a88ea51f2d02d7 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 16 Apr 2024 17:48:45 +0530
Subject: [PATCH 1/6] [Clang][Sema] placement new initializes typedef array
 with correct size (#83124)

When in-place new-ing a local variable of an array of trivial type, the
generated code calls 'memset' with the correct size of the array,
earlier it was generating size (squared of the typedef array + size).

The cause: `typedef TYPE TArray[8]; TArray x;` The type of declarator is
Tarray[8] and in `SemaExprCXX.cpp::BuildCXXNew` we check if it's of
typedef and of constant size then we get the original type and it works
fine for non-dependent cases.
But in case of template we do `TreeTransform.h:TransformCXXNEWExpr` and
there we again check the allocated type which is TArray[8] and it stays
that way, so ArraySize=(Tarray[8] type, alloc Tarray[8*type]) so the
squared size allocation.

ArraySize gets calculated earlier in `TreeTransform.h` so that
`if(!ArraySize)` condition was failing.
fix: I changed that condition to `if(ArraySize)`.


Fixes #41441
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 clang/lib/Sema/TreeTransform.h| 14 -
 .../instantiate-new-placement-size.cpp| 20 +++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/instantiate-new-placement-size.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 96ad92b540b47f..636d76f1836759 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -534,6 +534,8 @@ Bug Fixes to C++ Support
   Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), 
(#GH86398), and (#GH86399).
 - Fix a crash when deducing ``auto`` from an invalid dereference (#GH88329).
 - Fix a crash in requires expression with templated base class member 
function. Fixes (#GH84020).
+- placement new initializes typedef array with correct size
+  (`#GH41441 `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index eb05783a6219dc..0c7fdb357235e1 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize) {
+if (const ConstantArrayType *Array =
+SemaRef.Context.getAsConstantArrayType(AllocType)) {
+  ArraySize = IntegerLiteral::Create(SemaRef.Context, Array->getSize(),
+ SemaRef.Context.getSizeType(),
+ E->getBeginLoc());
+  AllocType = Array->getElementType();
+}
+  }
+
   // Transform the placement arguments (if any).
   bool ArgumentChanged = false;
   SmallVector PlacementArgs;
@@ -12925,7 +12938,6 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 return E;
   }
 
-  QualType AllocType = AllocTypeInfo->getType();
   if (!ArraySize) {
 // If no array size was specified, but the new expression was
 // instantiated with an array type (e.g., "new T" where T is
diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
new file mode 100644
index 00..7a29d3dee8491e
--- /dev/null
+++ b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fno-discard-value-names -emit-llvm -o - %s | FileCheck %s
+// Issue no: 41441
+#include 
+
+// CHECK: call void @llvm.memset.p0.i64(ptr align 1 %x, i8 0, i64 8, i1 false)
+// CHECK: call void @llvm.memset.p0.i64(ptr align 16 %x, i8 0, i64 32, i1 
false)
+template 
+void f()
+{
+typedef TYPE TArray[8];
+
+TArray x;
+new() TArray();
+}
+
+int main()
+{
+f();
+f();
+}

>From bdd7f66e560f60d1f0027b04ad12989b2f786df3 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Wed, 17 Apr 2024 00:12:14 +0530
Subject: [PATCH 2/6] [Clang][Sema] placement new initializes typedef array
 with correct size (#88902)

Build Failure Fix
Fixes build failures due to #83124
---
 .../{instantiate-new-placement-size.cpp => PR41441.cpp}   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
 rename clang/test/SemaCXX/{instantiate-new-placement-size.cpp => PR41441.cpp} 
(75%)

diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/PR41441.cpp
similarity index 75%
rename from clang/test/SemaCXX/instantiate-new-placement-size.cpp
rename to clang/test/SemaCXX/PR41441.cpp
index 7a29d3dee8491e..0b012b33fce343 100644
--- a/clang/test/SemaCXX/instantiate-new-placement-size.cpp
+++

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-20 Thread via cfe-commits 

mahtohappy wrote:

Hi, the build is failing for windows but there's not test failures and no 
errors in the log. What should I do from here?

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread via cfe-commits 

https://github.com/mahtohappy updated 
https://github.com/llvm/llvm-project/pull/89036

>From 56c2b4f70735a6ef3b80cb8461a88ea51f2d02d7 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 16 Apr 2024 17:48:45 +0530
Subject: [PATCH 1/5] [Clang][Sema] placement new initializes typedef array
 with correct size (#83124)

When in-place new-ing a local variable of an array of trivial type, the
generated code calls 'memset' with the correct size of the array,
earlier it was generating size (squared of the typedef array + size).

The cause: `typedef TYPE TArray[8]; TArray x;` The type of declarator is
Tarray[8] and in `SemaExprCXX.cpp::BuildCXXNew` we check if it's of
typedef and of constant size then we get the original type and it works
fine for non-dependent cases.
But in case of template we do `TreeTransform.h:TransformCXXNEWExpr` and
there we again check the allocated type which is TArray[8] and it stays
that way, so ArraySize=(Tarray[8] type, alloc Tarray[8*type]) so the
squared size allocation.

ArraySize gets calculated earlier in `TreeTransform.h` so that
`if(!ArraySize)` condition was failing.
fix: I changed that condition to `if(ArraySize)`.


Fixes #41441
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 clang/lib/Sema/TreeTransform.h| 14 -
 .../instantiate-new-placement-size.cpp| 20 +++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/instantiate-new-placement-size.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 96ad92b540b47f..636d76f1836759 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -534,6 +534,8 @@ Bug Fixes to C++ Support
   Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), 
(#GH86398), and (#GH86399).
 - Fix a crash when deducing ``auto`` from an invalid dereference (#GH88329).
 - Fix a crash in requires expression with templated base class member 
function. Fixes (#GH84020).
+- placement new initializes typedef array with correct size
+  (`#GH41441 `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index eb05783a6219dc..0c7fdb357235e1 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize) {
+if (const ConstantArrayType *Array =
+SemaRef.Context.getAsConstantArrayType(AllocType)) {
+  ArraySize = IntegerLiteral::Create(SemaRef.Context, Array->getSize(),
+ SemaRef.Context.getSizeType(),
+ E->getBeginLoc());
+  AllocType = Array->getElementType();
+}
+  }
+
   // Transform the placement arguments (if any).
   bool ArgumentChanged = false;
   SmallVector PlacementArgs;
@@ -12925,7 +12938,6 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 return E;
   }
 
-  QualType AllocType = AllocTypeInfo->getType();
   if (!ArraySize) {
 // If no array size was specified, but the new expression was
 // instantiated with an array type (e.g., "new T" where T is
diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
new file mode 100644
index 00..7a29d3dee8491e
--- /dev/null
+++ b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fno-discard-value-names -emit-llvm -o - %s | FileCheck %s
+// Issue no: 41441
+#include 
+
+// CHECK: call void @llvm.memset.p0.i64(ptr align 1 %x, i8 0, i64 8, i1 false)
+// CHECK: call void @llvm.memset.p0.i64(ptr align 16 %x, i8 0, i64 32, i1 
false)
+template 
+void f()
+{
+typedef TYPE TArray[8];
+
+TArray x;
+new() TArray();
+}
+
+int main()
+{
+f();
+f();
+}

>From bdd7f66e560f60d1f0027b04ad12989b2f786df3 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Wed, 17 Apr 2024 00:12:14 +0530
Subject: [PATCH 2/5] [Clang][Sema] placement new initializes typedef array
 with correct size (#88902)

Build Failure Fix
Fixes build failures due to #83124
---
 .../{instantiate-new-placement-size.cpp => PR41441.cpp}   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
 rename clang/test/SemaCXX/{instantiate-new-placement-size.cpp => PR41441.cpp} 
(75%)

diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/PR41441.cpp
similarity index 75%
rename from clang/test/SemaCXX/instantiate-new-placement-size.cpp
rename to clang/test/SemaCXX/PR41441.cpp
index 7a29d3dee8491e..0b012b33fce343 100644
--- a/clang/test/SemaCXX/instantiate-new-placement-size.cpp
+++

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread Erich Keane via cfe-commits 


@@ -0,0 +1,23 @@
+// RUN: %clang --target=x86_64-pc-linux -S -fno-discard-value-names -emit-llvm 
-o - %s | FileCheck %s
+
+namespace std {

erichkeane wrote:

Tests STILL haven't changed from the original patch.  Please add the regression 
to this test as well.

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread via cfe-commits 


@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize && E->isTypeDependent()) {

mahtohappy wrote:

No, it doesn't have value dependence. 

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread Erich Keane via cfe-commits 


@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize && E->isTypeDependent()) {

erichkeane wrote:

Note I asked about VALUE dependence, not TYPE dependence.  I see why type 
dependence would matter, but expressions can be both type and value dependent.

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread via cfe-commits 


@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize && E->isTypeDependent()) {

mahtohappy wrote:

Yes, it does. Both the original testcase from @slackito  and simpler version of 
@dwblaikie are working now. 
normal arrays with new initializer also were passing the check and their type 
was correct earlier so need for modification for them. ```typedef TYPE 
TArray[8]; TArray x;``` here type of array was Tarray which should be Type* so 
the check only for type dependent case. 

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread Erich Keane via cfe-commits 


@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize && E->isTypeDependent()) {

erichkeane wrote:

does Value dependence matter?  

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane edited 
https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane commented:

Where is the repro from the original author?  What did they share, and what 
ended up being the solution/test here for it?

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread via cfe-commits 

mahtohappy wrote:

Check is for dependent types and earlier I was not checking that with the 
condition being only
```if (ArraySize)``` so normal arrays with new initializer also were passing 
the check.
Now added the dependent type check as well
```if (ArraySize && E->isTypeDependent())```

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread David Blaikie via cfe-commits 

dwblaikie wrote:

> Yes it's fixed now.

Please include details of the fix I the patch description

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-19 Thread via cfe-commits 

mahtohappy wrote:

Yes it's fixed now.

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-18 Thread David Blaikie via cfe-commits 

dwblaikie wrote:

> The original patch pointed out a regression:
> 
> https://github.com/llvm/llvm-project/pull/83124#issuecomment-2060090590
> 
> Please contact that person and get a reproducer and make sure you aren't 
> breaking them.

Repro was provided further down: 
https://github.com/llvm/llvm-project/pull/83124#issuecomment-2062540922

Please include test coverage and a fix before approval/recommit.

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-18 Thread via cfe-commits 

https://github.com/mahtohappy updated 
https://github.com/llvm/llvm-project/pull/89036

>From 56c2b4f70735a6ef3b80cb8461a88ea51f2d02d7 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 16 Apr 2024 17:48:45 +0530
Subject: [PATCH 1/5] [Clang][Sema] placement new initializes typedef array
 with correct size (#83124)

When in-place new-ing a local variable of an array of trivial type, the
generated code calls 'memset' with the correct size of the array,
earlier it was generating size (squared of the typedef array + size).

The cause: `typedef TYPE TArray[8]; TArray x;` The type of declarator is
Tarray[8] and in `SemaExprCXX.cpp::BuildCXXNew` we check if it's of
typedef and of constant size then we get the original type and it works
fine for non-dependent cases.
But in case of template we do `TreeTransform.h:TransformCXXNEWExpr` and
there we again check the allocated type which is TArray[8] and it stays
that way, so ArraySize=(Tarray[8] type, alloc Tarray[8*type]) so the
squared size allocation.

ArraySize gets calculated earlier in `TreeTransform.h` so that
`if(!ArraySize)` condition was failing.
fix: I changed that condition to `if(ArraySize)`.


Fixes #41441
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 clang/lib/Sema/TreeTransform.h| 14 -
 .../instantiate-new-placement-size.cpp| 20 +++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/instantiate-new-placement-size.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 96ad92b540b47f..636d76f1836759 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -534,6 +534,8 @@ Bug Fixes to C++ Support
   Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), 
(#GH86398), and (#GH86399).
 - Fix a crash when deducing ``auto`` from an invalid dereference (#GH88329).
 - Fix a crash in requires expression with templated base class member 
function. Fixes (#GH84020).
+- placement new initializes typedef array with correct size
+  (`#GH41441 `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index eb05783a6219dc..0c7fdb357235e1 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize) {
+if (const ConstantArrayType *Array =
+SemaRef.Context.getAsConstantArrayType(AllocType)) {
+  ArraySize = IntegerLiteral::Create(SemaRef.Context, Array->getSize(),
+ SemaRef.Context.getSizeType(),
+ E->getBeginLoc());
+  AllocType = Array->getElementType();
+}
+  }
+
   // Transform the placement arguments (if any).
   bool ArgumentChanged = false;
   SmallVector PlacementArgs;
@@ -12925,7 +12938,6 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 return E;
   }
 
-  QualType AllocType = AllocTypeInfo->getType();
   if (!ArraySize) {
 // If no array size was specified, but the new expression was
 // instantiated with an array type (e.g., "new T" where T is
diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
new file mode 100644
index 00..7a29d3dee8491e
--- /dev/null
+++ b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fno-discard-value-names -emit-llvm -o - %s | FileCheck %s
+// Issue no: 41441
+#include 
+
+// CHECK: call void @llvm.memset.p0.i64(ptr align 1 %x, i8 0, i64 8, i1 false)
+// CHECK: call void @llvm.memset.p0.i64(ptr align 16 %x, i8 0, i64 32, i1 
false)
+template 
+void f()
+{
+typedef TYPE TArray[8];
+
+TArray x;
+new() TArray();
+}
+
+int main()
+{
+f();
+f();
+}

>From bdd7f66e560f60d1f0027b04ad12989b2f786df3 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Wed, 17 Apr 2024 00:12:14 +0530
Subject: [PATCH 2/5] [Clang][Sema] placement new initializes typedef array
 with correct size (#88902)

Build Failure Fix
Fixes build failures due to #83124
---
 .../{instantiate-new-placement-size.cpp => PR41441.cpp}   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
 rename clang/test/SemaCXX/{instantiate-new-placement-size.cpp => PR41441.cpp} 
(75%)

diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/PR41441.cpp
similarity index 75%
rename from clang/test/SemaCXX/instantiate-new-placement-size.cpp
rename to clang/test/SemaCXX/PR41441.cpp
index 7a29d3dee8491e..0b012b33fce343 100644
--- a/clang/test/SemaCXX/instantiate-new-placement-size.cpp
+++

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-18 Thread via cfe-commits 

https://github.com/mahtohappy updated 
https://github.com/llvm/llvm-project/pull/89036

>From 56c2b4f70735a6ef3b80cb8461a88ea51f2d02d7 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 16 Apr 2024 17:48:45 +0530
Subject: [PATCH 1/5] [Clang][Sema] placement new initializes typedef array
 with correct size (#83124)

When in-place new-ing a local variable of an array of trivial type, the
generated code calls 'memset' with the correct size of the array,
earlier it was generating size (squared of the typedef array + size).

The cause: `typedef TYPE TArray[8]; TArray x;` The type of declarator is
Tarray[8] and in `SemaExprCXX.cpp::BuildCXXNew` we check if it's of
typedef and of constant size then we get the original type and it works
fine for non-dependent cases.
But in case of template we do `TreeTransform.h:TransformCXXNEWExpr` and
there we again check the allocated type which is TArray[8] and it stays
that way, so ArraySize=(Tarray[8] type, alloc Tarray[8*type]) so the
squared size allocation.

ArraySize gets calculated earlier in `TreeTransform.h` so that
`if(!ArraySize)` condition was failing.
fix: I changed that condition to `if(ArraySize)`.


Fixes #41441
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 clang/lib/Sema/TreeTransform.h| 14 -
 .../instantiate-new-placement-size.cpp| 20 +++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/instantiate-new-placement-size.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 96ad92b540b47f..636d76f1836759 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -534,6 +534,8 @@ Bug Fixes to C++ Support
   Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), 
(#GH86398), and (#GH86399).
 - Fix a crash when deducing ``auto`` from an invalid dereference (#GH88329).
 - Fix a crash in requires expression with templated base class member 
function. Fixes (#GH84020).
+- placement new initializes typedef array with correct size
+  (`#GH41441 `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index eb05783a6219dc..0c7fdb357235e1 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize) {
+if (const ConstantArrayType *Array =
+SemaRef.Context.getAsConstantArrayType(AllocType)) {
+  ArraySize = IntegerLiteral::Create(SemaRef.Context, Array->getSize(),
+ SemaRef.Context.getSizeType(),
+ E->getBeginLoc());
+  AllocType = Array->getElementType();
+}
+  }
+
   // Transform the placement arguments (if any).
   bool ArgumentChanged = false;
   SmallVector PlacementArgs;
@@ -12925,7 +12938,6 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 return E;
   }
 
-  QualType AllocType = AllocTypeInfo->getType();
   if (!ArraySize) {
 // If no array size was specified, but the new expression was
 // instantiated with an array type (e.g., "new T" where T is
diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
new file mode 100644
index 00..7a29d3dee8491e
--- /dev/null
+++ b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fno-discard-value-names -emit-llvm -o - %s | FileCheck %s
+// Issue no: 41441
+#include 
+
+// CHECK: call void @llvm.memset.p0.i64(ptr align 1 %x, i8 0, i64 8, i1 false)
+// CHECK: call void @llvm.memset.p0.i64(ptr align 16 %x, i8 0, i64 32, i1 
false)
+template 
+void f()
+{
+typedef TYPE TArray[8];
+
+TArray x;
+new() TArray();
+}
+
+int main()
+{
+f();
+f();
+}

>From bdd7f66e560f60d1f0027b04ad12989b2f786df3 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Wed, 17 Apr 2024 00:12:14 +0530
Subject: [PATCH 2/5] [Clang][Sema] placement new initializes typedef array
 with correct size (#88902)

Build Failure Fix
Fixes build failures due to #83124
---
 .../{instantiate-new-placement-size.cpp => PR41441.cpp}   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
 rename clang/test/SemaCXX/{instantiate-new-placement-size.cpp => PR41441.cpp} 
(75%)

diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/PR41441.cpp
similarity index 75%
rename from clang/test/SemaCXX/instantiate-new-placement-size.cpp
rename to clang/test/SemaCXX/PR41441.cpp
index 7a29d3dee8491e..0b012b33fce343 100644
--- a/clang/test/SemaCXX/instantiate-new-placement-size.cpp
+++

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-18 Thread via cfe-commits 

https://github.com/mahtohappy updated 
https://github.com/llvm/llvm-project/pull/89036

>From 56c2b4f70735a6ef3b80cb8461a88ea51f2d02d7 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 16 Apr 2024 17:48:45 +0530
Subject: [PATCH 1/5] [Clang][Sema] placement new initializes typedef array
 with correct size (#83124)

When in-place new-ing a local variable of an array of trivial type, the
generated code calls 'memset' with the correct size of the array,
earlier it was generating size (squared of the typedef array + size).

The cause: `typedef TYPE TArray[8]; TArray x;` The type of declarator is
Tarray[8] and in `SemaExprCXX.cpp::BuildCXXNew` we check if it's of
typedef and of constant size then we get the original type and it works
fine for non-dependent cases.
But in case of template we do `TreeTransform.h:TransformCXXNEWExpr` and
there we again check the allocated type which is TArray[8] and it stays
that way, so ArraySize=(Tarray[8] type, alloc Tarray[8*type]) so the
squared size allocation.

ArraySize gets calculated earlier in `TreeTransform.h` so that
`if(!ArraySize)` condition was failing.
fix: I changed that condition to `if(ArraySize)`.


Fixes #41441
---
 clang/docs/ReleaseNotes.rst   |  2 ++
 clang/lib/Sema/TreeTransform.h| 14 -
 .../instantiate-new-placement-size.cpp| 20 +++
 3 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/instantiate-new-placement-size.cpp

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 96ad92b540b47f..636d76f1836759 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -534,6 +534,8 @@ Bug Fixes to C++ Support
   Fixes (#GH70604), (#GH79754), (#GH84163), (#GH84425), (#GH86054), 
(#GH86398), and (#GH86399).
 - Fix a crash when deducing ``auto`` from an invalid dereference (#GH88329).
 - Fix a crash in requires expression with templated base class member 
function. Fixes (#GH84020).
+- placement new initializes typedef array with correct size
+  (`#GH41441 `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index eb05783a6219dc..0c7fdb357235e1 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12864,6 +12864,19 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 ArraySize = NewArraySize.get();
   }
 
+  // Per C++0x [expr.new]p5, the type being constructed may be a
+  // typedef of an array type.
+  QualType AllocType = AllocTypeInfo->getType();
+  if (ArraySize) {
+if (const ConstantArrayType *Array =
+SemaRef.Context.getAsConstantArrayType(AllocType)) {
+  ArraySize = IntegerLiteral::Create(SemaRef.Context, Array->getSize(),
+ SemaRef.Context.getSizeType(),
+ E->getBeginLoc());
+  AllocType = Array->getElementType();
+}
+  }
+
   // Transform the placement arguments (if any).
   bool ArgumentChanged = false;
   SmallVector PlacementArgs;
@@ -12925,7 +12938,6 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
 return E;
   }
 
-  QualType AllocType = AllocTypeInfo->getType();
   if (!ArraySize) {
 // If no array size was specified, but the new expression was
 // instantiated with an array type (e.g., "new T" where T is
diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
new file mode 100644
index 00..7a29d3dee8491e
--- /dev/null
+++ b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
@@ -0,0 +1,20 @@
+// RUN: %clang -S -fno-discard-value-names -emit-llvm -o - %s | FileCheck %s
+// Issue no: 41441
+#include 
+
+// CHECK: call void @llvm.memset.p0.i64(ptr align 1 %x, i8 0, i64 8, i1 false)
+// CHECK: call void @llvm.memset.p0.i64(ptr align 16 %x, i8 0, i64 32, i1 
false)
+template 
+void f()
+{
+typedef TYPE TArray[8];
+
+TArray x;
+new() TArray();
+}
+
+int main()
+{
+f();
+f();
+}

>From bdd7f66e560f60d1f0027b04ad12989b2f786df3 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Wed, 17 Apr 2024 00:12:14 +0530
Subject: [PATCH 2/5] [Clang][Sema] placement new initializes typedef array
 with correct size (#88902)

Build Failure Fix
Fixes build failures due to #83124
---
 .../{instantiate-new-placement-size.cpp => PR41441.cpp}   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
 rename clang/test/SemaCXX/{instantiate-new-placement-size.cpp => PR41441.cpp} 
(75%)

diff --git a/clang/test/SemaCXX/instantiate-new-placement-size.cpp 
b/clang/test/SemaCXX/PR41441.cpp
similarity index 75%
rename from clang/test/SemaCXX/instantiate-new-placement-size.cpp
rename to clang/test/SemaCXX/PR41441.cpp
index 7a29d3dee8491e..0b012b33fce343 100644
--- a/clang/test/SemaCXX/instantiate-new-placement-size.cpp
+++

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-17 Thread Erich Keane via cfe-commits 

https://github.com/erichkeane commented:

The original patch pointed out a regression:

https://github.com/llvm/llvm-project/pull/83124#issuecomment-2060090590

Please contact that person and get a reproducer and make sure you aren't 
breaking them.

https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Reapply "[Clang][Sema] placement new initializes typedef array with correct size (#83124)" (PR #89036)

2024-04-17 Thread via cfe-commits 

https://github.com/Sirraide edited 
https://github.com/llvm/llvm-project/pull/89036
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits