From 5fccd36204f11c8491325038e6ffcc784399098e Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Wed, 3 Mar 2010 23:55:11 +0000 Subject: [PATCH] Reinstate r97674 with a fix for the assertion that was firing in git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@97686 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Sema/Sema.h | 8 +- lib/Sema/SemaExpr.cpp | 132 ++++++++++++++---- lib/Sema/SemaExprCXX.cpp | 2 +- lib/Sema/SemaInit.cpp | 3 +- lib/Sema/SemaOverload.cpp | 41 +++--- lib/Sema/TreeTransform.h | 2 +- .../class.derived/class.member.lookup/p8.cpp | 63 +++++++++ 7 files changed, 203 insertions(+), 48 deletions(-) create mode 100644 test/CXX/class.derived/class.member.lookup/p8.cpp diff --git a/lib/Sema/Sema.h b/lib/Sema/Sema.h index efd04e8edd..a94d07a6ac 100644 --- a/lib/Sema/Sema.h +++ b/lib/Sema/Sema.h @@ -1097,12 +1097,16 @@ public: ImplicitConversionSequence TryObjectArgumentInitialization(QualType FromType, CXXMethodDecl *Method, CXXRecordDecl *ActingContext); - bool PerformObjectArgumentInitialization(Expr *&From, CXXMethodDecl *Method); + bool PerformObjectArgumentInitialization(Expr *&From, + NestedNameSpecifier *Qualifier, + CXXMethodDecl *Method); ImplicitConversionSequence TryContextuallyConvertToBool(Expr *From); bool PerformContextuallyConvertToBool(Expr *&From); - bool PerformObjectMemberConversion(Expr *&From, NamedDecl *Member); + bool PerformObjectMemberConversion(Expr *&From, + NestedNameSpecifier *Qualifier, + NamedDecl *Member); // Members have to be NamespaceDecl* or TranslationUnitDecl*. // TODO: make this is a typesafe union. diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index d9464ad8c0..d8648f2b69 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -635,7 +635,7 @@ Sema::BuildAnonymousStructUnionMemberReference(SourceLocation Loc, MemberType = Context.getQualifiedType(MemberType, NewQuals); MarkDeclarationReferenced(Loc, *FI); - PerformObjectMemberConversion(Result, *FI); + PerformObjectMemberConversion(Result, /*FIXME:Qualifier=*/0, *FI); // FIXME: Might this end up being a qualified name? Result = new (Context) MemberExpr(Result, BaseObjectIsPointer, *FI, OpLoc, MemberType); @@ -1357,29 +1357,111 @@ Sema::LookupInObjCMethod(LookupResult &Lookup, Scope *S, /// \brief Cast member's object to its own class if necessary. bool -Sema::PerformObjectMemberConversion(Expr *&From, NamedDecl *Member) { - if (FieldDecl *FD = dyn_cast(Member)) - if (CXXRecordDecl *RD = - dyn_cast(FD->getDeclContext())) { - QualType DestType = - Context.getCanonicalType(Context.getTypeDeclType(RD)); - if (DestType->isDependentType() || From->getType()->isDependentType()) - return false; - QualType FromRecordType = From->getType(); - QualType DestRecordType = DestType; - if (FromRecordType->getAs()) { - DestType = Context.getPointerType(DestType); - FromRecordType = FromRecordType->getPointeeType(); - } - if (!Context.hasSameUnqualifiedType(FromRecordType, DestRecordType) && - CheckDerivedToBaseConversion(FromRecordType, - DestRecordType, - From->getSourceRange().getBegin(), - From->getSourceRange())) - return true; - ImpCastExprToType(From, DestType, CastExpr::CK_DerivedToBase, - /*isLvalue=*/true); +Sema::PerformObjectMemberConversion(Expr *&From, + NestedNameSpecifier *Qualifier, + NamedDecl *Member) { + CXXRecordDecl *RD = dyn_cast(Member->getDeclContext()); + if (!RD) + return false; + + QualType DestRecordType; + QualType DestType; + QualType FromRecordType; + QualType FromType = From->getType(); + bool PointerConversions = false; + if (isa(Member)) { + DestRecordType = Context.getCanonicalType(Context.getTypeDeclType(RD)); + + if (FromType->getAs()) { + DestType = Context.getPointerType(DestRecordType); + FromRecordType = FromType->getPointeeType(); + PointerConversions = true; + } else { + DestType = DestRecordType; + FromRecordType = FromType; } + } else if (CXXMethodDecl *Method = dyn_cast(Member)) { + if (Method->isStatic()) + return false; + + DestType = Method->getThisType(Context); + DestRecordType = DestType->getPointeeType(); + + if (FromType->getAs()) { + FromRecordType = FromType->getPointeeType(); + PointerConversions = true; + } else { + FromRecordType = FromType; + DestType = DestRecordType; + } + } else { + // No conversion necessary. + return false; + } + + if (DestType->isDependentType() || FromType->isDependentType()) + return false; + + // If the unqualified types are the same, no conversion is necessary. + if (Context.hasSameUnqualifiedType(FromRecordType, DestRecordType)) + return false; + + // C++ [class.member.lookup]p8: + // [...] Ambiguities can often be resolved by qualifying a name with its + // class name. + // + // If the member was a qualified name and the qualified referred to a + // specific base subobject type, we'll cast to that intermediate type + // first and then to the object in which the member is declared. That allows + // one to resolve ambiguities in, e.g., a diamond-shaped hierarchy such as: + // + // class Base { public: int x; }; + // class Derived1 : public Base { }; + // class Derived2 : public Base { }; + // class VeryDerived : public Derived1, public Derived2 { void f(); }; + // + // void VeryDerived::f() { + // x = 17; // error: ambiguous base subobjects + // Derived1::x = 17; // okay, pick the Base subobject of Derived1 + // } + QualType IntermediateRecordType; + QualType IntermediateType; + if (Qualifier) { + if (const RecordType *IntermediateRecord + = Qualifier->getAsType()->getAs()) { + IntermediateRecordType = QualType(IntermediateRecord, 0); + IntermediateType = IntermediateRecordType; + if (PointerConversions) + IntermediateType = Context.getPointerType(IntermediateType); + } + } + + if (!IntermediateType.isNull() && + IsDerivedFrom(FromRecordType, IntermediateRecordType) && + IsDerivedFrom(IntermediateRecordType, DestRecordType)) { + if (CheckDerivedToBaseConversion(FromRecordType, IntermediateRecordType, + From->getSourceRange().getBegin(), + From->getSourceRange()) || + CheckDerivedToBaseConversion(IntermediateRecordType, DestRecordType, + From->getSourceRange().getBegin(), + From->getSourceRange())) + return true; + + ImpCastExprToType(From, IntermediateType, CastExpr::CK_DerivedToBase, + /*isLvalue=*/!PointerConversions); + ImpCastExprToType(From, DestType, CastExpr::CK_DerivedToBase, + /*isLvalue=*/!PointerConversions); + return false; + } + + if (CheckDerivedToBaseConversion(FromRecordType, + DestRecordType, + From->getSourceRange().getBegin(), + From->getSourceRange())) + return true; + + ImpCastExprToType(From, DestType, CastExpr::CK_DerivedToBase, + /*isLvalue=*/true); return false; } @@ -2666,7 +2748,7 @@ Sema::BuildMemberReferenceExpr(ExprArg Base, QualType BaseExprType, } MarkDeclarationReferenced(MemberLoc, FD); - if (PerformObjectMemberConversion(BaseExpr, FD)) + if (PerformObjectMemberConversion(BaseExpr, Qualifier, FD)) return ExprError(); return Owned(BuildMemberExpr(Context, BaseExpr, IsArrow, SS, FD, MemberLoc, MemberType)); @@ -6651,7 +6733,7 @@ Sema::OwningExprResult Sema::ActOnBuiltinOffsetOf(Scope *S, Res = BuildAnonymousStructUnionMemberReference( OC.LocEnd, MemberDecl, Res, OC.LocEnd).takeAs(); } else { - PerformObjectMemberConversion(Res, MemberDecl); + PerformObjectMemberConversion(Res, /*Qualifier=*/0, MemberDecl); // MemberDecl->getType() doesn't get the right qualifiers, but it // doesn't matter here. Res = new (Context) MemberExpr(Res, false, MemberDecl, OC.LocEnd, diff --git a/lib/Sema/SemaExprCXX.cpp b/lib/Sema/SemaExprCXX.cpp index 5f4601961c..309da292e1 100644 --- a/lib/Sema/SemaExprCXX.cpp +++ b/lib/Sema/SemaExprCXX.cpp @@ -2881,7 +2881,7 @@ Sema::OwningExprResult Sema::ActOnPseudoDestructorExpr(Scope *S, ExprArg Base, CXXMemberCallExpr *Sema::BuildCXXMemberCallExpr(Expr *Exp, CXXMethodDecl *Method) { - if (PerformObjectArgumentInitialization(Exp, Method)) + if (PerformObjectArgumentInitialization(Exp, /*Qualifier=*/0, Method)) assert(0 && "Calling BuildCXXMemberCallExpr with invalid call?"); MemberExpr *ME = diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp index 0f8107ac5e..bf9f73c961 100644 --- a/lib/Sema/SemaInit.cpp +++ b/lib/Sema/SemaInit.cpp @@ -3365,7 +3365,8 @@ InitializationSequence::Perform(Sema &S, // FIXME: Should we move this initialization into a separate // derived-to-base conversion? I believe the answer is "no", because // we don't want to turn off access control here for c-style casts. - if (S.PerformObjectArgumentInitialization(CurInitExpr, Conversion)) + if (S.PerformObjectArgumentInitialization(CurInitExpr, /*Qualifier=*/0, + Conversion)) return S.ExprError(); // Do a little dance to make sure that CurInit has the proper diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp index ed0d137d80..ff59fc3a7d 100644 --- a/lib/Sema/SemaOverload.cpp +++ b/lib/Sema/SemaOverload.cpp @@ -1282,8 +1282,8 @@ bool Sema::CheckPointerConversion(Expr *From, QualType ToType, QualType FromPointeeType = FromPtrType->getPointeeType(), ToPointeeType = ToPtrType->getPointeeType(); - if (FromPointeeType->isRecordType() && - ToPointeeType->isRecordType()) { + if (FromPointeeType->isRecordType() && ToPointeeType->isRecordType() && + !Context.hasSameUnqualifiedType(FromPointeeType, ToPointeeType)) { // We must have a derived-to-base conversion. Check an // ambiguous or inaccessible conversion. if (CheckDerivedToBaseConversion(FromPointeeType, ToPointeeType, @@ -2304,7 +2304,9 @@ Sema::TryObjectArgumentInitialization(QualType OrigFromType, /// the implicit object parameter for the given Method with the given /// expression. bool -Sema::PerformObjectArgumentInitialization(Expr *&From, CXXMethodDecl *Method) { +Sema::PerformObjectArgumentInitialization(Expr *&From, + NestedNameSpecifier *Qualifier, + CXXMethodDecl *Method) { QualType FromRecordType, DestType; QualType ImplicitParamRecordType = Method->getThisType(Context)->getAs()->getPointeeType(); @@ -2327,15 +2329,12 @@ Sema::PerformObjectArgumentInitialization(Expr *&From, CXXMethodDecl *Method) { diag::err_implicit_object_parameter_init) << ImplicitParamRecordType << FromRecordType << From->getSourceRange(); - if (ICS.Standard.Second == ICK_Derived_To_Base && - CheckDerivedToBaseConversion(FromRecordType, - ImplicitParamRecordType, - From->getSourceRange().getBegin(), - From->getSourceRange())) - return true; + if (ICS.Standard.Second == ICK_Derived_To_Base) + return PerformObjectMemberConversion(From, Qualifier, Method); - ImpCastExprToType(From, DestType, CastExpr::CK_DerivedToBase, - /*isLvalue=*/true); + if (!Context.hasSameType(From->getType(), DestType)) + ImpCastExprToType(From, DestType, CastExpr::CK_NoOp, + /*isLvalue=*/!From->getType()->getAs()); return false; } @@ -5545,7 +5544,7 @@ Sema::CreateOverloadedUnaryOp(SourceLocation OpLoc, unsigned OpcIn, if (CXXMethodDecl *Method = dyn_cast(FnDecl)) { CheckMemberOperatorAccess(OpLoc, Args[0], Method, Best->getAccess()); - if (PerformObjectArgumentInitialization(Input, Method)) + if (PerformObjectArgumentInitialization(Input, /*Qualifier=*/0, Method)) return ExprError(); } else { // Convert the arguments. @@ -5738,7 +5737,8 @@ Sema::CreateOverloadedBinOp(SourceLocation OpLoc, if (Arg1.isInvalid()) return ExprError(); - if (PerformObjectArgumentInitialization(Args[0], Method)) + if (PerformObjectArgumentInitialization(Args[0], /*Qualifier=*/0, + Method)) return ExprError(); Args[1] = RHS = Arg1.takeAs(); @@ -5904,7 +5904,8 @@ Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc, // Convert the arguments. CXXMethodDecl *Method = cast(FnDecl); - if (PerformObjectArgumentInitialization(Args[0], Method)) + if (PerformObjectArgumentInitialization(Args[0], /*Qualifier=*/0, + Method)) return ExprError(); // Convert the arguments. @@ -6009,12 +6010,15 @@ Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE, MemberExpr *MemExpr; CXXMethodDecl *Method = 0; + NestedNameSpecifier *Qualifier = 0; if (isa(NakedMemExpr)) { MemExpr = cast(NakedMemExpr); Method = cast(MemExpr->getMemberDecl()); + Qualifier = MemExpr->getQualifier(); } else { UnresolvedMemberExpr *UnresExpr = cast(NakedMemExpr); - + Qualifier = UnresExpr->getQualifier(); + QualType ObjectType = UnresExpr->getBaseType(); // Add overload candidates @@ -6113,7 +6117,7 @@ Sema::BuildCallToMemberFunction(Scope *S, Expr *MemExprE, // Convert the object argument (for a non-static member function call). Expr *ObjectArg = MemExpr->getBase(); if (!Method->isStatic() && - PerformObjectArgumentInitialization(ObjectArg, Method)) + PerformObjectArgumentInitialization(ObjectArg, Qualifier, Method)) return ExprError(); MemExpr->setBase(ObjectArg); @@ -6333,7 +6337,8 @@ Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Object, bool IsError = false; // Initialize the implicit object parameter. - IsError |= PerformObjectArgumentInitialization(Object, Method); + IsError |= PerformObjectArgumentInitialization(Object, /*Qualifier=*/0, + Method); TheCall->setArg(0, Object); @@ -6458,7 +6463,7 @@ Sema::BuildOverloadedArrowExpr(Scope *S, ExprArg BaseIn, SourceLocation OpLoc) { // Convert the object parameter. CXXMethodDecl *Method = cast(Best->Function); - if (PerformObjectArgumentInitialization(Base, Method)) + if (PerformObjectArgumentInitialization(Base, /*Qualifier=*/0, Method)) return ExprError(); // No concerns about early exits now. diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index 2f3c482321..24ea62cde1 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -969,7 +969,7 @@ public: assert(!Qualifier && "Can't have an unnamed field with a qualifier!"); Expr *BaseExpr = Base.takeAs(); - if (getSema().PerformObjectMemberConversion(BaseExpr, Member)) + if (getSema().PerformObjectMemberConversion(BaseExpr, Qualifier, Member)) return getSema().ExprError(); MemberExpr *ME = diff --git a/test/CXX/class.derived/class.member.lookup/p8.cpp b/test/CXX/class.derived/class.member.lookup/p8.cpp new file mode 100644 index 0000000000..4d4acc3460 --- /dev/null +++ b/test/CXX/class.derived/class.member.lookup/p8.cpp @@ -0,0 +1,63 @@ +// RUN: %clang_cc1 -fsyntax-only -verify %s + +// FIXME: Access control checks + +namespace PR5820 { + // also + struct Base { + void Foo(); + int Member; + }; + + struct D1 : public Base {}; + struct D2 : public Base {}; + + struct Derived : public D1, public D2 { + void Inner(); + }; + + void Test() { + Derived d; + d.D1::Foo(); + d.D1::Member = 17; + } + + void Derived::Inner() { + D1::Foo(); + D1::Member = 42; + this->D1::Foo(); + this->D1::Member = 42; + } +} + +template +struct BaseT { + void Foo(); // expected-note{{found by ambiguous name lookup}} + int Member; +}; + +template struct Derived1T : BaseT { }; +template struct Derived2T : BaseT { }; + +template +struct DerivedT : public Derived1T, public Derived2T { + void Inner(); +}; + +template +void DerivedT::Inner() { + Derived1T::Foo(); + Derived2T::Member = 42; + this->Derived1T::Foo(); + this->Derived2T::Member = 42; + this->Foo(); // expected-error{{non-static member 'Foo' found in multiple base-class subobjects of type 'BaseT'}} +} + +template +void Test(DerivedT d) { + d.template Derived1T::Foo(); + d.template Derived2T::Member = 17; + d.Inner(); // expected-note{{in instantiation}} +} + +template void Test(DerivedT); -- 2.40.0