From ad323a856c0bf759e07d37d749510ddf4f8c6b96 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Wed, 27 Jan 2010 03:51:04 +0000 Subject: [PATCH] Fix a major oversight in the comparison of standard conversion sequences, where we would occasionally determine (incorrectly) that one standard conversion sequence was a proper subset of another when, in fact, they contained completely incomparable conversions. This change records the types in each step within a standard conversion sequence, so that we can check the specific comparison types to determine when one sequence is a proper subset of the other. Fixes this testcase (thanks, Anders!), which was distilled from PR6095 (also thanks to Anders). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@94660 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/SemaDeclCXX.cpp | 8 ++- lib/Sema/SemaExprCXX.cpp | 4 +- lib/Sema/SemaOverload.cpp | 85 +++++++++++++++++--------- lib/Sema/SemaOverload.h | 23 +++++-- test/SemaCXX/overload-call-copycon.cpp | 3 +- test/SemaCXX/overload-call.cpp | 12 ++++ 6 files changed, 94 insertions(+), 41 deletions(-) diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index a4b3eba4fb..f65dc4bd66 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -4455,7 +4455,9 @@ Sema::CheckReferenceInit(Expr *&Init, QualType DeclType, ICS->Standard.Second = DerivedToBase? ICK_Derived_To_Base : ICK_Identity; ICS->Standard.Third = ICK_Identity; ICS->Standard.FromTypePtr = T2.getAsOpaquePtr(); - ICS->Standard.ToTypePtr = T1.getAsOpaquePtr(); + ICS->Standard.setToType(0, T2); + ICS->Standard.setToType(1, T1); + ICS->Standard.setToType(2, T1); ICS->Standard.ReferenceBinding = true; ICS->Standard.DirectBinding = true; ICS->Standard.RRefBinding = false; @@ -4645,7 +4647,9 @@ Sema::CheckReferenceInit(Expr *&Init, QualType DeclType, ICS->Standard.Second = DerivedToBase? ICK_Derived_To_Base : ICK_Identity; ICS->Standard.Third = ICK_Identity; ICS->Standard.FromTypePtr = T2.getAsOpaquePtr(); - ICS->Standard.ToTypePtr = T1.getAsOpaquePtr(); + ICS->Standard.setToType(0, T2); + ICS->Standard.setToType(1, T1); + ICS->Standard.setToType(2, T1); ICS->Standard.ReferenceBinding = true; ICS->Standard.DirectBinding = false; ICS->Standard.RRefBinding = isRValRef; diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 3ff750ed4f..847991d6f5 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -1487,9 +1487,9 @@ QualType Sema::CheckPointerToMemberOperands( static QualType TargetType(const ImplicitConversionSequence &ICS) { switch (ICS.getKind()) { case ImplicitConversionSequence::StandardConversion: - return ICS.Standard.getToType(); + return ICS.Standard.getToType(2); case ImplicitConversionSequence::UserDefinedConversion: - return ICS.UserDefined.After.getToType(); + return ICS.UserDefined.After.getToType(2); case ImplicitConversionSequence::AmbiguousConversion: return ICS.Ambiguous.getToType(); case ImplicitConversionSequence::EllipsisConversion: diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index f2f24ea59a..2d48674228 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -148,7 +148,7 @@ bool StandardConversionSequence::isPointerConversionToBool() const { // array-to-pointer or function-to-pointer implicit conversions, so // check for their presence as well as checking whether FromType is // a pointer. - if (getToType()->isBooleanType() && + if (getToType(1)->isBooleanType() && (getFromType()->isPointerType() || getFromType()->isBlockPointerType() || First == ICK_Array_To_Pointer || First == ICK_Function_To_Pointer)) return true; @@ -164,7 +164,7 @@ bool StandardConversionSequence:: isPointerConversionToVoidPointer(ASTContext& Context) const { QualType FromType = getFromType(); - QualType ToType = getToType(); + QualType ToType = getToType(1); // Note that FromType has not necessarily been transformed by the // array-to-pointer implicit conversion, so check for its presence @@ -477,7 +477,7 @@ Sema::TryImplicitConversion(Expr* From, QualType ToType, ICS.setStandard(); ICS.Standard.setAsIdentityConversion(); ICS.Standard.setFromType(From->getType()); - ICS.Standard.setToType(ToType); + ICS.Standard.setAllToTypes(ToType); ICS.Standard.CopyConstructor = Constructor; if (ToCanon != FromCanon) ICS.Standard.Second = ICK_Derived_To_Base; @@ -595,7 +595,7 @@ Sema::IsStandardConversion(Expr* From, QualType ToType, // conversion (4.4). (C++ 4.2p2) SCS.Second = ICK_Identity; SCS.Third = ICK_Qualification; - SCS.setToType(ToType); + SCS.setAllToTypes(FromType); return true; } } else if (FromType->isFunctionType() && argIsLvalue == Expr::LV_Valid) { @@ -634,6 +634,7 @@ Sema::IsStandardConversion(Expr* From, QualType ToType, // We don't require any conversions for the first step. SCS.First = ICK_Identity; } + SCS.setToType(0, FromType); // The second conversion can be an integral promotion, floating // point promotion, integral conversion, floating point conversion, @@ -714,6 +715,7 @@ Sema::IsStandardConversion(Expr* From, QualType ToType, // No second conversion required. SCS.Second = ICK_Identity; } + SCS.setToType(1, FromType); QualType CanonFrom; QualType CanonTo; @@ -740,13 +742,13 @@ Sema::IsStandardConversion(Expr* From, QualType ToType, CanonFrom = CanonTo; } } + SCS.setToType(2, FromType); // If we have not converted the argument type to the parameter type, // this is a bad conversion sequence. if (CanonFrom != CanonTo) return false; - SCS.setToType(FromType); return true; } @@ -1605,7 +1607,7 @@ OverloadingResult Sema::IsUserDefinedConversion(Expr *From, QualType ToType, User.After.setAsIdentityConversion(); User.After.setFromType( ThisType->getAs()->getPointeeType()); - User.After.setToType(ToType); + User.After.setAllToTypes(ToType); return OR_Success; } else if (CXXConversionDecl *Conversion = dyn_cast(Best->Function)) { @@ -1722,6 +1724,43 @@ Sema::CompareImplicitConversionSequences(const ImplicitConversionSequence& ICS1, return ImplicitConversionSequence::Indistinguishable; } +// Per 13.3.3.2p3, compare the given standard conversion sequences to +// determine if one is a proper subset of the other. +static ImplicitConversionSequence::CompareKind +compareStandardConversionSubsets(ASTContext &Context, + const StandardConversionSequence& SCS1, + const StandardConversionSequence& SCS2) { + ImplicitConversionSequence::CompareKind Result + = ImplicitConversionSequence::Indistinguishable; + + if (SCS1.Second != SCS2.Second) { + if (SCS1.Second == ICK_Identity) + Result = ImplicitConversionSequence::Better; + else if (SCS2.Second == ICK_Identity) + Result = ImplicitConversionSequence::Worse; + else + return ImplicitConversionSequence::Indistinguishable; + } else if (!Context.hasSameType(SCS1.getToType(1), SCS2.getToType(1))) + return ImplicitConversionSequence::Indistinguishable; + + if (SCS1.Third == SCS2.Third) { + return Context.hasSameType(SCS1.getToType(2), SCS2.getToType(2))? Result + : ImplicitConversionSequence::Indistinguishable; + } + + if (SCS1.Third == ICK_Identity) + return Result == ImplicitConversionSequence::Worse + ? ImplicitConversionSequence::Indistinguishable + : ImplicitConversionSequence::Better; + + if (SCS2.Third == ICK_Identity) + return Result == ImplicitConversionSequence::Better + ? ImplicitConversionSequence::Indistinguishable + : ImplicitConversionSequence::Worse; + + return ImplicitConversionSequence::Indistinguishable; +} + /// CompareStandardConversionSequences - Compare two standard /// conversion sequences to determine whether one is better than the /// other or if they are indistinguishable (C++ 13.3.3.2p3). @@ -1737,21 +1776,9 @@ Sema::CompareStandardConversionSequences(const StandardConversionSequence& SCS1, // excluding any Lvalue Transformation; the identity conversion // sequence is considered to be a subsequence of any // non-identity conversion sequence) or, if not that, - if (SCS1.Second == SCS2.Second && SCS1.Third == SCS2.Third) - // Neither is a proper subsequence of the other. Do nothing. - ; - else if ((SCS1.Second == ICK_Identity && SCS1.Third == SCS2.Third) || - (SCS1.Third == ICK_Identity && SCS1.Second == SCS2.Second) || - (SCS1.Second == ICK_Identity && - SCS1.Third == ICK_Identity)) - // SCS1 is a proper subsequence of SCS2. - return ImplicitConversionSequence::Better; - else if ((SCS2.Second == ICK_Identity && SCS2.Third == SCS1.Third) || - (SCS2.Third == ICK_Identity && SCS2.Second == SCS1.Second) || - (SCS2.Second == ICK_Identity && - SCS2.Third == ICK_Identity)) - // SCS2 is a proper subsequence of SCS1. - return ImplicitConversionSequence::Worse; + if (ImplicitConversionSequence::CompareKind CK + = compareStandardConversionSubsets(Context, SCS1, SCS2)) + return CK; // -- the rank of S1 is better than the rank of S2 (by the rules // defined below), or, if not that, @@ -1856,8 +1883,8 @@ Sema::CompareStandardConversionSequences(const StandardConversionSequence& SCS1, // top-level cv-qualifiers, and the type to which the reference // initialized by S2 refers is more cv-qualified than the type // to which the reference initialized by S1 refers. - QualType T1 = SCS1.getToType(); - QualType T2 = SCS2.getToType(); + QualType T1 = SCS1.getToType(2); + QualType T2 = SCS2.getToType(2); T1 = Context.getCanonicalType(T1); T2 = Context.getCanonicalType(T2); Qualifiers T1Quals, T2Quals; @@ -1898,8 +1925,8 @@ Sema::CompareQualificationConversions(const StandardConversionSequence& SCS1, // FIXME: the example in the standard doesn't use a qualification // conversion (!) - QualType T1 = QualType::getFromOpaquePtr(SCS1.ToTypePtr); - QualType T2 = QualType::getFromOpaquePtr(SCS2.ToTypePtr); + QualType T1 = SCS1.getToType(2); + QualType T2 = SCS2.getToType(2); T1 = Context.getCanonicalType(T1); T2 = Context.getCanonicalType(T2); Qualifiers T1Quals, T2Quals; @@ -1988,9 +2015,9 @@ ImplicitConversionSequence::CompareKind Sema::CompareDerivedToBaseConversions(const StandardConversionSequence& SCS1, const StandardConversionSequence& SCS2) { QualType FromType1 = SCS1.getFromType(); - QualType ToType1 = SCS1.getToType(); + QualType ToType1 = SCS1.getToType(1); QualType FromType2 = SCS2.getFromType(); - QualType ToType2 = SCS2.getToType(); + QualType ToType2 = SCS2.getToType(1); // Adjust the types we're converting from via the array-to-pointer // conversion, if we need to. @@ -2280,7 +2307,7 @@ Sema::TryObjectArgumentInitialization(QualType OrigFromType, // Success. Mark this as a reference binding. ICS.setStandard(); ICS.Standard.setFromType(FromType); - ICS.Standard.setToType(ImplicitParamType); + ICS.Standard.setAllToTypes(ImplicitParamType); ICS.Standard.ReferenceBinding = true; ICS.Standard.DirectBinding = true; ICS.Standard.RRefBinding = false; @@ -2767,7 +2794,7 @@ Sema::AddConversionCandidate(CXXConversionDecl *Conversion, Candidate.IgnoreObjectArgument = false; Candidate.FinalConversion.setAsIdentityConversion(); Candidate.FinalConversion.setFromType(Conversion->getConversionType()); - Candidate.FinalConversion.setToType(ToType); + Candidate.FinalConversion.setAllToTypes(ToType); // Determine the implicit conversion sequence for the implicit // object parameter. diff --git a/lib/Sema/SemaOverload.h b/lib/Sema/SemaOverload.h index 06617d3afa..66ed2259bd 100644 --- a/lib/Sema/SemaOverload.h +++ b/lib/Sema/SemaOverload.h @@ -139,9 +139,10 @@ namespace clang { /// QualType. void *FromTypePtr; - /// ToType - The type that this conversion is converting to. This - /// is an opaque pointer that can be translated into a QualType. - void *ToTypePtr; + /// ToType - The types that this conversion is converting to in + /// each step. This is an opaque pointer that can be translated + /// into a QualType. + void *ToTypePtrs[3]; /// CopyConstructor - The copy constructor that is used to perform /// this conversion, when the conversion is actually just the @@ -151,12 +152,22 @@ namespace clang { CXXConstructorDecl *CopyConstructor; void setFromType(QualType T) { FromTypePtr = T.getAsOpaquePtr(); } - void setToType(QualType T) { ToTypePtr = T.getAsOpaquePtr(); } + void setToType(unsigned Idx, QualType T) { + assert(Idx < 3 && "To type index is out of range"); + ToTypePtrs[Idx] = T.getAsOpaquePtr(); + } + void setAllToTypes(QualType T) { + ToTypePtrs[0] = T.getAsOpaquePtr(); + ToTypePtrs[1] = ToTypePtrs[0]; + ToTypePtrs[2] = ToTypePtrs[0]; + } + QualType getFromType() const { return QualType::getFromOpaquePtr(FromTypePtr); } - QualType getToType() const { - return QualType::getFromOpaquePtr(ToTypePtr); + QualType getToType(unsigned Idx) const { + assert(Idx < 3 && "To type index is out of range"); + return QualType::getFromOpaquePtr(ToTypePtrs[Idx]); } void setAsIdentityConversion(); diff --git a/test/SemaCXX/overload-call-copycon.cpp b/test/SemaCXX/overload-call-copycon.cpp index 472fae26b8..f57484e506 100644 --- a/test/SemaCXX/overload-call-copycon.cpp +++ b/test/SemaCXX/overload-call-copycon.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only %s -Wnon-pod-varargs +// RUN: %clang_cc1 -fsyntax-only -verify %s -Wnon-pod-varargs class X { }; int& copycon(X x); @@ -37,7 +37,6 @@ void test_copycon3(B b, const B bc) { float& f1 = copycon3(bc); // expected-warning {{cannot pass object of non-POD type}} } - class C : public B { }; float& copycon4(A a); diff --git a/test/SemaCXX/overload-call.cpp b/test/SemaCXX/overload-call.cpp index 12dc5daccb..e2a4fd8a0d 100644 --- a/test/SemaCXX/overload-call.cpp +++ b/test/SemaCXX/overload-call.cpp @@ -347,3 +347,15 @@ namespace test3 { foo(*P); // expected-error {{no matching function for call to 'foo'}} } } + +namespace DerivedToBaseVsVoid { + struct A { }; + struct B : A { }; + + float &f(void *); + int &f(const A*); + + void g(B *b) { + int &ir = f(b); + } +} -- 2.40.0