Implement name hiding for names found through virtual base subobjects

that are hidden by other derived base subobjects reached along a
lookup path that does *not* pass through the hiding subobject (C++
[class.member.lookup]p6). Fixes PR6462.

llvm-svn: 97640

clang/include/clang/AST/DeclCXX.h

@@ -751,6 +751,21 @@ public:
   /// tangling input and output in \p Paths  
   bool isDerivedFrom(CXXRecordDecl *Base, CXXBasePaths &Paths) const;
 
+  /// \brief Determine whether this class is virtually derived from
+  /// the class \p Base.
+  ///
+  /// This routine only determines whether this class is virtually
+  /// derived from \p Base, but does not account for factors that may
+  /// make a Derived -> Base class ill-formed, such as
+  /// private/protected inheritance or multiple, ambiguous base class
+  /// subobjects.
+  ///
+  /// \param Base the base class we are searching for.
+  ///
+  /// \returns true if this class is virtually derived from Base,
+  /// false otherwise.
+  bool isVirtuallyDerivedFrom(CXXRecordDecl *Base) const;
+
   /// \brief Determine whether this class is provably not derived from
   /// the type \p Base.
   bool isProvablyNotDerivedFrom(const CXXRecordDecl *Base) const;
@@ -825,6 +840,18 @@ public:
   static bool FindBaseClass(const CXXBaseSpecifier *Specifier,
                             CXXBasePath &Path, void *BaseRecord);
 
+  /// \brief Base-class lookup callback that determines whether the
+  /// given base class specifier refers to a specific class
+  /// declaration and describes virtual derivation.
+  ///
+  /// This callback can be used with \c lookupInBases() to determine
+  /// whether a given derived class has is a virtual base class
+  /// subobject of a particular type.  The user data pointer should
+  /// refer to the canonical CXXRecordDecl of the base class that we
+  /// are searching for.
+  static bool FindVirtualBaseClass(const CXXBaseSpecifier *Specifier,
+                                   CXXBasePath &Path, void *BaseRecord);
+  
   /// \brief Base-class lookup callback that determines whether there exists
   /// a tag with the given name.
   ///

clang/lib/AST/CXXInheritance.cpp

@@ -90,6 +90,17 @@ bool CXXRecordDecl::isDerivedFrom(CXXRecordDecl *Base, CXXBasePaths &Paths) cons
   return lookupInBases(&FindBaseClass, Base->getCanonicalDecl(), Paths);
 }
 
