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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
