[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-05-30 Thread Yaxun Liu via cfe-commits

yxsamliu wrote:

> > This is mostly just a call to coordinate a bit better. I would suggest that 
> > if the patch author is engaged on the thread, it would make sense to tell 
> > them the schedule on which you plan to revert so they have an opportunity 
> > to tell you if that's going to be disruptive.
> 
> This part captures one particular issue with my way of action I happened to 
> miss. Thanks for putting this clearly @AaronBallman! Coordination was indeed 
> far from perfect here. And your suggestion totally makes sense.
> 
> > While I realize this is the 'policy', I'm asking for compassion, 
> > particularly with new developers who are engaging.
> 
> This is also a good point @erichkeane. I'll try to consider this as well.

Any plan to reland this PR? cuDF/hipDF depends on it. Also the subtle 
difference of behavior about list initialization between clang/gcc causes 
issues that are difficult to debug. Thanks.

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-02-21 Thread Yaxun Liu via cfe-commits

yxsamliu wrote:

The issue above seems to be a clang bug to me according to 
https://cplusplus.github.io/CWG/issues/1467.html

List-initialization of an object or reference of type T is defined as follows:

If T is a class type and the initializer list has a single element of type cv T 
or a class type derived from T, the object is initialized from that element.

Therefore `b` should be initialized from `a` instead of `A(a)`.

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-02-21 Thread Yaxun Liu via cfe-commits

yxsamliu wrote:

This PR caused a regression for AMDMIGraphX 
https://github.com/ROCm/AMDMIGraphX/blob/develop/src/targets/gpu/include/migraphx/gpu/prefix_scan_sum.hpp#L57

A reduced test case is:

#include 
#include 
using namespace std;
```
struct A {
int x;
A(int x_) : x(x_) {}
A(const vector& a) {}
};

int main() {
vector a{1,2};
vector b{a};

printf("%ld\n", b.size());
}
```
For variable b, it is expected to be constructed by copy ctor of vector, and 
have 2 elements.

With this patch, a is first converted to A(a), then b is constructed with one 
element A(a).

Is this expected or a bug? Thanks.

gcc has the same behavior https://godbolt.org/z/hTb795xWP

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-29 Thread Erich Keane via cfe-commits

erichkeane wrote:

> However, a revert shouldn't be perceived as an impolite or unfriendly action. 
> All contributions are welcome and appreciated. Reverting a commit is merely a 
> way to deal with problems one at a time without blocking others or racing 
> against the clock.

Unfortunately this isn't the way it is perceived.  No matter how much we say 
that, it causes folks to get discouraged/scares them away.  I've witnessed a 
significant number of 'new' contributors do great for 3-4 patches, then have 
their 'last' patch get reverted, and that person disappears from the project.  
I know when _I_ first got reverted (a long time ago now!) I nearly quit the 
project entirely, and I had many coworkers tell me they no longer wanted to 
contribute to clang after getting reverted.

While I realize this is the 'policy', I'm asking for compassion, particularly 
with new developers who are engaging.

>For example, I got a quick response here, but now, two days later the fix 
>hasn't been even sent for review (and this is totally fine, since the 
>problematic commit is reverted, and the author can work in their pace and with 
>their own priorities).

I find myself wondering if/afraid that the 'revert' is why we haven't seen them 
come back, and not because they now have 'more time'.

While I am sympathetic to the goals of keeping ToT green, I just asked that you 
be equally sympathetic to the person you're reverting.


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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-27 Thread Aaron Ballman via cfe-commits

AaronBallman wrote:

