logan-5 created this revision.
logan-5 added a reviewer: rsmith.
logan-5 added a project: clang.
Herald added a subscriber: cfe-commits.
The current text of the 'note' diagnostic for bad conversions is confusing in
the presence of user-defined conversion operations: https://godbolt.org/z/zrgeHH
For the first error, the conversion function is simply pointed at in a note
without any further explanation. For the second error, the final note claims
there is no conversion from X2 to Y2, even though it is plainly visible in the
source code. The reason for the error is that only one implicit user-defined
conversion may be applied in these circumstances, but the error message is
misleading to those who may not know this rule.
This is determined by searching for these illegal conversions in
CompleteNonViableCandidate after overload resolution fails.
Depends on D74234 <https://reviews.llvm.org/D74234>. Also depends on D74235
<https://reviews.llvm.org/D74235>.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D74238
Files:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Overload.h
clang/include/clang/Sema/Sema.h
clang/lib/Sema/SemaOverload.cpp
clang/test/CXX/drs/dr0xx.cpp
clang/test/SemaCXX/address-space-ctor.cpp
clang/test/SemaCXX/constructor-initializer.cpp
clang/test/SemaCXX/user-defined-conversions.cpp
Index: clang/test/SemaCXX/user-defined-conversions.cpp
===================================================================
--- clang/test/SemaCXX/user-defined-conversions.cpp
+++ clang/test/SemaCXX/user-defined-conversions.cpp
@@ -97,3 +97,51 @@
a = b; // expected-error{{calling a private constructor of class 'rdar10202900::A'}}
}
}
+
+namespace more_than_one {
+
+namespace std_example {
+struct X {
+ operator int();
+};
+
+struct Y {
+ operator X(); // expected-note{{converting from 'more_than_one::std_example::Y' to 'more_than_one::std_example::X' would apply more than one implicit user-defined conversion}}
+};
+
+Y a;
+int b = a; // expected-error{{no viable conversion}}
+int c = X(a);
+}
+
+namespace ctors {
+struct X {};
+struct Y {
+ Y(X);
+};
+struct Z { // expected-note 2{{candidate constructor (the implicit}}
+ Z(Y); // expected-note{{converting from 'more_than_one::ctors::X' to 'more_than_one::ctors::Y' would apply more than one implicit user-defined conversion}}
+};
+
+X x;
+Z z = x; // expected-error{{no viable conversion}}
+Z z2 = Y(x);
+}
+
+namespace ops {
+
+struct Y;
+struct X {
+ operator Y() const; // expected-note {{converting from 'more_than_one::ops::X' to 'more_than_one::ops::Y' would apply more than one implicit user-defined conversion}}
+};
+struct Y {};
+struct Z { // expected-note 2{{candidate constructor (the implicit}}
+ Z(Y); // expected-note{{converting from 'more_than_one::ops::X' to 'more_than_one::ops::Y' would apply more than one implicit user-defined conversion}}
+};
+
+X x;
+Z z = x; // expected-error{{no viable conversion}}
+Z z2 = static_cast<Y>(x);
+}
+} // namespace more_than_one
+
Index: clang/test/SemaCXX/constructor-initializer.cpp
===================================================================
--- clang/test/SemaCXX/constructor-initializer.cpp
+++ clang/test/SemaCXX/constructor-initializer.cpp
@@ -306,7 +306,7 @@
namespace PR10758 {
struct A;
struct B {
- B (A const &); // expected-note 2 {{candidate constructor not viable: no known conversion from 'const PR10758::B' to 'const PR10758::A &' for 1st argument}}
+ B (A const &); // expected-note 2 {{candidate constructor not viable: converting from 'const PR10758::B' to 'const PR10758::A &' would apply more than one implicit user-defined conversion}}
B (B &); // expected-note 2 {{candidate constructor not viable: 1st argument ('const PR10758::B') would lose const qualifier}}
};
struct A {
Index: clang/test/SemaCXX/address-space-ctor.cpp
===================================================================
--- clang/test/SemaCXX/address-space-ctor.cpp
+++ clang/test/SemaCXX/address-space-ctor.cpp
@@ -6,8 +6,8 @@
int i;
};
-//expected-note@-5{{candidate constructor (the implicit copy constructor) not viable: no known conversion from 'int' to 'const MyType &' for 1st argument}}
-//expected-note@-6{{candidate constructor (the implicit move constructor) not viable: no known conversion from 'int' to 'MyType &&' for 1st argument}}
+//expected-note@-5{{candidate constructor (the implicit copy constructor) not viable: converting from 'int' to 'const MyType &' would apply more than one implicit user-defined conversion}}
+//expected-note@-6{{candidate constructor (the implicit move constructor) not viable: converting from 'int' to 'MyType &&' would apply more than one implicit user-defined conversion}}
//expected-note@-6{{candidate constructor ignored: cannot be used to construct an object in address space '__attribute__((address_space(10)))'}}
//expected-note@-8{{candidate constructor ignored: cannot be used to construct an object in address space '__attribute__((address_space(10)))'}}
//expected-note@-9{{candidate constructor ignored: cannot be used to construct an object in address space '__attribute__((address_space(10)))'}}
Index: clang/test/CXX/drs/dr0xx.cpp
===================================================================
--- clang/test/CXX/drs/dr0xx.cpp
+++ clang/test/CXX/drs/dr0xx.cpp
@@ -964,7 +964,7 @@
struct C {};
struct B {
B(B&); // expected-note 0-1{{candidate}}
- B(C); // expected-note 0-1{{no known conversion from 'dr84::B' to 'dr84::C'}}
+ B(C); // expected-note 0-1{{'dr84::B' to 'dr84::C' would apply more than one implicit user-defined conversion}}
operator C() const;
};
A a;
Index: clang/lib/Sema/SemaOverload.cpp
===================================================================
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -94,7 +94,8 @@
UserDefinedConversionSequence& User,
OverloadCandidateSet& Conversions,
AllowedExplicit AllowExplicit,
- bool AllowObjCConversionOnExplicit);
+ bool AllowObjCConversionOnExplicit,
+ bool AllowMultipleUserDefined);
static ImplicitConversionSequence::CompareKind
CompareStandardConversionSequences(Sema &S, SourceLocation Loc,
@@ -1332,12 +1333,16 @@
return ICS;
}
+ bool AllowMultipleUserDefined
+ = UserConversions == Sema::UserDefinedConversions::AllowMultiple;
+
// Attempt user-defined conversion.
OverloadCandidateSet Conversions(From->getExprLoc(),
OverloadCandidateSet::CSK_Normal);
switch (IsUserDefinedConversion(S, From, ToType, ICS.UserDefined,
Conversions, AllowExplicit,
- AllowObjCConversionOnExplicit)) {
+ AllowObjCConversionOnExplicit,
+ AllowMultipleUserDefined)) {
case OR_Success:
case OR_Deleted:
ICS.setUserDefined();
@@ -3400,7 +3405,8 @@
UserDefinedConversionSequence &User,
OverloadCandidateSet &CandidateSet,
AllowedExplicit AllowExplicit,
- bool AllowObjCConversionOnExplicit) {
+ bool AllowObjCConversionOnExplicit,
+ bool AllowMultipleUserDefined) {
assert(AllowExplicit != AllowedExplicit::None ||
!AllowObjCConversionOnExplicit);
CandidateSet.clear(OverloadCandidateSet::CSK_InitByUserDefinedConversion);
@@ -3460,7 +3466,8 @@
Usable = Usable && Info.Constructor->isConvertingConstructor(
/*AllowExplicit*/ true);
if (Usable) {
- bool SuppressUserConversions = !ConstructorsOnly;
+ bool SuppressUserConversions
+ = !AllowMultipleUserDefined && !ConstructorsOnly;
if (SuppressUserConversions && ListInitializing) {
SuppressUserConversions = false;
if (NumArgs == 1) {
@@ -3612,7 +3619,8 @@
OverloadCandidateSet::CSK_Normal);
OverloadingResult OvResult =
IsUserDefinedConversion(*this, From, ToType, ICS.UserDefined,
- CandidateSet, AllowedExplicit::None, false);
+ CandidateSet, AllowedExplicit::None, false,
+ /*AllowMultipleUserDefined=*/false);
if (!(OvResult == OR_Ambiguous ||
(OvResult == OR_No_Viable_Function && !CandidateSet.empty())))
@@ -5412,6 +5420,7 @@
case BadConversionSequence::no_conversion:
case BadConversionSequence::unrelated_class:
+ case BadConversionSequence::multiple_user_defined:
break;
}
@@ -7320,6 +7329,9 @@
break;
case ImplicitConversionSequence::BadConversion:
+ // Save the bad conversion in the candidate, for later use by diagnostics.
+ Candidate.Conversions[0].setBad(BadConversionSequence::no_conversion,
+ From, CallResultType);
Candidate.Viable = false;
Candidate.FailureKind = ovl_fail_bad_final_conversion;
return;
@@ -10186,6 +10198,15 @@
QualType FromTy = Conv.Bad.getFromType();
QualType ToTy = Conv.Bad.getToType();
+ if (Conv.Bad.Kind == BadConversionSequence::multiple_user_defined) {
+ S.Diag(Fn->getLocation(),
+ diag::note_ovl_candidate_multiple_user_defined_conv)
+ << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second
+ << FnDesc << (FromExpr ? FromExpr->getSourceRange() : SourceRange())
+ << FromTy << ToTy;
+ return;
+ }
+
if (FromTy == S.Context.OverloadTy) {
assert(FromExpr && "overload set argument came from implicit argument?");
Expr *E = FromExpr->IgnoreParens();
@@ -10956,8 +10977,6 @@
case ovl_fail_trivial_conversion:
case ovl_fail_bad_final_conversion:
case ovl_fail_final_conversion_not_exact:
- return S.NoteOverloadCandidate(Cand->FoundDecl, Fn, Cand->getRewriteKind());
-
case ovl_fail_bad_conversion: {
unsigned I = (Cand->IgnoreObjectArgument ? 1 : 0);
for (unsigned N = Cand->Conversions.size(); I != N; ++I)
@@ -11277,6 +11296,36 @@
};
}
+/// See if a conversion could have been completed if multiple implicit
+/// user-defined conversions were allowed.
+static Optional<ImplicitConversionSequence>
+TryMultipleUserDefinedConversions(Sema &S,
+ const ImplicitConversionSequence& ICS) {
+ assert(ICS.isBad());
+
+ if (ICS.Bad.Kind != BadConversionSequence::no_conversion)
+ return None;
+
+ // Skip implicit object arguments.
+ if (!ICS.Bad.FromExpr)
+ return None;
+
+ ImplicitConversionSequence Multiple =
+ TryCopyInitialization(S, ICS.Bad.FromExpr, ICS.Bad.getToType(),
+ Sema::UserDefinedConversions::AllowMultiple,
+ /*InOverloadResolution=*/true,
+ /*AllowObjCWritebackConversion=*/
+ S.getLangOpts().ObjCAutoRefCount);
+
+ if (!Multiple.isBad()) {
+ Multiple.setBad(BadConversionSequence::multiple_user_defined,
+ ICS.Bad.getFromType(), ICS.Bad.getToType());
+ return Multiple;
+ }
+
+ return None;
+}
+
/// CompleteNonViableCandidate - Normally, overload resolution only
/// computes up to the first bad conversion. Produces the FixIt set if
/// possible.
@@ -11286,7 +11335,23 @@
OverloadCandidateSet::CandidateSetKind CSK) {
assert(!Cand->Viable);
- // Don't do anything on failures other than bad conversion.
+ auto SetAsMultipleUserDefined = [&S](ImplicitConversionSequence& ICS) {
+ if (auto Multiple = TryMultipleUserDefinedConversions(S, ICS)) {
+ ICS = *Multiple;
+ return true;
+ }
+ return false;
+ };
+
+ // Conversion candidates may hold a bad conversion that could have been
+ // completed if multiple user-defined conversions were allowed.
+ if (Cand->FailureKind == ovl_fail_bad_final_conversion
+ && Cand->Conversions[0].isBad()) {
+ SetAsMultipleUserDefined(Cand->Conversions[0]);
+ return;
+ }
+
+ // Don't do anything else on failures other than bad conversion.
if (Cand->FailureKind != ovl_fail_bad_conversion)
return;
@@ -11302,7 +11367,8 @@
assert(ConvIdx != ConvCount && "no bad conversion in candidate");
if (Cand->Conversions[ConvIdx].isInitialized() &&
Cand->Conversions[ConvIdx].isBad()) {
- Unfixable = !Cand->TryToFixBadConversion(ConvIdx, S);
+ if (!SetAsMultipleUserDefined(Cand->Conversions[ConvIdx]))
+ Unfixable = !Cand->TryToFixBadConversion(ConvIdx, S);
break;
}
}
@@ -11357,8 +11423,11 @@
/*AllowObjCWritebackConversion=*/
S.getLangOpts().ObjCAutoRefCount);
// Store the FixIt in the candidate if it exists.
- if (!Unfixable && Cand->Conversions[ConvIdx].isBad())
- Unfixable = !Cand->TryToFixBadConversion(ConvIdx, S);
+ if (Cand->Conversions[ConvIdx].isBad()) {
+ if (!SetAsMultipleUserDefined(Cand->Conversions[ConvIdx])
+ && !Unfixable)
+ Unfixable = !Cand->TryToFixBadConversion(ConvIdx, S);
+ }
}
} else
Cand->Conversions[ConvIdx].setEllipsis();
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -2956,6 +2956,11 @@
/// Don't find user-defined conversions.
Suppress,
+
+ /// Allow multiple user-defined conversions in one implicit conversion
+ /// sequence. Illegal in C++, but useful for finding and pointing out
+ /// invalid sequences in diagnostics.
+ AllowMultiple,
};
enum class AllowedExplicit {
Index: clang/include/clang/Sema/Overload.h
===================================================================
--- clang/include/clang/Sema/Overload.h
+++ clang/include/clang/Sema/Overload.h
@@ -466,7 +466,8 @@
unrelated_class,
bad_qualifiers,
lvalue_ref_to_rvalue,
- rvalue_ref_to_lvalue
+ rvalue_ref_to_lvalue,
+ multiple_user_defined,
};
// This can be null, e.g. for implicit object arguments.
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4068,6 +4068,12 @@
"; take the address of the argument with &|"
"; remove *|"
"; remove &}7">;
+def note_ovl_candidate_multiple_user_defined_conv : Note<
+ "candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
+ "converting "
+ "%diff{from $ to $|from argument type to parameter type}3,4 "
+ "would apply more than one implicit user-defined conversion"
+ >;
def note_ovl_candidate_bad_arc_conv : Note<
"candidate %sub{select_ovl_candidate_kind}0,1,2 not viable: "
"cannot implicitly convert argument "
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
