On 12/16/14 23:26, Richard Smith wrote:
On Fri, Dec 12, 2014 at 6:53 AM, Nathan Sidwell <[email protected]
<mailto:[email protected]>> wrote:
This patch addresses bug 6037 http://llvm.org/bugs/show_bug.cgi?id=6037
A request for a class definition to warn about inaccessible direct base classes.
That said, you should be able to do this even more efficiently by collecting a
list of all indirect base classes then checking if any direct base class is on
the list. (If any is, you'll still need the code you already have, in order to
describe the ambiguity.)
Patch adjusted. Builds a std::set of indirect bases of the new class -- if it
has more than 1 direct base. Then checks if a direct base is in the set before
determining the ambiguity of that base.[*] Note that a direct and indirect
base duplicate could exist, but not be ambiguous iff they are both virtual.
Rather than track that data in the std::set, we just check the result of
determining the base path ambiguity in the later loop.
def warn_inaccessible_base_class : Warning<
"direct base %0 is inaccessible due to ambiguity:%1">,
InGroup<DiagGroup<"warning-flag">>;
Done. Added -W[no-]inaccessible-base
A bunch of existing tests trigger. For some of them I added
-Wno-inaccessible-base to the test flags, for some I added the expected warning
check -- depending on whether I judged the warning appropriate for the test
case, or merely coincidental. A new test
tools/clang/test/SemaCXX/accessible-base.cpp is added
built and tested on x86-linux.
ok?
nathan
[*] As an aside, I wonder if completing the std:set with the direct bases and
keeping it with the RecordDecl would speed up base conversion checks -- the base
conversion machinery could use it for a quick 'is this even worth figuring out'
check.
# svn diff src
# svn diff src/projects/compiler-rt
# svn diff src/tools/clang
Index: src/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- src/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td (revision 224818)
+++ src/tools/clang/include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -6341,6 +6341,9 @@ def err_base_class_has_flexible_array_me
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">;
Index: src/tools/clang/lib/Sema/SemaDeclCXX.cpp
===================================================================
--- src/tools/clang/lib/Sema/SemaDeclCXX.cpp (revision 224818)
+++ src/tools/clang/lib/Sema/SemaDeclCXX.cpp (working copy)
@@ -1540,6 +1540,29 @@ Sema::ActOnBaseSpecifier(Decl *classdecl
return true;
}
+typedef std::set<QualType, QualTypeOrdering> 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 iuncomintincoming type is abase, it might not be
+ // a class -- it could be a template parm, for instamce.
+ if (RecordType const *rec = Type->getAs<RecordType>()) {
+ const CXXRecordDecl *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,
@@ -1553,6 +1576,10 @@ bool Sema::AttachBaseSpecifiers(CXXRecor
// 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;
@@ -1580,6 +1607,11 @@ bool Sema::AttachBaseSpecifiers(CXXRecor
// 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() &&
@@ -1600,11 +1632,33 @@ bool Sema::AttachBaseSpecifiers(CXXRecor
// Attach the remaining base class specifiers to the derived class.
Class->setBases(Bases, NumGoodBases);
+
+ for (unsigned idx = 0; idx < NumGoodBases; ++idx) {
- // Delete the remaining (good) base class specifiers, since their
- // data has been copied into the CXXRecordDecl.
- 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.find(CanonicalBase) != IndirectBaseTypes.end()) {
+ 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;
}
Index: src/tools/clang/test/Analysis/dtor.cpp
===================================================================
--- src/tools/clang/test/Analysis/dtor.cpp (revision 224818)
+++ src/tools/clang/test/Analysis/dtor.cpp (working copy)
@@ -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);
Index: src/tools/clang/test/CXX/class.derived/class.virtual/p2.cpp
===================================================================
--- src/tools/clang/test/CXX/class.derived/class.virtual/p2.cpp (revision 224818)
+++ src/tools/clang/test/CXX/class.derived/class.virtual/p2.cpp (working copy)
@@ -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}}
};
Index: src/tools/clang/test/CXX/conv/conv.mem/p4.cpp
===================================================================
--- src/tools/clang/test/CXX/conv/conv.mem/p4.cpp (revision 224818)
+++ src/tools/clang/test/CXX/conv/conv.mem/p4.cpp (working copy)
@@ -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':}}
Index: src/tools/clang/test/CXX/drs/dr0xx.cpp
===================================================================
--- src/tools/clang/test/CXX/drs/dr0xx.cpp (revision 224818)
+++ src/tools/clang/test/CXX/drs/dr0xx.cpp (working copy)
@@ -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}}
Index: src/tools/clang/test/CXX/special/class.copy/implicit-move.cpp
===================================================================
--- src/tools/clang/test/CXX/special/class.copy/implicit-move.cpp (revision 224818)
+++ src/tools/clang/test/CXX/special/class.copy/implicit-move.cpp (working copy)
@@ -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.
Index: src/tools/clang/test/Layout/ms-x86-pack-and-align.cpp
===================================================================
--- src/tools/clang/test/Layout/ms-x86-pack-and-align.cpp (revision 224818)
+++ src/tools/clang/test/Layout/ms-x86-pack-and-align.cpp (working copy)
@@ -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, ...);
Index: src/tools/clang/test/SemaCXX/accessible-base.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/accessible-base.cpp (revision 0)
+++ src/tools/clang/test/SemaCXX/accessible-base.cpp (working copy)
@@ -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}}
+{};
Index: src/tools/clang/test/SemaCXX/class-layout.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/class-layout.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/class-layout.cpp (working copy)
@@ -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]
Index: src/tools/clang/test/SemaCXX/constructor-initializer.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/constructor-initializer.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/constructor-initializer.cpp (working copy)
@@ -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;
Index: src/tools/clang/test/SemaCXX/default-assignment-operator.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/default-assignment-operator.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/default-assignment-operator.cpp (working copy)
@@ -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; }
}
Index: src/tools/clang/test/SemaCXX/derived-to-base-ambig.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/derived-to-base-ambig.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/derived-to-base-ambig.cpp (working copy)
@@ -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;
Index: src/tools/clang/test/SemaCXX/empty-class-layout.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/empty-class-layout.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/empty-class-layout.cpp (working copy)
@@ -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]
Index: src/tools/clang/test/SemaCXX/new-array-size-conv.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/new-array-size-conv.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/new-array-size-conv.cpp (working copy)
@@ -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}}
Index: src/tools/clang/test/SemaCXX/references.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/references.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/references.cpp (working copy)
@@ -70,7 +70,7 @@ class Test6 { // expected-warning{{class
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':}}
Index: src/tools/clang/test/SemaCXX/virtual-override.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/virtual-override.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/virtual-override.cpp (working copy)
@@ -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}}
Index: src/tools/clang/test/SemaCXX/warn-reinterpret-base-class.cpp
===================================================================
--- src/tools/clang/test/SemaCXX/warn-reinterpret-base-class.cpp (revision 224818)
+++ src/tools/clang/test/SemaCXX/warn-reinterpret-base-class.cpp (working copy)
@@ -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;
Index: src/tools/clang/test/SemaTemplate/anonymous-union.cpp
===================================================================
--- src/tools/clang/test/SemaTemplate/anonymous-union.cpp (revision 224818)
+++ src/tools/clang/test/SemaTemplate/anonymous-union.cpp (working copy)
@@ -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>
# svn diff src/tools/clang/tools/extra
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
