PR6037

Warn on inaccessible direct base

llvm-svn: 226423

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">;

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();
 
-  // Delete the remaining (good) base class specifiers, since their
-  // data has been copied into the CXXRecordDecl.
-  for (unsigned idx = 0; idx < NumGoodBases; ++idx)
+    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 base class specifier, since its data has been copied
+    // into the CXXRecordDecl.
     Context.Deallocate(Bases[idx]);
+  }
 
   return Invalid;
 }

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);

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}}
 };

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':}}

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}}

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.

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, ...);

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}}
+{};

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]

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;

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; }
 }

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;

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]

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}}

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':}}

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}}

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;

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>