[cfe-commits] r97674 - in /cfe/trunk: lib/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaInit.cpp lib/Sema/SemaOverload.cpp lib/Sema/TreeTransform.h test/CXX/class.derived/class.member.lookup/p8.cpp

Douglas Gregor dgregor at apple.com
Wed Mar 3 14:53:40 PST 2010


Author: dgregor
Date: Wed Mar  3 16:53:40 2010
New Revision: 97674

URL: http://llvm.org/viewvc/llvm-project?rev=97674&view=rev
Log:
Implement disambiguation of base class members via a
nested-name-specifier. For example, this allows member access in
diamond-shaped hierarchies like:

  struct Base {
    void Foo();
    int Member;
  };

  struct D1 : public Base {};
  struct D2 : public Base {};

  struct Derived : public D1, public D2 { }

  void Test(Derived d) {
    d.Member = 17; // error: ambiguous cast from Derived to Base
    d.D1::Member = 17; // error: okay, modify D1's Base's Member
  }

Fixes PR5820 and <rdar://problem/7535045>. Also, eliminate some
redundancy between Sema::PerformObjectMemberConversion() and
Sema::PerformObjectArgumentInitialization() -- the latter now calls
the former.

Added:
    cfe/trunk/test/CXX/class.derived/class.member.lookup/p8.cpp
Modified:
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/lib/Sema/SemaExprCXX.cpp
    cfe/trunk/lib/Sema/SemaInit.cpp
    cfe/trunk/lib/Sema/SemaOverload.cpp
    cfe/trunk/lib/Sema/TreeTransform.h

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=97674&r1=97673&r2=97674&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Wed Mar  3 16:53:40 2010
@@ -1097,12 +1097,16 @@
   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.

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=97674&r1=97673&r2=97674&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Mar  3 16:53:40 2010
@@ -635,7 +635,7 @@
       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 @@
 
 /// \brief Cast member's object to its own class if necessary.
 bool
-Sema::PerformObjectMemberConversion(Expr *&From, NamedDecl *Member) {
-  if (FieldDecl *FD = dyn_cast<FieldDecl>(Member))
-    if (CXXRecordDecl *RD =
-        dyn_cast<CXXRecordDecl>(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<PointerType>()) {
-        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<CXXRecordDecl>(Member->getDeclContext());
+  if (!RD)
+    return false;
+  
+  QualType DestRecordType;
+  QualType DestType;
+  QualType FromRecordType;
+  QualType FromType = From->getType();
+  bool PointerConversions = false;
+  if (isa<FieldDecl>(Member)) {
+    DestRecordType = Context.getCanonicalType(Context.getTypeDeclType(RD));
+    
+    if (FromType->getAs<PointerType>()) {
+      DestType = Context.getPointerType(DestRecordType);
+      FromRecordType = FromType->getPointeeType();
+      PointerConversions = true;
+    } else {
+      DestType = DestRecordType;
+      FromRecordType = FromType;
+    }
+  } else if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(Member)) {
+    if (Method->isStatic())
+      return false;
+    
+    DestType = Method->getThisType(Context);
+    DestRecordType = DestType->getPointeeType();
+    
+    if (FromType->getAs<PointerType>()) {
+      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<RecordType>()) {
+      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 @@
     }
 
     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 @@
         Res = BuildAnonymousStructUnionMemberReference(
             OC.LocEnd, MemberDecl, Res, OC.LocEnd).takeAs<Expr>();
       } 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,

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=97674&r1=97673&r2=97674&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Mar  3 16:53:40 2010
@@ -2881,7 +2881,7 @@
 
 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 = 

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=97674&r1=97673&r2=97674&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Wed Mar  3 16:53:40 2010
@@ -3365,7 +3365,8 @@
         // 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

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=97674&r1=97673&r2=97674&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Mar  3 16:53:40 2010
@@ -2304,7 +2304,9 @@
 /// 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<PointerType>()->getPointeeType();
@@ -2327,15 +2329,12 @@
                 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<PointerType>());
   return false;
 }
 
