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

[Pranav Kant via cfe-commits]

pranavk wrote:

```
In file included from /usr/local/foo/home/prka/wip/unique/test.cpp:1:
In file included from 
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/memory:78:
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:1085:14:
 error: no matching conversion for functional-style cast from 'unsigned short 
*' to 'unique_ptr'
 1085 | { return unique_ptr<_Tp>(new remove_extent_t<_Tp>[__num]()); }
  |  ^~
/usr/local/foo/home/prka/wip/unique/test.cpp:10:47: note: in instantiation of 
function template specialization 'std::make_unique' 
requested here
   10 |   const std::unique_ptr nodes_ = 
std::make_unique(max_nodes_);
  |   ^
/usr/local/foo/home/prka/wip/unique/test.cpp:15:38: note: in instantiation of 
member function 'Base<256>::Base' requested here
   15 |   explicit Child(size_t max_bytes) : Base<256>(max_bytes) {
  |  ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:648:7:
 note: candidate constructor not viable: no known conversion from 'unsigned 
short *' to 'unique_ptr' for 1st argument
  648 |   unique_ptr(unique_ptr&&) = default;
  |   ^  
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:652:12:
 note: candidate constructor template not viable: no known conversion from 
'unsigned short *' to 'nullptr_t' (aka 'std::nullptr_t') for 1st argument
  652 | constexpr unique_ptr(nullptr_t) noexcept
  |   ^  ~
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:796:7:
 note: candidate constructor not viable: no known conversion from 'unsigned 
short *' to 'const unique_ptr' for 1st argument
  796 |   unique_ptr(const unique_ptr&) = delete;
  |   ^  ~
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:603:2:
 note: candidate template ignored: requirement 
'__and_, std::is_same>, 
std::__and_, std::is_same, std::is_convertible>>>::value' was not satisfied [with _Up = unsigned 
short *, _Vp = std::default_delete, $2 = 
_DeleterConstraint>]
  603 | unique_ptr(_Up __p) noexcept
  | ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:662:2:
 note: candidate template ignored: could not match 'unique_ptr<_Up, _Ep>' 
against 'unsigned short *'
  662 | unique_ptr(unique_ptr<_Up, _Ep>&& __u) noexcept
```

We get error messages like above with this commit. This compiles fine without 
this commit or with gcc.

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

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

[Pranav Kant via cfe-commits]

pranavk wrote:

