[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values
Alan Zhao via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 11 14:37:17 PST 2023
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
More information about the cfe-commits
mailing list