PR6037

Warn on inaccessible direct base

llvm-svn: 226423

19 files changed, 107 insertions, 24 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 5d958a62672..9c097028b93 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6357,6 +6357,9 @@ def err_base_class_has_flexible_array_member : Error<
 def err_incomplete_base_class : Error<"base class has incomplete type">;
 def err_duplicate_base_class : Error<
   "base class %0 specified more than once as a direct base class">;
+def warn_inaccessible_base_class : Warning<
+  "direct base %0 is inaccessible due to ambiguity:%1">,
+  InGroup<DiagGroup<"inaccessible-base">>;
 // FIXME: better way to display derivation?  Pass entire thing into diagclient?
 def err_ambiguous_derived_to_base_conv : Error<
   "ambiguous conversion from derived class %0 to base class %1:%2">;
diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 57ace2b802b..5fb10f6da4a 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -1545,6 +1545,31 @@ Sema::ActOnBaseSpecifier(Decl *classdecl, SourceRange SpecifierRange,
   return true;
 }
 
+/// Use small set to collect indirect bases.  As this is only used
+/// locally, there's no need to abstract the small size parameter.
+typedef llvm::SmallPtrSet<QualType, 4> IndirectBaseSet;
+
+/// \brief Recursively add the bases of Type.  Don't add Type itself.
+static void
+NoteIndirectBases(ASTContext &Context, IndirectBaseSet &Set,
+                  const QualType &Type)
+{
+  // Even though the incoming type is a base, it might not be
+  // a class -- it could be a template parm, for instance.
+  if (auto Rec = Type->getAs<RecordType>()) {
+    auto Decl = Rec->getAsCXXRecordDecl();
+
+    // Iterate over its bases.
+    for (const auto &BaseSpec : Decl->bases()) {
+      QualType Base = Context.getCanonicalType(BaseSpec.getType())
+        .getUnqualifiedType();
+      if (Set.insert(Base).second)
+        // If we've not already seen it, recurse.
+        NoteIndirectBases(Context, Set, Base);
+    }
+  }
+}
+
 /// \brief Performs the actual work of attaching the given base class
 /// specifiers to a C++ class.
 bool Sema::AttachBaseSpecifiers(CXXRecordDecl *Class, CXXBaseSpecifier **Bases,
@@ -1558,6 +1583,10 @@ bool Sema::AttachBaseSpecifiers(CXXRecordDecl *Class, CXXBaseSpecifier **Bases,
   // class.
   std::map<QualType, CXXBaseSpecifier*, QualTypeOrdering> KnownBaseTypes;
 
+  // Used to track indirect bases so we can see if a direct base is
+  // ambiguous.
+  IndirectBaseSet IndirectBaseTypes;
+
   // Copy non-redundant base specifiers into permanent storage.
   unsigned NumGoodBases = 0;
   bool Invalid = false;
@@ -1585,6 +1614,11 @@ bool Sema::AttachBaseSpecifiers(CXXRecordDecl *Class, CXXBaseSpecifier **Bases,
       // Okay, add this new base class.
       KnownBase = Bases[idx];
       Bases[NumGoodBases++] = Bases[idx];
+
+      // Note this base's direct & indirect bases, if there could be ambiguity.
+      if (NumBases > 1)
+        NoteIndirectBases(Context, IndirectBaseTypes, NewBaseType);
+      
       if (const RecordType *Record = NewBaseType->getAs<RecordType>()) {
         const CXXRecordDecl *RD = cast<CXXRecordDecl>(Record->getDecl());
         if (Class->isInterface() &&
@@ -1605,11 +1639,32 @@ bool Sema::AttachBaseSpecifiers(CXXRecordDecl *Class, CXXBaseSpecifier **Bases,
 
   // Attach the remaining base class specifiers to the derived class.
   Class->setBases(Bases, NumGoodBases);
+  
+  for (unsigned idx = 0; idx < NumGoodBases; ++idx) {
+    // Check whether this direct base is inaccessible due to ambiguity.
+    QualType BaseType = Bases[idx]->getType();
+    CanQualType CanonicalBase = Context.getCanonicalType(BaseType)
+      .getUnqualifiedType();
+
+    if (IndirectBaseTypes.count(CanonicalBase)) {
+      CXXBasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/true,
+                         /*DetectVirtual=*/true);
+      bool found
+        = Class->isDerivedFrom(CanonicalBase->getAsCXXRecordDecl(), Paths);
+      assert(found);
+
+      if (Paths.isAmbiguous(CanonicalBase))
+        Diag(Bases[idx]->getLocStart (), diag::warn_inaccessible_base_class)
+          << BaseType << getAmbiguousPathsDisplayString(Paths)
+          << Bases[idx]->getSourceRange();
+      else
+        assert(Bases[idx]->isVirtual());
+    }
 
-  // Delete the remaining (good) base class specifiers, since their
-  // data has been copied into the CXXRecordDecl.
-  for (unsigned idx = 0; idx < NumGoodBases; ++idx)
+    // Delete the base class specifier, since its data has been copied
+    // into the CXXRecordDecl.
     Context.Deallocate(Bases[idx]);
+  }
 
   return Invalid;
 }
diff --git a/clang/test/Analysis/dtor.cpp b/clang/test/Analysis/dtor.cpp
index 8d6e30a6c27..bb1e6254fb7 100644
--- a/clang/test/Analysis/dtor.cpp
+++ b/clang/test/Analysis/dtor.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=destructors,cfg-temporary-dtors=true -Wno-null-dereference -verify %s
+// RUN: %clang_cc1 -analyze -analyzer-checker=core,unix.Malloc,debug.ExprInspection -analyzer-config c++-inlining=destructors,cfg-temporary-dtors=true -Wno-null-dereference -Wno-inaccessible-base -verify %s
 
 void clang_analyzer_eval(bool);
 void clang_analyzer_checkInlined(bool);
diff --git a/clang/test/CXX/class.derived/class.virtual/p2.cpp b/clang/test/CXX/class.derived/class.virtual/p2.cpp
index 64d93c88365..9e8d243fc3b 100644
--- a/clang/test/CXX/class.derived/class.virtual/p2.cpp
+++ b/clang/test/CXX/class.derived/class.virtual/p2.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-inaccessible-base %s
 struct A {
   virtual void f() = 0; // expected-note 2{{overridden virtual function}}
 };
diff --git a/clang/test/CXX/conv/conv.mem/p4.cpp b/clang/test/CXX/conv/conv.mem/p4.cpp
index e0748d89236..1618ae125d2 100644
--- a/clang/test/CXX/conv/conv.mem/p4.cpp
+++ b/clang/test/CXX/conv/conv.mem/p4.cpp
@@ -47,7 +47,7 @@ namespace test3 {
 // Can't be virtual even if there's a non-virtual path.
 namespace test4 {
   struct A : Base {};
-  struct Derived : Base, virtual A {};
+  struct Derived : Base, virtual A {}; // expected-warning  {{direct base 'Base' is inaccessible due to ambiguity:\n    struct test4::Derived -> struct Base\n    struct test4::Derived -> struct test4::A -> struct Base}}
   void test() {
     int (Derived::*d) = data_ptr; // expected-error {{ambiguous conversion from pointer to member of base class 'Base' to pointer to member of derived class 'test4::Derived':}}
     int (Derived::*m)() = method_ptr; // expected-error {{ambiguous conversion from pointer to member of base class 'Base' to pointer to member of derived class 'test4::Derived':}}
diff --git a/clang/test/CXX/drs/dr0xx.cpp b/clang/test/CXX/drs/dr0xx.cpp
index 5b34cc3380c..8a4334f8754 100644
--- a/clang/test/CXX/drs/dr0xx.cpp
+++ b/clang/test/CXX/drs/dr0xx.cpp
@@ -425,7 +425,7 @@ namespace dr39 { // dr39: no
       using V::z;
       float &z(float);
     };
-    struct C : A, B, virtual V {} c;
+    struct C : A, B, virtual V {} c; // expected-warning {{direct base 'dr39::example2::A' is inaccessible due to ambiguity:\n    struct dr39::example2::C -> struct dr39::example2::A\n    struct dr39::example2::C -> struct dr39::example2::B -> struct dr39::example2::A}}
     int &x = c.x(0); // expected-error {{found in multiple base classes}}
     // FIXME: This is valid, because we find the same static data member either way.
     int &y = c.y(0); // expected-error {{found in multiple base classes}}
diff --git a/clang/test/CXX/special/class.copy/implicit-move.cpp b/clang/test/CXX/special/class.copy/implicit-move.cpp
index a10d139fe3c..588778c400b 100644
--- a/clang/test/CXX/special/class.copy/implicit-move.cpp
+++ b/clang/test/CXX/special/class.copy/implicit-move.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base %s
 
 // Tests for implicit (non-)declaration of move constructor and
 // assignment: p9, p11, p20, p23.
diff --git a/clang/test/Layout/ms-x86-pack-and-align.cpp b/clang/test/Layout/ms-x86-pack-and-align.cpp
index 9783233d66a..d766a7d25d7 100644
--- a/clang/test/Layout/ms-x86-pack-and-align.cpp
+++ b/clang/test/Layout/ms-x86-pack-and-align.cpp
@@ -1,6 +1,6 @@
-// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>&1 \
+// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple i686-pc-win32 -fdump-record-layouts -fsyntax-only -Wno-inaccessible-base %s 2>&1 \
 // RUN:            | FileCheck %s
-// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only %s 2>/dev/null \
+// RUN: %clang_cc1 -fno-rtti -emit-llvm-only -triple x86_64-pc-win32 -fdump-record-layouts -fsyntax-only -Wno-inaccessible-base %s 2>/dev/null \
 // RUN:            | FileCheck %s -check-prefix CHECK-X64
 
 extern "C" int printf(const char *fmt, ...);
diff --git a/clang/test/SemaCXX/accessible-base.cpp b/clang/test/SemaCXX/accessible-base.cpp
new file mode 100644
index 00000000000..6bf06ce1260
--- /dev/null
+++ b/clang/test/SemaCXX/accessible-base.cpp
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+struct A {
+  int a;
+};
+
+struct X1 : virtual A 
+{};
+
+struct Y1 : X1, virtual A
+{};
+
+struct Y2 : X1, A // expected-warning{{direct base 'A' is inaccessible due to ambiguity:\n    struct Y2 -> struct X1 -> struct A\n    struct Y2 -> struct A}}
+{};
+
+struct X2 : A 
+{};
+
+struct Z1 : X2, virtual A // expected-warning{{direct base 'A' is inaccessible due to ambiguity:\n    struct Z1 -> struct X2 -> struct A\n    struct Z1 -> struct A}}
+{};
+
+struct Z2 : X2, A // expected-warning{{direct base 'A' is inaccessible due to ambiguity:\n    struct Z2 -> struct X2 -> struct A\n    struct Z2 -> struct A}}
+{};
diff --git a/clang/test/SemaCXX/class-layout.cpp b/clang/test/SemaCXX/class-layout.cpp
index 64c8ceba995..96552de1cbe 100644
--- a/clang/test/SemaCXX/class-layout.cpp
+++ b/clang/test/SemaCXX/class-layout.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++98 -Wno-inaccessible-base
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -std=c++11 -Wno-inaccessible-base
 // expected-no-diagnostics
 
 #define SA(n, p) int a##n[(p) ? 1 : -1]
diff --git a/clang/test/SemaCXX/constructor-initializer.cpp b/clang/test/SemaCXX/constructor-initializer.cpp
index 64b503d0865..e3ab610da7c 100644
--- a/clang/test/SemaCXX/constructor-initializer.cpp
+++ b/clang/test/SemaCXX/constructor-initializer.cpp
@@ -26,7 +26,7 @@ public:
   D() : B(), C() { }
 };
 
-class E : public D, public B { 
+class E : public D, public B {  // expected-warning{{direct base 'B' is inaccessible due to ambiguity:\n    class E -> class D -> class C -> class B\n    class E -> class B}}
 public:
   E() : B(), D() { } // expected-error{{base class initializer 'B' names both a direct base class and an inherited virtual base class}}
 };
@@ -204,7 +204,8 @@ struct A {
 };
 
 struct B : virtual A { };
-struct C : A, B { };
+
+  struct C : A, B { }; // expected-warning{{direct base 'Test2::A' is inaccessible due to ambiguity:\n    struct Test2::C -> struct Test2::A\n    struct Test2::C -> struct Test2::B -> struct Test2::A}}
 
 C f(C c) {
   return c;
diff --git a/clang/test/SemaCXX/default-assignment-operator.cpp b/clang/test/SemaCXX/default-assignment-operator.cpp
index 7ef6f777852..f202b61e109 100644
--- a/clang/test/SemaCXX/default-assignment-operator.cpp
+++ b/clang/test/SemaCXX/default-assignment-operator.cpp
@@ -112,7 +112,7 @@ namespace MultiplePaths {
 
   struct X1 : public virtual X0 { };
 
-  struct X2 : X0, X1 { };
+  struct X2 : X0, X1 { }; // expected-warning{{direct base 'MultiplePaths::X0' is inaccessible due to ambiguity:\n    struct MultiplePaths::X2 -> struct MultiplePaths::X0\n    struct MultiplePaths::X2 -> struct MultiplePaths::X1 -> struct MultiplePaths::X0}}
 
   void f(X2 x2) { x2 = x2; }
 }
diff --git a/clang/test/SemaCXX/derived-to-base-ambig.cpp b/clang/test/SemaCXX/derived-to-base-ambig.cpp
index 129ec79182f..93bd3619ccd 100644
--- a/clang/test/SemaCXX/derived-to-base-ambig.cpp
+++ b/clang/test/SemaCXX/derived-to-base-ambig.cpp
@@ -14,8 +14,8 @@ class A2 : public Object2 { };
 class B2 : public virtual A2 { };
 class C2 : virtual public A2 { };
 class D2 : public B2, public C2 { };
-class E2 : public D2, public C2, public virtual A2 { };
-class F2 : public E2, public A2 { };
+class E2 : public D2, public C2, public virtual A2 { }; // expected-warning{{direct base 'C2' is inaccessible due to ambiguity:\n    class E2 -> class D2 -> class C2\n    class E2 -> class C2}}
+class F2 : public E2, public A2 { }; // expected-warning{{direct base 'A2' is inaccessible due to ambiguity:\n    class F2 -> class E2 -> class D2 -> class B2 -> class A2\n    class F2 -> class A2}}
 
 void g(E2* e2, F2* f2) {
   Object2* o2;
diff --git a/clang/test/SemaCXX/empty-class-layout.cpp b/clang/test/SemaCXX/empty-class-layout.cpp
index 3cfc491ef66..e802a0cb786 100644
--- a/clang/test/SemaCXX/empty-class-layout.cpp
+++ b/clang/test/SemaCXX/empty-class-layout.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify 
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown %s -fsyntax-only -verify -Wno-inaccessible-base
 // expected-no-diagnostics
 
 #define SA(n, p) int a##n[(p) ? 1 : -1]
diff --git a/clang/test/SemaCXX/new-array-size-conv.cpp b/clang/test/SemaCXX/new-array-size-conv.cpp
index e8bb67955f9..c987c2820ad 100644
--- a/clang/test/SemaCXX/new-array-size-conv.cpp
+++ b/clang/test/SemaCXX/new-array-size-conv.cpp
@@ -16,7 +16,8 @@ struct ValueEnum {
 struct ValueBoth : ValueInt, ValueEnum { };
 
 struct IndirectValueInt : ValueInt { };
-struct TwoValueInts : ValueInt, IndirectValueInt { };
+struct TwoValueInts : ValueInt, IndirectValueInt { }; // expected-warning{{direct base 'ValueInt' is inaccessible due to ambiguity:\n    struct TwoValueInts -> struct ValueInt\n    struct TwoValueInts -> struct IndirectValueInt -> struct ValueInt}}
+
 
 void test() {
   (void)new int[ValueInt(10)]; // expected-warning{{implicit conversion from array size expression of type 'ValueInt' to integral type 'int' is a C++11 extension}}
diff --git a/clang/test/SemaCXX/references.cpp b/clang/test/SemaCXX/references.cpp
index cfe7dc1f428..b1768b1a3a4 100644
--- a/clang/test/SemaCXX/references.cpp
+++ b/clang/test/SemaCXX/references.cpp
@@ -70,7 +70,7 @@ class Test6 { // expected-warning{{class 'Test6' does not declare any constructo
   int& okay; // expected-note{{reference member 'okay' will never be initialized}}
 };
 
-struct C : B, A { };
+struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due to ambiguity:\n    struct C -> struct B -> struct A\nstruct C -> struct A}}
 
 void test7(C& c) {
   A& a1 = c; // expected-error {{ambiguous conversion from derived class 'C' to base class 'A':}}
diff --git a/clang/test/SemaCXX/virtual-override.cpp b/clang/test/SemaCXX/virtual-override.cpp
index e95acabad2b..ec884f36323 100644
--- a/clang/test/SemaCXX/virtual-override.cpp
+++ b/clang/test/SemaCXX/virtual-override.cpp
@@ -46,7 +46,7 @@ namespace T4 {
 
 struct a { };
 struct a1 : a { };
-struct b : a, a1 { };
+struct b : a, a1 { }; // expected-warning{{direct base 'T4::a' is inaccessible due to ambiguity:\n    struct T4::b -> struct T4::a\n    struct T4::b -> struct T4::a1 -> struct T4::a}}
   
 class A {
   virtual a* f(); // expected-note{{overridden virtual function is here}}
diff --git a/clang/test/SemaCXX/warn-reinterpret-base-class.cpp b/clang/test/SemaCXX/warn-reinterpret-base-class.cpp
index 0231f194ace..d73b4872117 100644
--- a/clang/test/SemaCXX/warn-reinterpret-base-class.cpp
+++ b/clang/test/SemaCXX/warn-reinterpret-base-class.cpp
@@ -20,7 +20,7 @@ class DVA : public virtual A {
 };
 class DDVA : public virtual DA {
 };
-class DMA : public virtual A, public virtual DA {
+class DMA : public virtual A, public virtual DA { //expected-warning{{direct base 'A' is inaccessible due to ambiguity:\n    class DMA -> class A\n    class DMA -> class DA -> class A}}
 };
 
 class B;
diff --git a/clang/test/SemaTemplate/anonymous-union.cpp b/clang/test/SemaTemplate/anonymous-union.cpp
index 97ecd6e60cc..75d53aa5345 100644
--- a/clang/test/SemaTemplate/anonymous-union.cpp
+++ b/clang/test/SemaTemplate/anonymous-union.cpp
@@ -8,7 +8,7 @@ struct T0 {
   };
 };
 template <typename T>
-struct T1 : public T0, public T { 
+struct T1 : public T0, public T { //expected-warning{{direct base 'T0' is inaccessible due to ambiguity:\n    struct T1<struct A> -> struct T0\n    struct T1<struct A> -> struct A -> struct T0}}
   void f0() { 
     m0 = 0; // expected-error{{ambiguous conversion}}
   } 
@@ -16,7 +16,7 @@ struct T1 : public T0, public T {
 
 struct A : public T0 { };
 
-void f1(T1<A> *S) { S->f0(); } // expected-note{{instantiation of member function}}
+void f1(T1<A> *S) { S->f0(); } // expected-note{{instantiation of member function}} expected-note{{in instantiation of template class 'T1<A>' requested here}}
 
 namespace rdar8635664 {
   template<typename T>