This commit is still problematic. Minimal reproducer 
(https://godbolt.org/z/hE7M8EfT1).

We should get this reverted again.

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

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

[David Blaikie via cfe-commits]

dwblaikie wrote:

Simplified a bit more:
```
template 
void f1() {
  int (*x)[1] = new int[1][1]; // fails
}
template void f1();
void f2() {
  int (*x)[1] = new int[1][1]; // passes
}
```
https://godbolt.org/z/MhaYbvefG - yeah seems pretty clear this is a regression. 
Compiles with GCC, fails with clang, and the non-dependent context passes with 
either.

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

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

[Jorge Gorbe Moya via cfe-commits]

slackito wrote:

The following program fails to build after this change:
```
#include 
#include 

template
struct X {
  void f() {
values_.reset(new int64_t[123][65]);
  }

  std::unique_ptr values_;
};

int main() {
  X x;
  x.f();
}
```
you get similar errors with both libstdc++:
```
jgorbe@gorbe:~/llvm-build/bin$ ./clang++ -o /dev/null ~/repro.cc
/usr/local/google/home/jgorbe/repro.cc:7:13: error: no matching member function 
for call to 'reset'
7 | values_.reset(new int64_t[123][65]);
  | ^
/usr/local/google/home/jgorbe/repro.cc:15:5: note: in instantiation of member 
function 'X::f' requested here
   15 |   x.f();
  | ^
/usr/lib/gcc/x86_64-linux-gnu/13/../../../../include/c++/13/bits/unique_ptr.h:779:7:
 note: candidate template ignored: requirement 
'__and_, 
std::__and_, std::is_pointer, 
std::is_convertible>>>::value' was not satisfied 
[with _Up = int64_t *]
  779 |   reset(_Up __p) noexcept
  |   ^
1 error generated.
```
and libc++
```
jgorbe@gorbe:~/llvm-build/bin$ ./clang++ -o /dev/null ~/repro.cc -stdlib=libc++
/usr/local/google/home/jgorbe/repro.cc:7:13: error: no matching member function 
for call to 'reset'
7 | values_.reset(new int64_t[123][65]);
  | ^
/usr/local/google/home/jgorbe/repro.cc:15:5: note: in instantiation of member 
function 'X::f' requested here
   15 |   x.f();
  | ^
/usr/local/google/home/jgorbe/llvm-build/bin/../include/c++/v1/__memory/unique_ptr.h:457:60:
 note: candidate template ignored: requirement 
'_CheckArrayPointerConversion::value' was not satisfied [with _Pp = 
int64_t *]
  457 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void reset(_Pp 
__p) _NOEXCEPT {
  |^
1 error generated.
```

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

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

[via cfe-commits]

mahtohappy wrote:

@Sterling-Augustine please provide a reproducer script for your errors. 

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

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

[Erich Keane via cfe-commits]

erichkeane wrote:

> Unless the original test was subtly broken, the forward fix in 
> [0a789ea](https://github.com/llvm/llvm-project/commit/0a789ea8a829da345e46d8224d73b2ddaba6969f)
>  seems erroneous.
> 
> The forward fix changes the test to have a different declaration of `new`. 
> But I would not expect this original change to require source-code level 
> changes. Or was that intended?
> 
> We are experiencing errors that look like this:
> 
> ```
> [foo.cc:1045] error: no matching member function for call to 'reset'
>  1045 |   values_remaining_at_rank_of_width_.reset(new int64_t[ranks][65]);
>   |   ~~~^
> [foo.cc:1784] note: in instantiation of member function '(anonymous 
> namespace)::Writer::InitStats' requested here
>  1784 |   InitStats();
>   |   ^
> [foo.cc:2045] note: in instantiation of member function '(anonymous 
> namespace)::Writer::DoIt' requested here
>  2045 |   writer.DoIt();
>   |  ^
> [.../toolchain/bin/../include/c++/v1/__memory/unique_ptr.h:487] note: 
> candidate template ignored: requirement '_CheckArrayPointerConversion *>::value' was not satisfied [with _Pp = int64_t *]
>   487 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 void reset(_Pp 
> __p) _NOEXCEPT {
>   |^
> ```

That forward-fix was just a test change, doing a #include in our tests isn't 
something we can do, as it makes them fail with "file not found".  As far as 
your errors, it shouldn't be because of those, but could definitely be a 
problem wiht the originnal patch, hopefully @mahtohappy can get a chance to 
look at this.

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

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

[Mikhail Goncharov via cfe-commits]

metaflow wrote:

The problem is still present. I have reverted this chain of commits.

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

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

[via cfe-commits]

Sterling-Augustine wrote:

Unless the original test was subtly broken, the forward fix in 
0a789ea8a829da345e46d8224d73b2ddaba6969f seems erroneous.

The forward fix changes the test to have a different declaration of `new`. But 
I would not expect this original change to require source-code level changes. 
Or was that intended?

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

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

[antoine moynault via cfe-commits]

antmox wrote:

hello, instantiate-new-placement-size.cpp also fails here :
https://lab.llvm.org/buildbot/#/builders/188/builds/44501

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

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

[via cfe-commits]

dyung wrote:

@mahtohappy the test you added seems to be failing on quite a few buildbots 
including the documentation build and some x86 ones:
- https://github.com/llvm/llvm-project/actions/runs/8705457952/job/23875893823
- https://lab.llvm.org/buildbot/#/builders/121/builds/40812
- https://lab.llvm.org/buildbot/#/builders/230/builds/27331
- https://lab.llvm.org/buildbot/#/builders/94/builds/19405
- https://lab.llvm.org/buildbot/#/builders/98/builds/36470
- https://lab.llvm.org/buildbot/#/builders/280/builds/1958
- https://lab.llvm.org/buildbot/#/builders/139/builds/63510
- https://lab.llvm.org/buildbot/#/builders/216/builds/37557

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

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

[via cfe-commits]

mahtohappy wrote:

I don't observe the failures on x86 machine that I have. Pushed a version of 
test that was not breaking the build. Currently build is going on. I'll update 
once it's done.


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

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

[Nico Weber via cfe-commits]

nico wrote:

Looks like this breaks check-clang on mac: 
http://45.33.8.238/macm1/83529/step_6.txt

Please take a look and revert for now if it takes a while to fix.

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

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

[via cfe-commits]

github-actions[bot] wrote:



@mahtohappy Congratulations on having your first Pull Request (PR) merged into 
the LLVM Project!

Your changes will be combined with recent changes from other authors, then 
tested
by our [build bots](https://lab.llvm.org/buildbot/). If there is a problem with 
a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as
the builds can include changes from many authors. It is not uncommon for your
change to be included in a build that fails due to someone else's changes, or
infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail 
[here](https://llvm.org/docs/MyFirstTypoFix.html#myfirsttypofix-issues-after-landing-your-pr).

If your change does cause a problem, it may be reverted, or you can revert it 
yourself.
This is a normal part of [LLVM 
development](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy).
 You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are 
working as expected, well done!


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

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

[via cfe-commits]

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

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

[via cfe-commits]

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

>From a6fc7dbd5d71c1484d78e518648fb3f0f8e593d2 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 27 Feb 2024 03:13:51 -0800
Subject: [PATCH] [Clang][Sema] placement new initializes typedef array with
 correct size

---
 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 76701dc723b6c3..255d2cc0440438 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -539,6 +539,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 8c96134af7c8f0..9d15f3eacbb0f4 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12802,6 +12802,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;
@@ -12863,7 +12876,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();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

@@ -285,6 +285,8 @@ Bug Fixes to C++ Support
   templates when determining the primary template of an explicit 
specialization.
 - Fixed a crash in Microsoft compatibility mode where unqualified dependent 
base class
   lookup searches the bases of an incomplete class.
+- placement new initializes typedef array with correct size
+  (`#41441 `_)

cor3ntin wrote:

```suggestion
  (#GH41441)
```

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

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

[via cfe-commits]

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


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

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

[via cfe-commits]

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

>From 517134e20bafba3509531cdd884d617df4d8d86f Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 27 Feb 2024 03:13:51 -0800
Subject: [PATCH] [Clang][Sema] placement new initializes typedef array with
 correct size

---
 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 515dffa28df186..1e4a35ab432d1d 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -285,6 +285,8 @@ Bug Fixes to C++ Support
   templates when determining the primary template of an explicit 
specialization.
 - Fixed a crash in Microsoft compatibility mode where unqualified dependent 
base class
   lookup searches the bases of an incomplete class.
+- placement new initializes typedef array with correct size
+  (`#41441 `_)
 
 Bug Fixes to AST Handling
 ^
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..0cda78650fcc83 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12669,6 +12669,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;
@@ -12730,7 +12743,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();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

cor3ntin wrote:

Can you:
 - add a changelog entry referencing the issue
 -  rename the test file or reference the issue number in a comment in the test?

thanks!

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

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

[via cfe-commits]

mahtohappy wrote:

Hi Please Review this. @Endilll @cor3ntin @AaronBallman 

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

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

[via cfe-commits]

mahtohappy wrote:

Ping @AaronBallman @cor3ntin @Endilll 

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

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

[Erich Keane via cfe-commits]

@@ -12669,6 +12669,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.

erichkeane wrote:

This comment doesn't really match what is happening below?  the 'if' is still 
based on 'ArraySize' (presumably you mean this to be a proxy for 
`E->isArray`?).  But there is no standardeeze quoted here about why you'd pick 
up the size/alloc type from it like this, rather than in the `E->isArray` 
above.   

Also, does this also handle cases where this is going to still be dependent?  

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

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

[via cfe-commits]

mahtohappy wrote:

Hi @shafik @cor3ntin @AaronBallman @erichkeane @Endilll Please review this. 
Thank you. 

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

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

[via cfe-commits]

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

>From 991ca334f7efb4a7fc1e184ab6b4603db681b863 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 27 Feb 2024 03:13:51 -0800
Subject: [PATCH] [Clang][Sema] placement new initializes typedef array with
 correct size

---
 clang/lib/Sema/TreeTransform.h| 14 +-
 .../instantiate-new-placement-size.cpp| 19 +++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/instantiate-new-placement-size.cpp

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..0cda78650fcc83 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12669,6 +12669,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;
@@ -12730,7 +12743,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..bad162ffe7c42f
--- /dev/null
+++ b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -S -fno-discard-value-names -emit-llvm -o - %s | FileCheck %s
+#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();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

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

>From 7de2ca730de7a51f4bb3c6178914bf57a7efb321 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 27 Feb 2024 03:13:51 -0800
Subject: [PATCH] [Clang][Sema] placement new initializes typedef array with
 correct size

---
 clang/lib/Sema/TreeTransform.h| 14 +-
 .../instantiate-new-placement-size.cpp| 19 +++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/SemaCXX/instantiate-new-placement-size.cpp

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..0cda78650fcc83 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12669,6 +12669,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;
@@ -12730,7 +12743,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..ccb67f9fe822bc
--- /dev/null
+++ b/clang/test/SemaCXX/instantiate-new-placement-size.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -S -Xclang -triple=x86_64-pc-linux-gnu -fno-discard-value-names 
-emit-llvm -o - %s | FileCheck %s
+#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();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

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

>From 7f6bbd2a9e5ab162b0efb5c6dcca91e1d83a0bce Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 27 Feb 2024 03:13:51 -0800
Subject: [PATCH] [Clang][Sema] placement new initializes typedef array with
 correct size

---
 clang/lib/Sema/TreeTransform.h| 14 +-
 .../X86/instantiate-new-placement-size.cpp| 19 +++
 2 files changed, 32 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/X86/instantiate-new-placement-size.cpp

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..0cda78650fcc83 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12669,6 +12669,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;
@@ -12730,7 +12743,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/CodeGen/X86/instantiate-new-placement-size.cpp 
b/clang/test/CodeGen/X86/instantiate-new-placement-size.cpp
new file mode 100644
index 00..042554272a9bf6
--- /dev/null
+++ b/clang/test/CodeGen/X86/instantiate-new-placement-size.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -S -Xclang -triple=x86_64-unknown-linux-gnu 
-fno-discard-value-names -emit-llvm -o - %s | FileCheck %s
+#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();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

mahtohappy wrote:

@shafik I'm not aware of which cases it was catching before, I was hoping 
testing will reveal those testcases. I agree with your thinking that we might 
be breaking something and and don't have test coverage for it. I'll leave this 
condition as it is and add a separate check for this issue. It'll be more 
robust solution. 

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

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

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Can you explain which cases the `if (!ArraySize)` condition was catching before 
and why the change does not effect those cases? My fear is that we are breaking 
other cases but we don't have test coverage for those cases and we are missing 
those breaks.

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

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

[via cfe-commits]

mahtohappy wrote:

// If no array size was specified, but the new expression was
// instantiated with an array type (e.g., "new T" where T is
// instantiated with "int[4]"), extract the outer bound from the
// array type as our array size. We do this with constant and
// dependently-sized array types.
```
typedef TYPE TArray[8];
TArray x;
``` this is the same case. In `TreeTransform.h` we calculate the ArraySize 
earlier so that condition was failing. 

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

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

[via cfe-commits]

mahtohappy wrote:

> I definitely agree with both of Shafik's comments!
> 
> The fix itself concerns me, the logic in the block that is having its 
> condition inverted is specifically made for 'if no array size was specified' 
> (see comment), so that makes me think this is an incorrect patch. It doesn't 
> seem to follow the suggestion given in the original bug either.
I agree with that but this was simpler solution, and it didn't break any 
testcase so I pushed this solution. 
I had a other solution where the check for typedef was separate condition and 
that was working as well. But this looked neat to me and didn't break anything 
so I pushed this one.

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

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

[via cfe-commits]

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

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

[Erich Keane via cfe-commits]

erichkeane wrote:

I definitely agree with both of Shafik's comments!  

The fix itself concerns me, the logic in the block that is having its condition 
inverted is specifically made for 'if no array size was specified' (see 
comment), so that makes me think this is an incorrect patch.  It doesn't seem 
to follow the suggestion given in the original bug either.

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

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

[Shafik Yaghmour via cfe-commits]

shafik wrote:

Thank you for this fix.

Can you put more details in your summary. The approach I like to take is 1. 
what the problem is 2. what is the approach of the fix 3. any other important 
details.

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

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

[Shafik Yaghmour via cfe-commits]

@@ -0,0 +1,19 @@
+// RUN: %clang++ -S -emit-llvm -o - %s | FileCheck %s
+#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 

shafik wrote:

We need more test cases to make sure we are not breaking other cases. 
Non-dependent Vs dependent, constant Vs dynamically sized, VLA etc

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

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

[via cfe-commits]

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

>From 4661010da31d7fa83dd0e7282bcea8732b3598f0 Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 27 Feb 2024 03:13:51 -0800
Subject: [PATCH] [Clang][Sema] placement new initializes typedef array with
 correct size

---
 clang/lib/Sema/TreeTransform.h|  2 +-
 .../X86/instantiate-new-placement-size.cpp| 19 +++
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/X86/instantiate-new-placement-size.cpp

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..0a46deda194826 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12731,7 +12731,7 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
   }
 
   QualType AllocType = AllocTypeInfo->getType();
-  if (!ArraySize) {
+  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
 // instantiated with "int[4]"), extract the outer bound from the
diff --git a/clang/test/CodeGen/X86/instantiate-new-placement-size.cpp 
b/clang/test/CodeGen/X86/instantiate-new-placement-size.cpp
new file mode 100644
index 00..8916376a61fa21
--- /dev/null
+++ b/clang/test/CodeGen/X86/instantiate-new-placement-size.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -S -Xclang -triple=x86_64-unknown-unknown 
-fno-discard-value-names -emit-llvm -o - %s | FileCheck %s
+#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();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[via cfe-commits]

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

>From a4c9d01926176beddbd9b7e704ed23d8a3356c2b Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 27 Feb 2024 03:13:51 -0800
Subject: [PATCH] [Clang][Sema] placement new initializes typedef array with
 correct size

---
 clang/lib/Sema/TreeTransform.h|  2 +-
 .../instantiate-new-placement-size.cpp| 19 +++
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/instantiate-new-placement-size.cpp

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..0a46deda194826 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12731,7 +12731,7 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
   }
 
   QualType AllocType = AllocTypeInfo->getType();
-  if (!ArraySize) {
+  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
 // instantiated with "int[4]"), extract the outer bound from the
diff --git a/clang/test/CodeGen/instantiate-new-placement-size.cpp 
b/clang/test/CodeGen/instantiate-new-placement-size.cpp
new file mode 100644
index 00..62c4766eb4f997
--- /dev/null
+++ b/clang/test/CodeGen/instantiate-new-placement-size.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang++ -S -emit-llvm -o - %s | FileCheck %s
+#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();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Shivam Gupta via cfe-commits]

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

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

[via cfe-commits]

mahtohappy wrote:

It fixes this issue https://github.com/llvm/llvm-project/issues/41441
https://godbolt.org/z/8qoz3GM9h

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

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

[via cfe-commits]

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: None (mahtohappy)


Changes

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. 
#fixes 41441

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


2 Files Affected:

- (modified) clang/lib/Sema/TreeTransform.h (+1-1) 
- (added) clang/test/CodeGen/instantiate-new-placement-size.cpp (+19) 


``diff
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..0a46deda194826 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12731,7 +12731,7 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
   }
 
   QualType AllocType = AllocTypeInfo->getType();
-  if (!ArraySize) {
+  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
 // instantiated with "int[4]"), extract the outer bound from the
diff --git a/clang/test/CodeGen/instantiate-new-placement-size.cpp 
b/clang/test/CodeGen/instantiate-new-placement-size.cpp
new file mode 100644
index 00..eaf013a57a6176
--- /dev/null
+++ b/clang/test/CodeGen/instantiate-new-placement-size.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -S -emit-llvm -o - %s | FileCheck %s
+#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();
+}

``




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

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

[via cfe-commits]

github-actions[bot] wrote:



Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this 
page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using `@` followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from 
other developers.

If you have further questions, they may be answered by the [LLVM GitHub User 
Guide](https://llvm.org/docs/GitHub.html).

You can also ask questions in a comment on this PR, on the [LLVM 
Discord](https://discord.com/invite/xS7Z362) or on the 
[forums](https://discourse.llvm.org/).

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

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

[via cfe-commits]

https://github.com/mahtohappy created 
https://github.com/llvm/llvm-project/pull/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. 
#fixes 41441

>From 2a42e43d5816d13938efae03afed21db6be3539f Mon Sep 17 00:00:00 2001
From: mahtohappy 
Date: Tue, 27 Feb 2024 03:13:51 -0800
Subject: [PATCH] [Clang][Sema] placement new initializes typedef array with
 correct size

---
 clang/lib/Sema/TreeTransform.h|  2 +-
 .../instantiate-new-placement-size.cpp| 19 +++
 2 files changed, 20 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGen/instantiate-new-placement-size.cpp

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 7389a48fe56fcc..0a46deda194826 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12731,7 +12731,7 @@ TreeTransform::TransformCXXNewExpr(CXXNewExpr 
*E) {
   }
 
   QualType AllocType = AllocTypeInfo->getType();
-  if (!ArraySize) {
+  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
 // instantiated with "int[4]"), extract the outer bound from the
diff --git a/clang/test/CodeGen/instantiate-new-placement-size.cpp 
b/clang/test/CodeGen/instantiate-new-placement-size.cpp
new file mode 100644
index 00..eaf013a57a6176
--- /dev/null
+++ b/clang/test/CodeGen/instantiate-new-placement-size.cpp
@@ -0,0 +1,19 @@
+// RUN: %clang -S -emit-llvm -o - %s | FileCheck %s
+#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();
+}

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits