[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

[Alan Zhao via Phabricator via cfe-commits]

ayzhao added inline comments.


================
Comment at: clang/lib/Sema/SemaInit.cpp:6198-6199
+              dyn_cast<CXXRecordDecl>(DestType->getAs<RecordType>()->getDecl());
+          S.getLangOpts().CPlusPlus20 && RD && RD->isAggregate() && Failed() &&
+          getFailureKind() == FK_ConstructorOverloadFailed &&
+          onlyHasDefaultedCtors(getFailedCandidateSet())) {
----------------
rsmith wrote:
> For when you look at re-landing this, we encountered a regression for this case:
> 
> ```
> struct A {};
> struct B : A { Noncopyable nc; };
> ```
> 
> Here, given `B b`, a `B(b)` direct-initialization should be rejected because it selects a deleted copy constructor of `B`, but it looks like Clang instead performed aggregate initialization, as if by `B{(const A&)b, {}}`. This is pretty bad -- copies that should be disallowed end up working but not actually copying any members of the derived class!
> 
> The general problem is that Clang's overload resolution incorrectly handles deleted functions, treating them as an overload resolution failure rather than a success. In this particular case, what happens is constructor overload resolution produces `OR_Deleted`, which gets mapped into `FK_ConstructorOverloadFailed` despite the fact that overload resolution succeeded and picked a viable function.
> 
> I guess we can split `FK_*OverloadFailed` into separate "failed" and "succeeded but found a deleted function" cases?
> For when you look at re-landing this, we encountered a regression for this case:
> 
> ```
> struct A {};
> struct B : A { Noncopyable nc; };
> ```
> 
> Here, given `B b`, a `B(b)` direct-initialization should be rejected because it selects a deleted copy constructor of `B`, but it looks like Clang instead performed aggregate initialization, as if by `B{(const A&)b, {}}`. This is pretty bad -- copies that should be disallowed end up working but not actually copying any members of the derived class!
> 
> The general problem is that Clang's overload resolution incorrectly handles deleted functions, treating them as an overload resolution failure rather than a success. In this particular case, what happens is constructor overload resolution produces `OR_Deleted`, which gets mapped into `FK_ConstructorOverloadFailed` despite the fact that overload resolution succeeded and picked a viable function.
> 
> I guess we can split `FK_*OverloadFailed` into separate "failed" and "succeeded but found a deleted function" cases?

This was the cause of https://github.com/llvm/llvm-project/issues/59675 as well. The reland at D141546 should handle this case.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129531/new/

https://reviews.llvm.org/D129531