[cfe-commits] r97640 - in /cfe/trunk: include/clang/AST/DeclCXX.h lib/AST/CXXInheritance.cpp test/CXX/class.derived/class.member.lookup/p6.cpp test/SemaCXX/member-name-lookup.cpp

Douglas Gregor dgregor at apple.com
Tue Mar 2 20:38:46 PST 2010


Author: dgregor
Date: Tue Mar  2 22:38:46 2010
New Revision: 97640

URL: http://llvm.org/viewvc/llvm-project?rev=97640&view=rev
Log:
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.

Added:
    cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp   (with props)
Modified:
    cfe/trunk/include/clang/AST/DeclCXX.h
    cfe/trunk/lib/AST/CXXInheritance.cpp
    cfe/trunk/test/SemaCXX/member-name-lookup.cpp

Modified: cfe/trunk/include/clang/AST/DeclCXX.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclCXX.h?rev=97640&r1=97639&r2=97640&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclCXX.h (original)
+++ cfe/trunk/include/clang/AST/DeclCXX.h Tue Mar  2 22:38:46 2010
@@ -751,6 +751,21 @@
   /// 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;
@@ -824,6 +839,18 @@
   /// base class that we are searching for.
   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.

Modified: cfe/trunk/lib/AST/CXXInheritance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/CXXInheritance.cpp?rev=97640&r1=97639&r2=97640&view=diff
==============================================================================
--- cfe/trunk/lib/AST/CXXInheritance.cpp (original)
+++ cfe/trunk/lib/AST/CXXInheritance.cpp Tue Mar  2 22:38:46 2010
@@ -90,6 +90,17 @@
   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 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 @@
            ->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) {

Added: cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp?rev=97640&view=auto
==============================================================================
--- cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp (added)
+++ cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp Tue Mar  2 22:38:46 2010
@@ -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(); }

Propchange: cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp
------------------------------------------------------------------------------
    svn:keywords = Id

Propchange: cfe/trunk/test/CXX/class.derived/class.member.lookup/p6.cpp
------------------------------------------------------------------------------
    svn:mime-type = text/plain

Modified: cfe/trunk/test/SemaCXX/member-name-lookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/member-name-lookup.cpp?rev=97640&r1=97639&r2=97640&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/member-name-lookup.cpp (original)
+++ cfe/trunk/test/SemaCXX/member-name-lookup.cpp Tue Mar  2 22:38:46 2010
@@ -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 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 @@
 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 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





More information about the cfe-commits mailing list