From: Nathan Sidwell Date: Mon, 19 Jan 2015 01:44:02 +0000 (+0000) Subject: PR6037 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b5a0f8e88490495a12db0ed73b606282217f1b42;p=clang PR6037 Warn on inaccessible direct base git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@226423 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 5d958a6267..9c097028b9 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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>; // 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/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 57ace2b802..5fb10f6da4 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/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 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()) { + 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 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()) { const CXXRecordDecl *RD = cast(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/test/Analysis/dtor.cpp b/test/Analysis/dtor.cpp index 8d6e30a6c2..bb1e6254fb 100644 --- a/test/Analysis/dtor.cpp +++ b/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/test/CXX/class.derived/class.virtual/p2.cpp b/test/CXX/class.derived/class.virtual/p2.cpp index 64d93c8836..9e8d243fc3 100644 --- a/test/CXX/class.derived/class.virtual/p2.cpp +++ b/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/test/CXX/conv/conv.mem/p4.cpp b/test/CXX/conv/conv.mem/p4.cpp index e0748d8923..1618ae125d 100644 --- a/test/CXX/conv/conv.mem/p4.cpp +++ b/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/test/CXX/drs/dr0xx.cpp b/test/CXX/drs/dr0xx.cpp index 5b34cc3380..8a4334f875 100644 --- a/test/CXX/drs/dr0xx.cpp +++ b/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/test/CXX/special/class.copy/implicit-move.cpp b/test/CXX/special/class.copy/implicit-move.cpp index a10d139fe3..588778c400 100644 --- a/test/CXX/special/class.copy/implicit-move.cpp +++ b/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/test/Layout/ms-x86-pack-and-align.cpp b/test/Layout/ms-x86-pack-and-align.cpp index 9783233d66..d766a7d25d 100644 --- a/test/Layout/ms-x86-pack-and-align.cpp +++ b/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/test/SemaCXX/accessible-base.cpp b/test/SemaCXX/accessible-base.cpp new file mode 100644 index 0000000000..6bf06ce126 --- /dev/null +++ b/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/test/SemaCXX/class-layout.cpp b/test/SemaCXX/class-layout.cpp index 64c8ceba99..96552de1cb 100644 --- a/test/SemaCXX/class-layout.cpp +++ b/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/test/SemaCXX/constructor-initializer.cpp b/test/SemaCXX/constructor-initializer.cpp index 64b503d086..e3ab610da7 100644 --- a/test/SemaCXX/constructor-initializer.cpp +++ b/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/test/SemaCXX/default-assignment-operator.cpp b/test/SemaCXX/default-assignment-operator.cpp index 7ef6f77785..f202b61e10 100644 --- a/test/SemaCXX/default-assignment-operator.cpp +++ b/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/test/SemaCXX/derived-to-base-ambig.cpp b/test/SemaCXX/derived-to-base-ambig.cpp index 129ec79182..93bd3619cc 100644 --- a/test/SemaCXX/derived-to-base-ambig.cpp +++ b/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/test/SemaCXX/empty-class-layout.cpp b/test/SemaCXX/empty-class-layout.cpp index 3cfc491ef6..e802a0cb78 100644 --- a/test/SemaCXX/empty-class-layout.cpp +++ b/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/test/SemaCXX/new-array-size-conv.cpp b/test/SemaCXX/new-array-size-conv.cpp index e8bb67955f..c987c2820a 100644 --- a/test/SemaCXX/new-array-size-conv.cpp +++ b/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/test/SemaCXX/references.cpp b/test/SemaCXX/references.cpp index cfe7dc1f42..b1768b1a3a 100644 --- a/test/SemaCXX/references.cpp +++ b/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/test/SemaCXX/virtual-override.cpp b/test/SemaCXX/virtual-override.cpp index e95acabad2..ec884f3632 100644 --- a/test/SemaCXX/virtual-override.cpp +++ b/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/test/SemaCXX/warn-reinterpret-base-class.cpp b/test/SemaCXX/warn-reinterpret-base-class.cpp index 0231f194ac..d73b487211 100644 --- a/test/SemaCXX/warn-reinterpret-base-class.cpp +++ b/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/test/SemaTemplate/anonymous-union.cpp b/test/SemaTemplate/anonymous-union.cpp index 97ecd6e60c..75d53aa534 100644 --- a/test/SemaTemplate/anonymous-union.cpp +++ b/test/SemaTemplate/anonymous-union.cpp @@ -8,7 +8,7 @@ struct T0 { }; }; template -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 T0\n struct T1 -> 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 *S) { S->f0(); } // expected-note{{instantiation of member function}} +void f1(T1 *S) { S->f0(); } // expected-note{{instantiation of member function}} expected-note{{in instantiation of template class 'T1' requested here}} namespace rdar8635664 { template