> > > As I understood the author, more time was needed to prepare the tests and 
> > > the fix. I just tried to bring the tip of the tree to a good state soon 
> > > (our internal release process was blocked on this) without putting any 
> > > pressure on the author.
> > 
> > 
> > Typically when the author is engaging fairly quickly and only needs a short 
> > time (he had already identified the problem and only had process-related 
> > concerns), it is preferred to be understanding/polite and give them time to 
> > work on it.
> 
> First of all, let me quote the developer policy: ["Remember, it is normal and 
> healthy to have patches reverted. Having a patch reverted does not 
> necessarily mean you did anything 
> wrong."](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). 
> Now, I totally understand the desire to "capture the progress" as well as 
> frustration one might have seeing their work reverted. However, a revert 
> shouldn't be perceived as an impolite or unfriendly action. All contributions 
> are welcome and appreciated. Reverting a commit is merely a way to deal with 
> problems one at a time without blocking others or racing against the clock.

The developer policy also says: "Reverts should be reasonably timely. A change 
submitted two hours ago can be reverted without prior discussion. A change 
submitted two years ago should not be. Where exactly the transition point is is 
hard to say, but it’s probably in the handful of days in tree territory. If you 
are unsure, we encourage you to reply to the commit thread, give the author a 
bit to respond, and then proceed with the revert if the author doesn’t seem to 
be actively responding."

"Reasonably timely" also means that authors who are engaged on a fix are 
expected to have the opportunity to land that fix. Reverts can be disruptive 
and they can clutter the revision history, so *if* we can avoid them, it's 
better to avoid them. However, we don't want to avoid reverts when they help 
keep the tree stable; not reverting can be plenty disruptive, too! There's a 
happy medium somewhere between "revert as quickly as possible at the first sign 
of trouble" and "let authors have infinite time to fix issues".

I think it's better that we err on the side of reverting quickly, and I don't 
think anyone has done anything wrong here. But I think we have some downstream 
projects which are quicker to revert than is comfortable and so some code 
owners are pushing back (gently) on reverts that we think may have jumped the 
gun a bit. For example, I know I've personally had several instances where I've 
had things reverted out from under me as I'm trying to land the fix and that's 
caused significantly more work for me. This review seems to have had a similar 
outcome: the community was told about the issue, the patch author immediately 
responded, and the patch revert happened regardless and more effort is required 
from everyone as a result. Again, I still think the revert is defensible. But I 
also think the push back on the speed at which the revert happened is 
defensible as well. This is mostly just a call to coordinate a bit better. I 
would suggest that if the patch author is engaged on the thread, it would make 
sense to tell them the schedule on which you plan to revert so they have an 
opportunity to tell you if that's going to be disruptive.

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-26 Thread via cfe-commits

alexfh wrote:

> > As I understood the author, more time was needed to prepare the tests and 
> > the fix. I just tried to bring the tip of the tree to a good state soon 
> > (our internal release process was blocked on this) without putting any 
> > pressure on the author.
> 
> Typically when the author is engaging fairly quickly and only needs a short 
> time (he had already identified the problem and only had process-related 
> concerns), it is preferred to be understanding/polite and give them time to 
> work on it.

First of all, let me quote the developer policy: ["Remember, it is normal and 
healthy to have patches reverted. Having a patch reverted does not necessarily 
mean you did anything 
wrong."](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy). 
Now, I totally understand the desire to "capture the progress" as well as 
frustration one might have seeing their work reverted. However, a revert 
shouldn't be perceived as an impolite or unfriendly action. All contributions 
are welcome and appreciated. Reverting a commit is merely a way to deal with 
problems one at a time without blocking others or racing against the clock.

Frequently, it is the most sustainable way forward that helps keeping the tip 
of the tree in a reasonably good shape while not putting unnecessary time 
pressure on contributors. ["As a community, we strongly value having the tip of 
tree in a good state while allowing rapid iterative development. As such, we 
tend to make much heavier use of reverts to keep the tree healthy than some 
other open source projects, and our norms are a bit 
different."](https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy)

A quick and obvious forward fix is also an option sometimes, but though there 
are certain signals like fast responses from all involved parties, the time to 
get a **correct** fix into the tree is unpredictable in general. Sometimes, an 
"obvious" fix turns out to be suboptimal or just wrong, which can be found 
during a review or much later - after it's committed and tested (by us or other 
users). For example, I got a quick response here, but now, two days later the 
fix hasn't been even sent for review (and this is totally fine, since the 
problematic commit is reverted, and the author can work in their pace and with 
their own priorities).

Things get even more complicated, when a problem is long enough in the tree and 
other problems start to pile up. It's a matter of just two-three sufficiently 
widespread issues being simultaneously in the tree, when even finding the 
culprit for each problem becomes much longer and requires more effort. Been 
there, done that. For example, this week we've already seen a handful of clang 
bugs affecting the same files, leading to them be hidden behind each other.

> While I am sympathetic to your internal processes, it seems to me that they 
> should be more tolerant of a ToT that is not particularly stable, as is the 
> case with Clang.

You may be surprised, but ToT Clang (as well as LLVM in general) is quite 
stable. Our release process ensures that every week or so there is an LLVM 
revision that requires just a handful of cherrypicks (sometimes, no cherrypicks 
at all) to correctly compile all of our code, which, given the immense size, is 
a pretty high bar (though, this obviously applies only to the configurations 
and targets we use).

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-24 Thread via cfe-commits

alexfh wrote:

> Doing a revert like this during our release process is actually quite a 
> disturbance, 

My apologies, I didn't realize the additional complexities due to the release.

> particularly when the committer is actively working on the fix (and has 
> identified it!). In the future please give them a chance to fix it before 
> reverting on them.

As I understood the author, more time was needed to prepare the tests and the 
fix. I just tried to bring the tip of the tree to a good state soon (our 
internal release process was blocked on this) without putting any pressure on 
the author.

> ALSO, in the future, if you are going to do a revert while we are in a 
> release process, you need to make sure to submit a request to revert it from 
> the release branch at the same time, you cannot just revert in 'main'.

Should I send a PR to revert the commit in the release branch now?

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-24 Thread via cfe-commits

alexfh wrote:

I'll go ahead and revert the commit for now. My local test run has just 
finished with no new errors.

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-24 Thread via cfe-commits

cor3ntin wrote:

@MitalAshok Please make a new PR!
We can then cherry-pick it to 18

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-24 Thread Mital Ashok via cfe-commits

MitalAshok wrote:

With the standard as is, `B(A)` should be a better match because of 
[[over.ics.list]p(7.1)](https://wg21.link/over.ics.list#7.1), it is an Exact 
Match, and `B(std::vector)` is a user defined conversion.

With the current CWG2311 fix, the call to `B(A)` is an Exact Match because it's 
an `A` prvalue.

https://github.com/llvm/llvm-project/blob/7b11c08c664863fbcd7a3058179b0af3de5d28e4/clang/lib/Sema/SemaOverload.cpp#L1593

^ This line is the issue: `isCopyConstructor()` checks only for lvalue 
reference copy constructors, and the move constructor is selected (i.e., if you 
force a copy constructor to be called instead, it works: 
https://godbolt.org/z/rGEsPdMx8)

Removing `Constructor->isCopyConstructor() &&` fixes the issue (`B(A)` is 
chosen, and all existing tests still pass)

@cor3ntin I have some extra test cases I would want to add too. Should I make a 
new pull request? Or are we going to reopen this one and target llvm 19?


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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-24 Thread via cfe-commits

alexfh wrote:

Given the broad impact we see in our code, I suppose this can affect other 
users as well. I suggest to revert first and then find a proper solution.

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-19 Thread via cfe-commits

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-19 Thread via cfe-commits

cor3ntin wrote:

Let me merge that lest I forget. Our windows bots have a lot of issues

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-19 Thread via cfe-commits

cor3ntin wrote:

Do you want me to merge on your behalf?

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-19 Thread via cfe-commits

cor3ntin wrote:

@MitalAshok the bot found some trailing whitespace issues

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-13 Thread Vlad Serebrennikov via cfe-commits

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

Looks perfect to me now. Thank you!

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-13 Thread Mital Ashok via cfe-commits


@@ -132,6 +142,36 @@ namespace dr2126 { // dr2126: 12
 #endif
 }
 
+namespace dr2137 { // dr2137: 18
+#if __cplusplus >= 201103L
+  struct Q {
+Q();
+Q(Q&&);
+Q(std::initializer_list) = delete; // since-cxx11-note 2 {{has been 
explicitly marked deleted here}}
+  };
+
+  Q x = Q { Q() }; // since-cxx11-error {{call to deleted constructor}}
+
+  int f(Q); // since-cxx11-note {{passing argument to parameter here}}
+  int y = f({ Q() }); // since-cxx11-error {{call to deleted constructor}}

MitalAshok wrote:

@Endilll Is this good now? Thanks in advance

https://github.com/llvm/llvm-project/pull/77768/files#diff-3426b975672f064b9f32caf4ebab729b815f4f65aa3e76bda4b5bdee986e2e6fR130-R131

https://github.com/llvm/llvm-project/pull/77768/files#diff-8807231ab4d193692d6cecdc6428aaf3c55dfd96101cff8fe0eb7034654658fbR154-R161

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-12 Thread Vlad Serebrennikov via cfe-commits


@@ -132,6 +142,36 @@ namespace dr2126 { // dr2126: 12
 #endif
 }
 
+namespace dr2137 { // dr2137: 18
+#if __cplusplus >= 201103L
+  struct Q {
+Q();
+Q(Q&&);
+Q(std::initializer_list) = delete; // since-cxx11-note 2 {{has been 
explicitly marked deleted here}}
+  };
+
+  Q x = Q { Q() }; // since-cxx11-error {{call to deleted constructor}}
+
+  int f(Q); // since-cxx11-note {{passing argument to parameter here}}
+  int y = f({ Q() }); // since-cxx11-error {{call to deleted constructor}}

Endilll wrote:

Can you follow the example of test for CWG2141? Specifically,
1. not trimming diagnostic messages (including identifiers);
2. placing expected errors on the following line (via `// 
since-cxx11-error@-1`);
3. grouping notes together with errors the same way they are grouped in console 
output (2170 is a good example how to handle notes that point to a different 
source line).

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-12 Thread Mark de Wever via cfe-commits

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

Thanks @cor3ntin. The patch still looks fine from libc++'s PoV.

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-11 Thread Mital Ashok via cfe-commits

https://github.com/MitalAshok updated 
https://github.com/llvm/llvm-project/pull/77768

>From 696d4f964805d1af04d4f94dbc8f47adfbc02428 Mon Sep 17 00:00:00 2001
From: Mital Ashok 
Date: Sat, 22 Jul 2023 20:07:00 +0100
Subject: [PATCH 1/2] [SemaCXX] Implement CWG2137 (list-initialization from
 objects of the same type)

Differential Revision: https://reviews.llvm.org/D156032
---
 clang/docs/ReleaseNotes.rst   |  2 +
 clang/lib/Sema/SemaInit.cpp   | 14 +++
 clang/lib/Sema/SemaOverload.cpp   | 38 +-
 clang/test/CXX/drs/dr14xx.cpp | 10 -
 clang/test/CXX/drs/dr21xx.cpp | 40 +++
 clang/www/cxx_dr_status.html  |  2 +-
 .../pairs.pair/ctor.pair_U_V_move.pass.cpp| 21 +-
 7 files changed, 98 insertions(+), 29 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 59732962caac65..a52fb805497756 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -199,6 +199,8 @@ C++2c Feature Support
 
 Resolutions to C++ Defect Reports
 ^
+- Implemented `CWG2137 `_ which allows
+  list-initialization from objects of the same type.
 
 C Language Changes
 --
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 408ee5f775804b..a3a7486af093fb 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4199,7 +4199,7 @@ static OverloadingResult ResolveConstructorOverload(
 /// \param IsListInit Is this list-initialization?
 /// \param IsInitListCopy Is this non-list-initialization resulting from a
 ///   list-initialization from {x} where x is the same
-///   type as the entity?
+///   aggregate type as the entity?
 static void TryConstructorInitialization(Sema ,
  const InitializedEntity ,
  const InitializationKind ,
@@ -4239,8 +4239,8 @@ static void TryConstructorInitialization(Sema ,
   // ObjC++: Lambda captured by the block in the lambda to block conversion
   // should avoid copy elision.
   if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
-  UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isPRValue() &&
-  S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) 
{
+  Args.size() == 1 && Args[0]->isPRValue() &&
+  S.Context.hasSameUnqualifiedType(Args[0]->getType(), DestType)) {
 // Convert qualifications if necessary.
 Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
 if (ILE)
@@ -4571,9 +4571,9 @@ static void TryListInitialization(Sema ,
 return;
   }
 
-  // C++11 [dcl.init.list]p3, per DR1467:
-  // - If T is a class type and the initializer list has a single element of
-  //   type cv U, where U is T or a class derived from T, the object is
+  // C++11 [dcl.init.list]p3, per DR1467 and DR2137:
+  // - If T is an aggregate class and the initializer list has a single element
+  //   of type cv U, where U is T or a class derived from T, the object is
   //   initialized from that element (by copy-initialization for
   //   copy-list-initialization, or by direct-initialization for
   //   direct-list-initialization).
@@ -4584,7 +4584,7 @@ static void TryListInitialization(Sema ,
   // - Otherwise, if T is an aggregate, [...] (continue below).
   if (S.getLangOpts().CPlusPlus11 && InitList->getNumInits() == 1 &&
   !IsDesignatedInit) {
-if (DestType->isRecordType()) {
+if (DestType->isRecordType() && DestType->isAggregateType()) {
   QualType InitType = InitList->getInit(0)->getType();
   if (S.Context.hasSameUnqualifiedType(InitType, DestType) ||
   S.IsDerivedFrom(InitList->getBeginLoc(), InitType, DestType)) {
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 64bc3851980272..30bc2036cf781c 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1562,19 +1562,37 @@ TryUserDefinedConversion(Sema , Expr *From, QualType 
ToType,
 //   called for those cases.
 if (CXXConstructorDecl *Constructor
   = dyn_cast(ICS.UserDefined.ConversionFunction)) {
-  QualType FromCanon
-= S.Context.getCanonicalType(From->getType().getUnqualifiedType());
+  QualType FromType;
+  SourceLocation FromLoc;
+  // C++11 [over.ics.list]p6, per DR2137:
+  // C++17 [over.ics.list]p6:
+  //   If C is not an initializer-list constructor and the initializer list
+  //   has a single element of type cv U, where U is X or a class derived
+  //   from X, the implicit conversion sequence has Exact Match rank if U 
is
+  //   X, or Conversion rank if U is derived from X.
+  if (const auto *InitList = dyn_cast(From);
+  InitList && InitList->getNumInits() == 1 &&
+  

[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-11 Thread Vlad Serebrennikov via cfe-commits

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-11 Thread Vlad Serebrennikov via cfe-commits


@@ -132,6 +142,36 @@ namespace dr2126 { // dr2126: 12
 #endif
 }
 
+namespace dr2137 { // dr2137: 18
+#if __cplusplus >= 201103L
+  struct Q {
+Q();
+Q(Q&&);
+Q(std::initializer_list) = delete; // expected-note 2 {{has been 
explicitly marked deleted here}}
+  };
+
+  Q x = Q { Q() }; // expected-error {{call to deleted constructor}}

Endilll wrote:

Can you redo `expected` directives, so that they are consistent with the rest 
of the file?

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-11 Thread Vlad Serebrennikov via cfe-commits

https://github.com/Endilll requested changes to this pull request.


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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-11 Thread via cfe-commits

cor3ntin wrote:

@mordante ping for visibility (you approved that on Phab already)

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-11 Thread via cfe-commits

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

I think all the comments made on phab have been applied, and I do believe this 
implements the DRs (or their intent)

Please give a few days for other folks to look at it before merging.
@zygoloid in particular as he opened CW2311

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


[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-11 Thread via cfe-commits

llvmbot wrote:



@llvm/pr-subscribers-clang

@llvm/pr-subscribers-libcxx

Author: Mital Ashok (MitalAshok)


Changes

Closes #77638, #24186

Rebased from https://reviews.llvm.org/D156032;, see there for more 
information.

Implements wording change in [CWG2137](https://wg21.link/CWG2137) in the first 
commit.

This also implements an approach to [CWG2311](https://wg21.link/CWG2311) in the 
second commit, because too much code that relies on `T{ T_prvalue }` being an 
elision would break. Because that issue is still open and the CWG issue doesn't 
provide wording to fix the issue, there may be different behaviours on other 
compilers.



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


8 Files Affected:

- (modified) clang/docs/ReleaseNotes.rst (+5) 
- (modified) clang/lib/Sema/SemaInit.cpp (+30-10) 
- (modified) clang/lib/Sema/SemaOverload.cpp (+28-10) 
- (modified) clang/test/CXX/drs/dr14xx.cpp (-10) 
- (modified) clang/test/CXX/drs/dr21xx.cpp (+40) 
- (modified) clang/test/CXX/drs/dr23xx.cpp (+84) 
- (modified) clang/www/cxx_dr_status.html (+2-2) 
- (modified) 
libcxx/test/std/utilities/utility/pairs/pairs.pair/ctor.pair_U_V_move.pass.cpp 
(+20-1) 


``diff
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 59732962caac65..66d388bb954bda 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -199,6 +199,11 @@ C++2c Feature Support
 
 Resolutions to C++ Defect Reports
 ^
+- Implemented `CWG2137 `_ which allows
+  list-initialization from objects of the same type.
+- Implemented `CWG2311 `_: given a prvalue ``e`` of 
object type
+  ``T``, ``T{e}`` will try to resolve an initializer list constructor and will 
use it if successful (CWG2137).
+  Otherwise, if there is no initializer list constructor, the copy will be 
elided as if it was ``T(e)``.
 
 C Language Changes
 --
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 408ee5f775804b..2822a8d5d78b3e 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4199,7 +4199,7 @@ static OverloadingResult ResolveConstructorOverload(
 /// \param IsListInit Is this list-initialization?
 /// \param IsInitListCopy Is this non-list-initialization resulting from a
 ///   list-initialization from {x} where x is the same
-///   type as the entity?
+///   aggregate type as the entity?
 static void TryConstructorInitialization(Sema ,
  const InitializedEntity ,
  const InitializationKind ,
@@ -4229,6 +4229,14 @@ static void TryConstructorInitialization(Sema ,
 Entity.getKind() !=
 InitializedEntity::EK_LambdaToBlockConversionBlockElement);
 
+  bool CopyElisionPossible = false;
+  auto ElideConstructor = [&] {
+// Convert qualifications if necessary.
+Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
+if (ILE)
+  Sequence.RewrapReferenceInitList(DestType, ILE);
+  };
+
   // C++17 [dcl.init]p17:
   // - If the initializer expression is a prvalue and the cv-unqualified
   //   version of the source type is the same class as the class of the
@@ -4241,11 +4249,17 @@ static void TryConstructorInitialization(Sema ,
   if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
   UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isPRValue() &&
   S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) 
{
-// Convert qualifications if necessary.
-Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
-if (ILE)
-  Sequence.RewrapReferenceInitList(DestType, ILE);
-return;
+if (ILE && !DestType->isAggregateType()) {
+  // CWG2311: T{ prvalue_of_type_T } is not eligible for copy elision
+  // Make this an elision if this won't call an initializer-list
+  // constructor. (Always on an aggregate type or check constructors 
first.)
+  assert(!IsInitListCopy &&
+ "IsInitListCopy only possible with aggregate types");
+  CopyElisionPossible = true;
+} else {
+  ElideConstructor();
+  return;
+}
   }
 
   const RecordType *DestRecordType = DestType->getAs();
@@ -4290,6 +4304,12 @@ static void TryConstructorInitialization(Sema ,
   S, Kind.getLocation(), Args, CandidateSet, DestType, Ctors, Best,
   CopyInitialization, AllowExplicit,
   /*OnlyListConstructors=*/true, IsListInit, RequireActualConstructor);
+
+if (CopyElisionPossible && Result == OR_No_Viable_Function) {
+  // No initializer list candidate
+  ElideConstructor();
+  return;
+}
   }
 
   // C++11 [over.match.list]p1:
@@ -4571,9 +4591,9 @@ static void TryListInitialization(Sema ,
 return;
   }
 
-  // C++11 [dcl.init.list]p3, per DR1467:
-  // - If T is 

[clang] [libcxx] [SemaCXX] Implement CWG2137 (list-initialization from objects of the same type) (PR #77768)

2024-01-11 Thread Mital Ashok via cfe-commits

https://github.com/MitalAshok created 
https://github.com/llvm/llvm-project/pull/77768

Closes #77638, #24186

Rebased from , see there for more information.

Implements wording change in [CWG2137](https://wg21.link/CWG2137) in the first 
commit.

This also implements an approach to [CWG2311](https://wg21.link/CWG2311) in the 
second commit, because too much code that relies on `T{ T_prvalue }` being an 
elision would break. Because that issue is still open and the CWG issue doesn't 
provide wording to fix the issue, there may be different behaviours on other 
compilers.



>From 08e2db75625ff4494f352df70922174275d7c2dd Mon Sep 17 00:00:00 2001
From: Mital Ashok 
Date: Sat, 22 Jul 2023 20:07:00 +0100
Subject: [PATCH 1/2] [SemaCXX] Implement CWG2137 (list-initialization from
 objects of the same type)

Differential Revision: https://reviews.llvm.org/D156032
---
 clang/docs/ReleaseNotes.rst   |  2 +
 clang/lib/Sema/SemaInit.cpp   | 14 +++
 clang/lib/Sema/SemaOverload.cpp   | 38 +-
 clang/test/CXX/drs/dr14xx.cpp | 10 -
 clang/test/CXX/drs/dr21xx.cpp | 40 +++
 clang/www/cxx_dr_status.html  |  2 +-
 .../pairs.pair/ctor.pair_U_V_move.pass.cpp| 21 +-
 7 files changed, 98 insertions(+), 29 deletions(-)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 59732962caac65..a52fb805497756 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -199,6 +199,8 @@ C++2c Feature Support
 
 Resolutions to C++ Defect Reports
 ^
+- Implemented `CWG2137 `_ which allows
+  list-initialization from objects of the same type.
 
 C Language Changes
 --
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 408ee5f775804b..a3a7486af093fb 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -4199,7 +4199,7 @@ static OverloadingResult ResolveConstructorOverload(
 /// \param IsListInit Is this list-initialization?
 /// \param IsInitListCopy Is this non-list-initialization resulting from a
 ///   list-initialization from {x} where x is the same
-///   type as the entity?
+///   aggregate type as the entity?
 static void TryConstructorInitialization(Sema ,
  const InitializedEntity ,
  const InitializationKind ,
@@ -4239,8 +4239,8 @@ static void TryConstructorInitialization(Sema ,
   // ObjC++: Lambda captured by the block in the lambda to block conversion
   // should avoid copy elision.
   if (S.getLangOpts().CPlusPlus17 && !RequireActualConstructor &&
-  UnwrappedArgs.size() == 1 && UnwrappedArgs[0]->isPRValue() &&
-  S.Context.hasSameUnqualifiedType(UnwrappedArgs[0]->getType(), DestType)) 
{
+  Args.size() == 1 && Args[0]->isPRValue() &&
+  S.Context.hasSameUnqualifiedType(Args[0]->getType(), DestType)) {
 // Convert qualifications if necessary.
 Sequence.AddQualificationConversionStep(DestType, VK_PRValue);
 if (ILE)
@@ -4571,9 +4571,9 @@ static void TryListInitialization(Sema ,
 return;
   }
 
-  // C++11 [dcl.init.list]p3, per DR1467:
-  // - If T is a class type and the initializer list has a single element of
-  //   type cv U, where U is T or a class derived from T, the object is
+  // C++11 [dcl.init.list]p3, per DR1467 and DR2137:
+  // - If T is an aggregate class and the initializer list has a single element
+  //   of type cv U, where U is T or a class derived from T, the object is
   //   initialized from that element (by copy-initialization for
   //   copy-list-initialization, or by direct-initialization for
   //   direct-list-initialization).
@@ -4584,7 +4584,7 @@ static void TryListInitialization(Sema ,
   // - Otherwise, if T is an aggregate, [...] (continue below).
   if (S.getLangOpts().CPlusPlus11 && InitList->getNumInits() == 1 &&
   !IsDesignatedInit) {
-if (DestType->isRecordType()) {
+if (DestType->isRecordType() && DestType->isAggregateType()) {
   QualType InitType = InitList->getInit(0)->getType();
   if (S.Context.hasSameUnqualifiedType(InitType, DestType) ||
   S.IsDerivedFrom(InitList->getBeginLoc(), InitType, DestType)) {
diff --git a/clang/lib/Sema/SemaOverload.cpp b/clang/lib/Sema/SemaOverload.cpp
index 64bc3851980272..30bc2036cf781c 100644
--- a/clang/lib/Sema/SemaOverload.cpp
+++ b/clang/lib/Sema/SemaOverload.cpp
@@ -1562,19 +1562,37 @@ TryUserDefinedConversion(Sema , Expr *From, QualType 
ToType,
 //   called for those cases.
 if (CXXConstructorDecl *Constructor
   = dyn_cast(ICS.UserDefined.ConversionFunction)) {
-  QualType FromCanon
-= S.Context.getCanonicalType(From->getType().getUnqualifiedType());
+  QualType