@@ -5545,7 +5544,7 @@
       if (CXXMethodDecl *Method = dyn_cast<CXXMethodDecl>(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 @@
           if (Arg1.isInvalid())
             return ExprError();
 
-          if (PerformObjectArgumentInitialization(Args[0], Method))
+          if (PerformObjectArgumentInitialization(Args[0], /*Qualifier=*/0, 
+                                                  Method))
             return ExprError();
 
           Args[1] = RHS = Arg1.takeAs<Expr>();
@@ -5904,7 +5904,8 @@
 
         // Convert the arguments.
         CXXMethodDecl *Method = cast<CXXMethodDecl>(FnDecl);
-        if (PerformObjectArgumentInitialization(Args[0], Method))
+        if (PerformObjectArgumentInitialization(Args[0], /*Qualifier=*/0, 
+                                                Method))
           return ExprError();
 
         // Convert the arguments.
@@ -6009,12 +6010,15 @@
   
   MemberExpr *MemExpr;
   CXXMethodDecl *Method = 0;
+  NestedNameSpecifier *Qualifier = 0;
   if (isa<MemberExpr>(NakedMemExpr)) {
     MemExpr = cast<MemberExpr>(NakedMemExpr);
     Method = cast<CXXMethodDecl>(MemExpr->getMemberDecl());
+    Qualifier = MemExpr->getQualifier();
   } else {
     UnresolvedMemberExpr *UnresExpr = cast<UnresolvedMemberExpr>(NakedMemExpr);
-
+    Qualifier = UnresExpr->getQualifier();
+    
     QualType ObjectType = UnresExpr->getBaseType();
 
     // Add overload candidates
@@ -6113,7 +6117,7 @@
   // 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 @@
   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 @@
 
   // Convert the object parameter.
   CXXMethodDecl *Method = cast<CXXMethodDecl>(Best->Function);
-  if (PerformObjectArgumentInitialization(Base, Method))
+  if (PerformObjectArgumentInitialization(Base, /*Qualifier=*/0, Method))
     return ExprError();
 
   // No concerns about early exits now.

Modified: cfe/trunk/lib/Sema/TreeTransform.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/TreeTransform.h?rev=97674&r1=97673&r2=97674&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/TreeTransform.h (original)
+++ cfe/trunk/lib/Sema/TreeTransform.h Wed Mar  3 16:53:40 2010
@@ -969,7 +969,7 @@
       assert(!Qualifier && "Can't have an unnamed field with a qualifier!");
 
       Expr *BaseExpr = Base.takeAs<Expr>();
-      if (getSema().PerformObjectMemberConversion(BaseExpr, Member))
+      if (getSema().PerformObjectMemberConversion(BaseExpr, Qualifier, Member))
         return getSema().ExprError();
 
       MemberExpr *ME =

Added: cfe/trunk/test/CXX/class.derived/class.member.lookup/p8.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.member.lookup/p8.cpp?rev=97674&view=auto
==============================================================================
--- cfe/trunk/test/CXX/class.derived/class.member.lookup/p8.cpp (added)
+++ cfe/trunk/test/CXX/class.derived/class.member.lookup/p8.cpp Wed Mar  3 16:53:40 2010
@@ -0,0 +1,63 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// FIXME: Access control checks
+
+namespace PR5820 {
+  // also <rdar://problem/7535045>
+  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<typename T>
+struct BaseT {
+  void Foo(); // expected-note{{found by ambiguous name lookup}}
+  int Member;
+};
+
+template<typename T> struct Derived1T : BaseT<T> { };
+template<typename T> struct Derived2T : BaseT<T> { };
+
+template<typename T>
+struct DerivedT : public Derived1T<T>, public Derived2T<T> {
+  void Inner();
+};
+
+template<typename T>
+void DerivedT<T>::Inner() {
+  Derived1T<T>::Foo();
+  Derived2T<T>::Member = 42;
+  this->Derived1T<T>::Foo();
+  this->Derived2T<T>::Member = 42;
+  this->Foo(); // expected-error{{non-static member 'Foo' found in multiple base-class subobjects of type 'BaseT<int>'}}
+}
+
+template<typename T>
+void Test(DerivedT<T> d) {
+  d.template Derived1T<T>::Foo();
+  d.template Derived2T<T>::Member = 17;
+  d.Inner(); // expected-note{{in instantiation}}
+}
+
+template void Test(DerivedT<int>);





More information about the cfe-commits mailing list