+bool CXXRecordDecl::isVirtuallyDerivedFrom(CXXRecordDecl *Base) const {
+  CXXBasePaths Paths(/*FindAmbiguities=*/false, /*RecordPaths=*/false,
+                     /*DetectVirtual=*/false);
+
+  if (getCanonicalDecl() == Base->getCanonicalDecl())
+    return false;
+  
+  Paths.setOrigin(const_cast<CXXRecordDecl*>(this));  
+  return lookupInBases(&FindVirtualBaseClass, Base->getCanonicalDecl(), Paths);
+}
+
 static bool BaseIsNot(const CXXRecordDecl *Base, void *OpaqueTarget) {
   // OpaqueTarget is a CXXRecordDecl*.
   return Base->getCanonicalDecl() != (const CXXRecordDecl*) OpaqueTarget;
@@ -271,7 +282,68 @@ bool CXXBasePaths::lookupInBases(ASTContext &Context,
 bool CXXRecordDecl::lookupInBases(BaseMatchesCallback *BaseMatches,
                                   void *UserData,
                                   CXXBasePaths &Paths) const {
-  return Paths.lookupInBases(getASTContext(), this, BaseMatches, UserData);
+  // If we didn't find anything, report that.
+  if (!Paths.lookupInBases(getASTContext(), this, BaseMatches, UserData))
+    return false;
+
+  // If we're not recording paths or we won't ever find ambiguities,
+  // we're done.
+  if (!Paths.isRecordingPaths() || !Paths.isFindingAmbiguities())
+    return true;
+  
+  // C++ [class.member.lookup]p6:
+  //   When virtual base classes are used, a hidden declaration can be
+  //   reached along a path through the sub-object lattice that does
+  //   not pass through the hiding declaration. This is not an
+  //   ambiguity. The identical use with nonvirtual base classes is an
+  //   ambiguity; in that case there is no unique instance of the name
+  //   that hides all the others.
+  //
+  // FIXME: This is an O(N^2) algorithm, but DPG doesn't see an easy
+  // way to make it any faster.
+  for (CXXBasePaths::paths_iterator P = Paths.begin(), PEnd = Paths.end();
+       P != PEnd; /* increment in loop */) {
+    bool Hidden = false;
+
+    for (CXXBasePath::iterator PE = P->begin(), PEEnd = P->end();
+         PE != PEEnd && !Hidden; ++PE) {
+      if (PE->Base->isVirtual()) {
+        CXXRecordDecl *VBase = 0;
+        if (const RecordType *Record = PE->Base->getType()->getAs<RecordType>())
+          VBase = cast<CXXRecordDecl>(Record->getDecl());
+        if (!VBase)
+          break;
+
+        // The declaration(s) we found along this path were found in a
+        // subobject of a virtual base. Check whether this virtual
+        // base is a subobject of any other path; if so, then the
+        // declaration in this path are hidden by that patch.
+        for (CXXBasePaths::paths_iterator HidingP = Paths.begin(),
+                                       HidingPEnd = Paths.end();
+             HidingP != HidingPEnd;
+             ++HidingP) {
+          CXXRecordDecl *HidingClass = 0;
+          if (const RecordType *Record
+                       = HidingP->back().Base->getType()->getAs<RecordType>())
+            HidingClass = cast<CXXRecordDecl>(Record->getDecl());
+          if (!HidingClass)
+            break;
+
+          if (HidingClass->isVirtuallyDerivedFrom(VBase)) {
+            Hidden = true;
+            break;
+          }
+        }
+      }
+    }
+
+    if (Hidden)
+      P = Paths.Paths.erase(P);
+    else
+      ++P;
+  }
+  
+  return true;
 }
 
 bool CXXRecordDecl::FindBaseClass(const CXXBaseSpecifier *Specifier, 
@@ -283,6 +355,16 @@ bool CXXRecordDecl::FindBaseClass(const CXXBaseSpecifier *Specifier,
            ->getCanonicalDecl() == BaseRecord;
 }
 
+bool CXXRecordDecl::FindVirtualBaseClass(const CXXBaseSpecifier *Specifier, 
+                                         CXXBasePath &Path,
+                                         void *BaseRecord) {
+  assert(((Decl *)BaseRecord)->getCanonicalDecl() == BaseRecord &&
+         "User data for FindBaseClass is not canonical!");
+  return Specifier->isVirtual() &&
+         Specifier->getType()->getAs<RecordType>()->getDecl()
+           ->getCanonicalDecl() == BaseRecord;
+}
+
 bool CXXRecordDecl::FindTagMember(const CXXBaseSpecifier *Specifier, 
                                   CXXBasePath &Path,
                                   void *Name) {

clang/test/CXX/class.derived/class.member.lookup/p6.cpp

@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+class V { 
+public: 
+  int f(); 
+  int x; 
+};
+
+class W { 
+public: 
+  int g(); // expected-note{{member found by ambiguous name lookup}}
+  int y; // expected-note{{member found by ambiguous name lookup}}
+};
+
+class B : public virtual V, public W
+{
+public:
+  int f(); 
+  int x;
+  int g();  // expected-note{{member found by ambiguous name lookup}}
+  int y; // expected-note{{member found by ambiguous name lookup}}
+};
+
+class C : public virtual V, public W { };
+
+class D : public B, public C { void glorp(); };
+
+void D::glorp() {
+  x++;
+  f();
+  y++; // expected-error{{member 'y' found in multiple base classes of different types}}
+  g(); // expected-error{{error: member 'g' found in multiple base classes of different types}}
+}
+
+// PR6462
+struct BaseIO { BaseIO* rdbuf() { return 0; } };
+struct Pcommon : virtual BaseIO { int rdbuf() { return 0; } };
+struct P : virtual BaseIO, Pcommon {};
+
+void f() { P p; p.rdbuf(); }

clang/test/SemaCXX/member-name-lookup.cpp

@@ -2,7 +2,7 @@
 struct A { 
   int a;  // expected-note 4{{member found by ambiguous name lookup}}
   static int b;
-  static int c; // expected-note 4{{member found by ambiguous name lookup}}
+  static int c; // expected-note 2{{member found by ambiguous name lookup}}
 
   enum E { enumerator };
 
@@ -75,7 +75,7 @@ struct B2 : virtual A {
 };
 
 struct C2 : virtual A {
-  int c; // expected-note 2{{member found by ambiguous name lookup}}
+  int c;
   int d; // expected-note 2{{member found by ambiguous name lookup}}
 
   enum E3 { enumerator3_2 }; // expected-note 2{{member found by ambiguous name lookup}}
@@ -93,7 +93,7 @@ struct G : F, D2 {
 void test_virtual_lookup(D2 d2, G g) {
   (void)d2.a;
   (void)d2.b;
-  d2.c; // expected-error{{member 'c' found in multiple base classes of different types}}
+  (void)d2.c; // okay
   d2.d; // expected-error{{member 'd' found in multiple base classes of different types}}
   d2.f(0); // okay
   d2.static_f(0); // okay
@@ -112,7 +112,7 @@ void test_virtual_lookup(D2 d2, G g) {
 void D2::test_virtual_lookup() {
   (void)a;
   (void)b;
-  c; // expected-error{{member 'c' found in multiple base classes of different types}}
+  (void)c; // okay
   d; // expected-error{{member 'd' found in multiple base classes of different types}}
   f(0); // okay
   static_f(0); // okay