rsmith created this revision. rsmith added a reviewer: rjmccall. rsmith requested review of this revision. Herald added a project: clang.
Under C++ core issue 39, the rules for class-scope name lookup were reworked so that name lookup no longer concerns itself with whether the names were found in an ambiguous subobject. The checks for an ambiguous subobject are now done as part of conversion of the object argument in a member access (if needed) instead. One other important consequence of the new lookup rule is that it's now OK to find different lookup results in multiple different classes, so long as the same set of entities is found in each case (in any order, perhaps with duplicates, and in the type case, perhaps via unrelated typedef declarations). This patch implements the new lookup rule. This also has some follow-on impact on access checks: it's now OK for name lookup to find different member declarations in different base classes so long as they all resolve to the same set of entities, so we now need to compute what the corresponding found declaration is, and access-check it, for each path independently. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D94987 Files: clang/include/clang/AST/CXXInheritance.h clang/include/clang/Basic/DiagnosticSemaKinds.td clang/include/clang/Sema/DelayedDiagnostic.h clang/include/clang/Sema/Lookup.h clang/lib/Sema/SemaAccess.cpp clang/lib/Sema/SemaLookup.cpp clang/test/CXX/class.derived/class.member.lookup/p6.cpp clang/test/CXX/class.derived/class.member.lookup/p8.cpp clang/test/CXX/class.derived/class.member.lookup/p9.cpp clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp clang/test/CXX/drs/dr0xx.cpp clang/test/CXX/drs/dr1xx.cpp clang/test/CXX/drs/dr3xx.cpp clang/test/CXX/drs/dr4xx.cpp clang/test/CXX/temp/temp.res/temp.local/p3.cpp clang/test/SemaCXX/access.cpp clang/test/SemaCXX/lookup-member.cpp clang/test/SemaCXX/member-name-lookup.cpp clang/test/SemaCXX/microsoft-dtor-lookup.cpp clang/test/SemaCXX/new-delete.cpp clang/test/SemaTemplate/dependent-base-classes.cpp clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp clang/test/SemaTemplate/temp.cpp clang/www/cxx_dr_status.html
Index: clang/www/cxx_dr_status.html =================================================================== --- clang/www/cxx_dr_status.html +++ clang/www/cxx_dr_status.html @@ -272,7 +272,7 @@ <td><a href="https://wg21.link/cwg39">39</a></td> <td>CD1</td> <td>Conflicting ambiguity rules</td> - <td class="none" align="center">No</td> + <td class="unreleased" align="center">Clang 12</td> </tr> <tr id="40"> <td><a href="https://wg21.link/cwg40">40</a></td> @@ -1876,7 +1876,7 @@ <td><a href="https://wg21.link/cwg306">306</a></td> <td>CD1</td> <td>Ambiguity by class name injection</td> - <td class="none" align="center">Duplicate of <a href="#39">39</a></td> + <td class="unreleased" align="center">Duplicate of <a href="#39">39</a></td> </tr> <tr id="307"> <td><a href="https://wg21.link/cwg307">307</a></td> Index: clang/test/SemaTemplate/temp.cpp =================================================================== --- clang/test/SemaTemplate/temp.cpp +++ clang/test/SemaTemplate/temp.cpp @@ -12,7 +12,7 @@ namespace B { template<typename T> struct Base { typedef T t; }; } // expected-note {{member type 'test1::B::Base<int>' found}} template<typename T> struct Derived : A::Base<char>, B::Base<int> { - typename Derived::Base<float>::t x; // expected-error {{found in multiple base classes of different types}} + typename Derived::Base<float>::t x; // expected-error {{found in multiple base classes}} }; class X : A::Base<int> {}; // expected-note 2{{private}} @@ -41,10 +41,10 @@ template<typename ...T> struct X : T... {}; void f() { - X<A, B>::x<int>(); // expected-error {{found in multiple base classes of different types}} - X<A, C>::x<int>(); // expected-error {{found in multiple base classes of different types}} - X<B, C>::x<int>(); // expected-error {{found in multiple base classes of different types}} - X<A, B, C>::x<int>(); // expected-error {{found in multiple base classes of different types}} - X<A, B, D>::x<int>(); // expected-error {{found in multiple base classes of different types}} + X<A, B>::x<int>(); // expected-error {{found in multiple base classes}} + X<A, C>::x<int>(); // expected-error {{found in multiple base classes}} + X<B, C>::x<int>(); // expected-error {{found in multiple base classes}} + X<A, B, C>::x<int>(); // expected-error {{found in multiple base classes}} + X<A, B, D>::x<int>(); // expected-error {{found in multiple base classes}} } } Index: clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp =================================================================== --- clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp +++ clang/test/SemaTemplate/ms-lookup-template-base-classes.cpp @@ -307,7 +307,7 @@ template <typename T> struct A { typedef T NameFromBase; }; // expected-note {{member type 'int' found by ambiguous name lookup}} template <typename T> struct B { struct NameFromBase { T m; }; }; // expected-note {{member type 'two_types_in_base::B<int>::NameFromBase' found by ambiguous name lookup}} template <typename T> struct C : A<T>, B<T> { - NameFromBase m; // expected-error {{member 'NameFromBase' found in multiple base classes of different types}} expected-warning {{use of member 'NameFromBase' found via unqualified lookup into dependent bases of class templates is a Microsoft extension}} + NameFromBase m; // expected-error {{member 'NameFromBase' found in multiple base classes}} expected-warning {{use of member 'NameFromBase' found via unqualified lookup into dependent bases of class templates is a Microsoft extension}} }; static_assert(sizeof(C<int>) != 0, ""); // expected-note {{in instantiation of template class 'two_types_in_base::C<int>' requested here}} } Index: clang/test/SemaTemplate/dependent-base-classes.cpp =================================================================== --- clang/test/SemaTemplate/dependent-base-classes.cpp +++ clang/test/SemaTemplate/dependent-base-classes.cpp @@ -73,7 +73,7 @@ template<typename T> struct Derived : Base1<T>, Base2 { - typedef typename Derived::type type; // expected-error{{member 'type' found in multiple base classes of different types}} + typedef typename Derived::type type; // expected-error{{member 'type' found in multiple base classes}} type *foo(float *fp) { return fp; } }; Index: clang/test/SemaCXX/new-delete.cpp =================================================================== --- clang/test/SemaCXX/new-delete.cpp +++ clang/test/SemaCXX/new-delete.cpp @@ -244,7 +244,7 @@ }; void f(X8 *x8) { - delete x8; // expected-error {{member 'operator delete' found in multiple base classes of different types}} + delete x8; // expected-error {{member 'operator delete' found in multiple base classes}} } class X9 { Index: clang/test/SemaCXX/microsoft-dtor-lookup.cpp =================================================================== --- clang/test/SemaCXX/microsoft-dtor-lookup.cpp +++ clang/test/SemaCXX/microsoft-dtor-lookup.cpp @@ -20,7 +20,7 @@ }; struct VC : A, B { - virtual ~VC(); // expected-error {{member 'operator delete' found in multiple base classes of different types}} + virtual ~VC(); // expected-error {{member 'operator delete' found in multiple base classes with differing lookup results}} }; void f() { Index: clang/test/SemaCXX/member-name-lookup.cpp =================================================================== --- clang/test/SemaCXX/member-name-lookup.cpp +++ clang/test/SemaCXX/member-name-lookup.cpp @@ -1,6 +1,6 @@ // RUN: %clang_cc1 -fsyntax-only -verify %s struct A { - int a; // expected-note 4{{member found by ambiguous name lookup}} + int a; static int b; static int c; // expected-note 2{{member found by ambiguous name lookup}} @@ -9,7 +9,7 @@ typedef int type; static void f(int); - void f(float); // expected-note 2{{member found by ambiguous name lookup}} + void f(float); static void static_f(int); static void static_f(double); @@ -35,11 +35,12 @@ }; void test_lookup(D d) { - d.a; // expected-error{{non-static member 'a' found in multiple base-class subobjects of type 'A':}} + d.a; // expected-error{{ambiguous conversion from derived class 'D' to base class 'A'}} (void)d.b; // okay - d.c; // expected-error{{member 'c' found in multiple base classes of different types}} - d.d; // expected-error{{member 'd' found in multiple base classes of different types}} - d.f(0); // expected-error{{non-static member 'f' found in multiple base-class subobjects of type 'A':}} + d.c; // expected-error{{member 'c' found in multiple base classes with differing lookup results}} + d.d; // expected-error{{member 'd' found in multiple base classes with differing lookup results}} + d.f(0); + d.f(0.0f); // expected-error{{ambiguous conversion from derived class 'D' to base class 'A'}} d.static_f(0); // okay D::E e = D::enumerator; // okay @@ -51,11 +52,12 @@ } void D::test_lookup() { - a; // expected-error{{non-static member 'a' found in multiple base-class subobjects of type 'A':}} + a; // expected-error{{ambiguous conversion from derived class 'D' to base class 'A'}} (void)b; // okay - c; // expected-error{{member 'c' found in multiple base classes of different types}} - d; // expected-error{{member 'd' found in multiple base classes of different types}} - f(0); // expected-error{{non-static member 'f' found in multiple base-class subobjects of type 'A':}} + c; // expected-error{{member 'c' found in multiple base classes with differing lookup results}} + d; // expected-error{{member 'd' found in multiple base classes with differing lookup results}} + f(0); + f(0.0f); // expected-error {{ambiguous conversion from derived class 'D' to base class 'A'}} static_f(0); // okay E e = enumerator; // okay @@ -63,7 +65,7 @@ E2 e2 = enumerator2; // okay - E3 e3; // expected-error{{member 'E3' found in multiple base classes of different types}} + E3 e3; // expected-error{{member 'E3' found in multiple base classes with differing lookup results}} } struct B2 : virtual A { @@ -94,8 +96,9 @@ (void)d2.a; (void)d2.b; (void)d2.c; // okay - d2.d; // expected-error{{member 'd' found in multiple base classes of different types}} + d2.d; // expected-error{{member 'd' found in multiple base classes with differing lookup results}} d2.f(0); // okay + d2.f(0.0f); d2.static_f(0); // okay D2::E e = D2::enumerator; // okay @@ -103,9 +106,9 @@ D2::E2 e2 = D2::enumerator2; // okay - D2::E3 e3; // expected-error{{member 'E3' found in multiple base classes of different types}} + D2::E3 e3; // expected-error{{member 'E3' found in multiple base classes with differing lookup results}} - g.a; // expected-error{{non-static member 'a' found in multiple base-class subobjects of type 'A':}} + g.a; // expected-error{{ambiguous conversion from derived class 'G' to base class 'A'}} g.static_f(0); // okay } @@ -113,8 +116,9 @@ (void)a; (void)b; (void)c; // okay - d; // expected-error{{member 'd' found in multiple base classes of different types}} + d; // expected-error{{member 'd' found in multiple base classes with differing lookup results}} f(0); // okay + f(0.0f); // okay static_f(0); // okay E e = enumerator; // okay @@ -122,11 +126,11 @@ E2 e2 = enumerator2; // okay - E3 e3; // expected-error{{member 'E3' found in multiple base classes of different types}} + E3 e3; // expected-error{{member 'E3' found in multiple base classes with differing lookup results}} } void G::test_virtual_lookup() { - a; // expected-error{{non-static member 'a' found in multiple base-class subobjects of type 'A':}} + a; // expected-error{{ambiguous conversion from derived class 'G' to base class 'A'}} static_f(0); // okay } @@ -144,7 +148,7 @@ }; struct UsesAmbigMemberType : HasMemberType1, HasMemberType2 { - type t; // expected-error{{member 'type' found in multiple base classes of different types}} + type t; // expected-error{{member 'type' found in multiple base classes with differing lookup results}} }; struct X0 { Index: clang/test/SemaCXX/lookup-member.cpp =================================================================== --- clang/test/SemaCXX/lookup-member.cpp +++ clang/test/SemaCXX/lookup-member.cpp @@ -27,13 +27,13 @@ // multiple base classes. struct A { static void f(); }; struct B { static void f(int); }; - struct C : A, B { using A::f; using B::f; }; // expected-note {{found}} + struct C : A, B { using A::f; using B::f; }; // expected-note 2{{found}} struct D : B, A { using B::f; using A::f; }; struct E : C, D {}; void f(E e) { e.f(0); } // But a different declaration set in different base classes does result in ambiguity. - struct X : B, A { using B::f; using A::f; static void f(int, int); }; // expected-note {{found}} + struct X : B, A { using B::f; using A::f; static void f(int, int); }; // expected-note 3{{found}} struct Y : C, X {}; - void g(Y y) { y.f(0); } // expected-error {{found in multiple base classes of different types}} + void g(Y y) { y.f(0); } // expected-error {{found in multiple base classes with differing lookup results}} } Index: clang/test/SemaCXX/access.cpp =================================================================== --- clang/test/SemaCXX/access.cpp +++ clang/test/SemaCXX/access.cpp @@ -216,3 +216,105 @@ } }; } + +namespace MultiplePathsToSameMember { + namespace NonVirtual { + struct A { + static int a; // expected-note {{declared here}} + using T = int; // expected-note {{declared here}} + }; + struct B : private A {}; // expected-note 2{{constrained by private inheritance here}} + struct C : public A { + private: + using A::a; // expected-note {{declared private here}} + using T = int; // expected-note {{declared private here}} + }; + struct D : public A {}; + struct E : public A { + public: + using A::a; + using T = int; + }; + + struct X1 : B, C {}; + struct X2 : C, B {}; + struct X3 : D, C {}; + struct X4 : C, D {}; + struct X5 : B, E {}; + struct X6 : E, B {}; + + int k1 = X1::a; // expected-error {{'a' is a private member of 'MultiplePathsToSameMember::NonVirtual::A'}} + int k2 = X2::a; // expected-error {{'a' is a private member of 'MultiplePathsToSameMember::NonVirtual::C'}} + int k3 = X3::a; + int k4 = X4::a; + int k5 = X5::a; + int k6 = X6::a; + X1::T u1; // expected-error {{'T' is a private member of 'MultiplePathsToSameMember::NonVirtual::A'}} + X2::T u2; // expected-error {{'T' is a private member of 'MultiplePathsToSameMember::NonVirtual::C'}} + X3::T u3; + X4::T u4; + X5::T u5; + X6::T u6; + } + + namespace Virtual { + struct A { + int a; + using T = int; + }; + struct B : private virtual A {}; + struct C : public virtual A { + private: + using A::a; // expected-note 4{{declared private here}} + using T = int; // expected-note 4{{declared private here}} + }; + struct D : public virtual A {}; + struct E : public virtual A { + public: + using A::a; + using T = int; + }; + + struct X1 : B, C {}; + struct X2 : C, B {}; + struct X3 : D, C {}; + struct X4 : C, D {}; + struct X5 : B, E {}; + struct X6 : E, B {}; + + int k1 = X1().a; // expected-error {{'a' is a private member of 'MultiplePathsToSameMember::Virtual::C'}} + int k2 = X2().a; // expected-error {{'a' is a private member of 'MultiplePathsToSameMember::Virtual::C'}} + // Public declarations in vbase A (reached via D) are hidden by private + // declarations in C. + int k3 = X3().a; // expected-error {{'a' is a private member of 'MultiplePathsToSameMember::Virtual::C'}} + int k4 = X4().a; // expected-error {{'a' is a private member of 'MultiplePathsToSameMember::Virtual::C'}} + int k5 = X5().a; + int k6 = X6().a; + X1::T u1; // expected-error {{'T' is a private member of 'MultiplePathsToSameMember::Virtual::C'}} + X2::T u2; // expected-error {{'T' is a private member of 'MultiplePathsToSameMember::Virtual::C'}} + // Public declarations in vbase A (reached via D) are hidden by private + // declarations in C. + X3::T u3; // expected-error {{'T' is a private member of 'MultiplePathsToSameMember::Virtual::C'}} + X4::T u4; // expected-error {{'T' is a private member of 'MultiplePathsToSameMember::Virtual::C'}} + X5::T u5; + X6::T u6; + } + + namespace Unrelated { + struct A { using T = int; }; + class B { using T = int; }; + struct C { using T = int; }; // expected-note 3{{declared here}} + struct D1 : A, B {}; + struct D2 : B, A {}; + struct D3 : private C, B {}; // expected-note {{constrained by}} + struct D4 : B, private C {}; // expected-note {{constrained by}} + struct D5 : B, private C {}; // expected-note {{constrained by}} + + using T = int; + using T = D1::T; + using T = D2::T; + using T = D3::T; // expected-error {{private}} + using T = D4::T; // expected-error {{private}} + using T = D5::T; // expected-error {{private}} + } +} Index: clang/test/CXX/temp/temp.res/temp.local/p3.cpp =================================================================== --- clang/test/CXX/temp/temp.res/temp.local/p3.cpp +++ clang/test/CXX/temp/temp.res/temp.local/p3.cpp @@ -9,13 +9,13 @@ struct X0 { }; template <class T> struct Derived: Base<int>, Base<char> { - typename Derived::Base b; // expected-error{{member 'Base' found in multiple base classes of different types}} + typename Derived::Base b; // expected-error{{member 'Base' found in multiple base classes}} typename Derived::Base<double> d; // OK void g(X0 *t) { t->Derived::Base<T>::f(); t->Base<T>::f(); - t->Base::f(); // expected-error{{member 'Base' found in multiple base classes of different types}} \ + t->Base::f(); // expected-error{{member 'Base' found in multiple base classes}} \ // expected-error{{no member named 'f' in 'X0'}} } }; Index: clang/test/CXX/drs/dr4xx.cpp =================================================================== --- clang/test/CXX/drs/dr4xx.cpp +++ clang/test/CXX/drs/dr4xx.cpp @@ -825,8 +825,10 @@ } namespace dr471 { // dr471: yes - struct A { int n; }; - struct B : private virtual A {}; + // It's unimportant which of the two private paths we show. (Maybe ideally we + // should show both?) + struct A { int n; }; // expected-note 0-1{{here}} + struct B : private virtual A {}; // expected-note 0-1{{constrained by private inheritance}} struct C : protected virtual A {}; struct D : B, C { int f() { return n; } }; struct E : private virtual A { @@ -835,7 +837,7 @@ struct F : E, B { int f() { return n; } }; struct G : virtual A { private: - using A::n; // expected-note {{here}} + using A::n; // expected-note 0-1{{declared private here}} }; struct H : B, G { int f() { return n; } }; // expected-error {{private}} } Index: clang/test/CXX/drs/dr3xx.cpp =================================================================== --- clang/test/CXX/drs/dr3xx.cpp +++ clang/test/CXX/drs/dr3xx.cpp @@ -147,7 +147,7 @@ template<typename T> struct Y { typedef T X; }; // expected-note {{member type 'const dr306::X' found}} template<typename T> struct Z : X, Y<T> {}; Z<X>::X zx; - Z<const X>::X zcx; // expected-error {{member 'X' found in multiple base classes of different types}} + Z<const X>::X zcx; // expected-error {{member 'X' found in multiple base classes with differing lookup results}} } // dr307: na Index: clang/test/CXX/drs/dr1xx.cpp =================================================================== --- clang/test/CXX/drs/dr1xx.cpp +++ clang/test/CXX/drs/dr1xx.cpp @@ -843,7 +843,7 @@ } }; - template<typename T> struct Base {}; // expected-note 2{{found}} + template<typename T> struct Base {}; // expected-note-re 2{{Base<{{int|char}}>' found}} template<typename T> struct Derived : public Base<T> { void f() { typedef typename Derived::template Base<T> A; Index: clang/test/CXX/drs/dr0xx.cpp =================================================================== --- clang/test/CXX/drs/dr0xx.cpp +++ clang/test/CXX/drs/dr0xx.cpp @@ -456,7 +456,7 @@ template X<int> operator+<int>(X<int>, X<int>); } -namespace dr39 { // dr39: no +namespace dr39 { // dr39: 12 namespace example1 { struct A { int &f(int); }; struct B : A { @@ -476,15 +476,14 @@ }; struct B : A, virtual V { using A::x; // expected-note {{found}} - float &x(float); + float &x(float); // expected-note {{found}} using A::y; // expected-note {{found}} - static float &y(float); + static float &y(float); // expected-note {{found}} using V::z; float &z(float); }; 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}} int &z = c.z(0); } @@ -498,23 +497,54 @@ } namespace example4 { - struct A { int n; }; // expected-note {{found}} + struct A { int n; }; struct B : A {}; struct C : A {}; - struct D : B, C { int f() { return n; } }; // expected-error {{found in multiple base-class}} + struct D : B, C { int f() { return n; } }; // expected-error {{ambiguous conversion}} } namespace PR5916 { - // FIXME: This is valid. - struct A { int n; }; // expected-note +{{found}} + struct A { int n; }; struct B : A {}; struct C : A {}; struct D : B, C {}; - int k = sizeof(D::n); // expected-error {{found in multiple base}} expected-error {{unknown type name}} -#if __cplusplus >= 201103L - decltype(D::n) n; // expected-error {{found in multiple base}} + int k = sizeof(D::n); +#if __cplusplus < 201103L + // expected-error@-2 {{invalid use of non-static data member}} +#else + decltype(D::n) n; #endif } + + namespace type { + struct A { + typedef int I; + typedef int J; // expected-note {{found}} + }; + struct B { + typedef type::A A; + typedef int I; + typedef const int J; // expected-note {{found}} + }; + struct C : A, B {}; + C::A ca; + C::I i; + C::J j; // expected-error {{found in multiple base classes}} + } + + namespace resolve_overload_before_checking_subobject_ambiguity { + struct A { + void f(); + static void f(int); + }; + struct B : A {}; + struct C : A {}; + struct D : B, C {}; + void f(D d) { + d.f(0); // OK + d.f(); // expected-error {{ambiguous conversion}} + } + } } // dr40: na Index: clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp =================================================================== --- clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp +++ clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp @@ -286,7 +286,7 @@ template <> struct std::tuple_size<Derived> { static constexpr size_t value = 1; }; template <> struct std::tuple_element<0, Derived> { typedef int type; }; -auto [x] = Derived(); // expected-error{{member 'get' found in multiple base classes of different types}} +auto [x] = Derived(); // expected-error{{member 'get' found in multiple base classes}} struct Base { template<int> int get(); Index: clang/test/CXX/class.derived/class.member.lookup/p9.cpp =================================================================== --- clang/test/CXX/class.derived/class.member.lookup/p9.cpp +++ clang/test/CXX/class.derived/class.member.lookup/p9.cpp @@ -10,19 +10,38 @@ class ClsB : virtual private ClsA { public: using ClsA::f; - using ClsA::g; // expected-note{{member found by ambiguous name lookup}} + using ClsA::g; }; class ClsF : virtual private ClsA { public: using ClsA::f; - using ClsA::g; // expected-note{{member found by ambiguous name lookup}} + using ClsA::g; }; class ClsE : public ClsB, public ClsF { - void test() { + void test() { f(); - g(); // expected-error{{member 'g' found in multiple base classes of different types}} + g(); + } + }; + + struct A { + static void f(); + void g(); + }; + struct B : private A { + using A::f; + using A::g; + }; + struct C : private A { + using A::f; + using A::g; + }; + struct D : B, C { + void test() { + f(); + g(); // expected-error {{ambiguous conversion from derived class}} } }; } Index: clang/test/CXX/class.derived/class.member.lookup/p8.cpp =================================================================== --- clang/test/CXX/class.derived/class.member.lookup/p8.cpp +++ clang/test/CXX/class.derived/class.member.lookup/p8.cpp @@ -32,7 +32,7 @@ template<typename T> struct BaseT { - void Foo(); // expected-note{{found by ambiguous name lookup}} + void Foo(); int Member; }; @@ -50,7 +50,7 @@ Derived2T<T>::Member = 42; this->Derived1T<T>::Foo(); this->Derived2T<T>::Member = 42; - this->Foo(); // expected-error{{non-static member 'Foo' found in multiple base-class subobjects of type 'BaseT<int>'}} + this->Foo(); // expected-error{{ambiguous conversion from derived class 'DerivedT<int>' to base class 'BaseT<int>'}} } template<typename T> Index: clang/test/CXX/class.derived/class.member.lookup/p6.cpp =================================================================== --- clang/test/CXX/class.derived/class.member.lookup/p6.cpp +++ clang/test/CXX/class.derived/class.member.lookup/p6.cpp @@ -28,8 +28,8 @@ void D::glorp() { x++; f(); - y++; // expected-error{{member 'y' found in multiple base classes of different types}} - g(); // expected-error{{member 'g' found in multiple base classes of different types}} + y++; // expected-error{{member 'y' found in multiple base classes with differing lookup results}} + g(); // expected-error{{member 'g' found in multiple base classes with differing lookup results}} } // PR6462 Index: clang/lib/Sema/SemaLookup.cpp =================================================================== --- clang/lib/Sema/SemaLookup.cpp +++ clang/lib/Sema/SemaLookup.cpp @@ -333,11 +333,9 @@ isa<FunctionTemplateDecl>((*begin())->getUnderlyingDecl()))); assert(ResultKind != FoundUnresolvedValue || sanityCheckUnresolved()); assert(ResultKind != Ambiguous || Decls.size() > 1 || - (Decls.size() == 1 && (Ambiguity == AmbiguousBaseSubobjects || - Ambiguity == AmbiguousBaseSubobjectTypes))); + (Decls.size() == 1 && Ambiguity == AmbiguousBaseSubobjectResults)); assert((Paths != nullptr) == (ResultKind == Ambiguous && - (Ambiguity == AmbiguousBaseSubobjectTypes || - Ambiguity == AmbiguousBaseSubobjects))); + Ambiguity == AmbiguousBaseSubobjectResults)); return true; } @@ -551,10 +549,13 @@ if (ExistingI) { // This is not a unique lookup result. Pick one of the results and - // discard the other. + // discard the other. Keep the more-permissive access specifier. + AccessSpecifier AS = + std::min(Decls[I].getAccess(), Decls[*ExistingI].getAccess()); if (isPreferredLookupResult(getSema(), getLookupKind(), Decls[I], Decls[*ExistingI])) Decls[*ExistingI] = Decls[I]; + Decls[*ExistingI].setAccess(AS); Decls[I] = Decls[--N]; continue; } @@ -643,20 +644,12 @@ addDecl(*DI); } -void LookupResult::setAmbiguousBaseSubobjects(CXXBasePaths &P) { +void LookupResult::setAmbiguousBaseSubobjectResults(CXXBasePaths &P) { Paths = new CXXBasePaths; Paths->swap(P); addDeclsFromBasePaths(*Paths); resolveKind(); - setAmbiguous(AmbiguousBaseSubobjects); -} - -void LookupResult::setAmbiguousBaseSubobjectTypes(CXXBasePaths &P) { - Paths = new CXXBasePaths; - Paths->swap(P); - addDeclsFromBasePaths(*Paths); - resolveKind(); - setAmbiguous(AmbiguousBaseSubobjectTypes); + setAmbiguous(AmbiguousBaseSubobjectResults); } void LookupResult::print(raw_ostream &Out) { @@ -2156,7 +2149,7 @@ if (LookupCtx->isFileContext()) return LookupQualifiedNameInUsingDirectives(*this, R, LookupCtx); - // If this isn't a C++ class, we aren't allowed to look into base + // If this isn't a C++ class, or we aren't allowed to look into base // classes, we're done. CXXRecordDecl *LookupRec = dyn_cast<CXXRecordDecl>(LookupCtx); if (!LookupRec || !LookupRec->getDefinition()) @@ -2181,6 +2174,13 @@ } // Perform lookup into our base classes. + // + // C++ [class.member.lookup] describes the process to perform here as if we + // recursively perform lookups in all base classes and merge them, but it's + // equivalent and much simpler to think about it differently: find all the + // base class subobjects that have lookup results that are not shadowed along + // any inheritance path to that subobject. Those lookup results are required + // to all be the same, and are the result of the class-scope lookup. DeclarationName Name = R.getLookupName(); unsigned IDNS = R.getIdentifierNamespace(); @@ -2206,154 +2206,117 @@ R.setNamingClass(LookupRec); - // C++ [class.member.lookup]p2: - // [...] If the resulting set of declarations are not all from - // sub-objects of the same type, or the set has a nonstatic member - // and includes members from distinct sub-objects, there is an - // ambiguity and the program is ill-formed. Otherwise that set is - // the result of the lookup. - QualType SubobjectType; - int SubobjectNumber = 0; - AccessSpecifier SubobjectAccess = AS_none; - - // Check whether the given lookup result contains only static members. - auto HasOnlyStaticMembers = [&](DeclContextLookupResult Result) { - for (NamedDecl *ND : Result) - if (ND->isInIdentifierNamespace(IDNS) && ND->isCXXInstanceMember()) - return false; - return true; - }; - - bool TemplateNameLookup = R.isTemplateNameLookup(); - - // Determine whether two sets of members contain the same members, as - // required by C++ [class.member.lookup]p6. - auto HasSameDeclarations = [&](DeclContextLookupResult A, - DeclContextLookupResult B) { - using Iterator = DeclContextLookupResult::iterator; - using Result = const void *; - - auto Next = [&](Iterator &It, Iterator End) -> Result { - while (It != End) { - NamedDecl *ND = *It++; - if (!ND->isInIdentifierNamespace(IDNS)) - continue; - - // C++ [temp.local]p3: - // A lookup that finds an injected-class-name (10.2) can result in - // an ambiguity in certain cases (for example, if it is found in - // more than one base class). If all of the injected-class-names - // that are found refer to specializations of the same class - // template, and if the name is used as a template-name, the - // reference refers to the class template itself and not a - // specialization thereof, and is not ambiguous. - if (TemplateNameLookup) - if (auto *TD = getAsTemplateNameDecl(ND)) - ND = TD; - - // C++ [class.member.lookup]p3: - // type declarations (including injected-class-names) are replaced by - // the types they designate - if (const TypeDecl *TD = dyn_cast<TypeDecl>(ND->getUnderlyingDecl())) { - QualType T = Context.getTypeDeclType(TD); - return T.getCanonicalType().getAsOpaquePtr(); - } - - return ND->getUnderlyingDecl()->getCanonicalDecl(); - } - return nullptr; - }; - - // We'll often find the declarations are in the same order. Handle this - // case (and the special case of only one declaration) efficiently. - Iterator AIt = A.begin(), BIt = B.begin(), AEnd = A.end(), BEnd = B.end(); - while (true) { - Result AResult = Next(AIt, AEnd); - Result BResult = Next(BIt, BEnd); - if (!AResult && !BResult) - return true; - if (!AResult || !BResult) - return false; - if (AResult != BResult) { - // Found a mismatch; carefully check both lists, accounting for the - // possibility of declarations appearing more than once. - llvm::SmallDenseMap<Result, bool, 32> AResults; - for (; AResult; AResult = Next(AIt, AEnd)) - AResults.insert({AResult, /*FoundInB*/false}); - unsigned Found = 0; - for (; BResult; BResult = Next(BIt, BEnd)) { - auto It = AResults.find(BResult); - if (It == AResults.end()) - return false; - if (!It->second) { - It->second = true; - ++Found; - } - } - return AResults.size() == Found; - } - } - }; - - for (CXXBasePaths::paths_iterator Path = Paths.begin(), PathEnd = Paths.end(); - Path != PathEnd; ++Path) { - const CXXBasePathElement &PathElement = Path->back(); - - // Pick the best (i.e. most permissive i.e. numerically lowest) access - // across all paths. - SubobjectAccess = std::min(SubobjectAccess, Path->Access); - - // Determine whether we're looking at a distinct sub-object or not. - if (SubobjectType.isNull()) { - // This is the first subobject we've looked at. Record its type. - SubobjectType = Context.getCanonicalType(PathElement.Base->getType()); - SubobjectNumber = PathElement.SubobjectNumber; - continue; - } - - if (SubobjectType != - Context.getCanonicalType(PathElement.Base->getType())) { - // We found members of the given name in two subobjects of - // different types. If the declaration sets aren't the same, this - // lookup is ambiguous. - // - // FIXME: The language rule says that this applies irrespective of - // whether the sets contain only static members. - if (HasOnlyStaticMembers(Path->Decls) && - HasSameDeclarations(Paths.begin()->Decls, Path->Decls)) + // Fast-path the common case where we find results along exactly one path. + if (const CXXBasePath *Path = Paths.getUniquePath()) { + for (NamedDecl *ND : Path->Decls) { + if (!ND->isInIdentifierNamespace(R.getIdentifierNamespace())) continue; - R.setAmbiguousBaseSubobjectTypes(Paths); - return true; - } - - // FIXME: This language rule no longer exists. Checking for ambiguous base - // subobjects should be done as part of formation of a class member access - // expression (when converting the object parameter to the member's type). - if (SubobjectNumber != PathElement.SubobjectNumber) { - // We have a different subobject of the same type. - - // C++ [class.member.lookup]p5: - // A static member, a nested type or an enumerator defined in - // a base class T can unambiguously be found even if an object - // has more than one base class subobject of type T. - if (HasOnlyStaticMembers(Path->Decls)) - continue; - - // We have found a nonstatic member name in multiple, distinct - // subobjects. Name lookup is ambiguous. - R.setAmbiguousBaseSubobjects(Paths); - return true; - } - } - - // Lookup in a base class succeeded; return these results. - - for (auto *D : Paths.front().Decls) { - AccessSpecifier AS = CXXRecordDecl::MergeAccess(SubobjectAccess, - D->getAccess()); - if (NamedDecl *ND = R.getAcceptableDecl(D)) + AccessSpecifier AS = + CXXRecordDecl::MergeAccess(Path->Access, ND->getAccess()); R.addDecl(ND, AS); + } + R.resolveKind(); + return true; + } + + // Otherwise we need to compute the corresponding members in all base classes + // in order to check for ambiguity and find the path with maximal access. + + using MemberResult = const void*; + + auto AsMemberResult = [&](NamedDecl *ND) -> MemberResult { + ND = ND->getUnderlyingDecl(); + + // C++ [temp.local]p3: + // A lookup that finds an injected-class-name (10.2) can result in + // an ambiguity in certain cases (for example, if it is found in + // more than one base class). If all of the injected-class-names + // that are found refer to specializations of the same class + // template, and if the name is used as a template-name, the + // reference refers to the class template itself and not a + // specialization thereof, and is not ambiguous. + if (R.isTemplateNameLookup()) + if (auto *TD = getAsTemplateNameDecl(ND)) + ND = TD; + + // C++ [class.member.lookup]p3: + // type declarations (including injected-class-names) are replaced by + // the types they designate + if (const TypeDecl *TD = dyn_cast<TypeDecl>(ND)) { + QualType T = Context.getTypeDeclType(TD); + return T.getCanonicalType().getAsOpaquePtr(); + } + + return ND->getCanonicalDecl(); + }; + + struct MemberInfo { + MemberInfo() : Access(AS_none), FoundInBases(0) {} + unsigned Access : 2; + unsigned FoundInBases : 30; + }; + + llvm::SmallDenseMap<MemberResult, MemberInfo, 16> Members; + + // C++ [class.member.lookup]/6.2: + // if the declaration sets of [any two paths we found] differ, the merge is + // ambiguous: the [result] is a lookup set with an invalid declaration set + // C++ [class.member.lookup]/7: + // If [the result] is an invalid set, the program is ill-formed. + unsigned Bases = 0; + for (const CXXBasePath &Path : Paths) { + unsigned FoundMembers = 0; + for (NamedDecl *ND : Path.Decls) { + if (!ND->isInIdentifierNamespace(IDNS)) + continue; + + MemberResult Res = AsMemberResult(ND); + auto InfoIt = + Bases ? Members.find(Res) : Members.insert({Res, MemberInfo()}).first; + if (InfoIt == Members.end()) { + // A member in this path was not in the first path. The lookup is + // ambiguous. + R.setAmbiguousBaseSubobjectResults(Paths); + return true; + } + + // C++ [class.paths]p1: + // If a name can be reached by several paths through a multiple + // inheritance graph, the access is that of the path that gives the + // most access. + AccessSpecifier Access = + CXXRecordDecl::MergeAccess(Path.Access, ND->getAccess()); + InfoIt->second.Access = std::min<unsigned>(InfoIt->second.Access, Access); + + if (InfoIt->second.FoundInBases == Bases) { + ++FoundMembers; + ++InfoIt->second.FoundInBases; + } + } + + // If this path didn't have all the members from the first path, the lookup + // is ambiguous. + if (FoundMembers != Members.size()) { + R.setAmbiguousBaseSubobjectResults(Paths); + return true; + } + + ++Bases; + } + + // Lookup in a base class succeeded; return these results. + + // Different paths can have different (but equivalent) sets of declarations; + // arbitrarily pick the declarations from the first path, but use the best + // access from any path. + for (NamedDecl *ND : Paths.front().Decls) { + if (!ND->isInIdentifierNamespace(R.getIdentifierNamespace())) + continue; + + MemberInfo Info = Members[AsMemberResult(ND)]; + assert(Info.FoundInBases == Bases && "member not found in all bases"); + R.addDecl(ND, static_cast<AccessSpecifier>(Info.Access)); } R.resolveKind(); return true; @@ -2380,7 +2343,6 @@ if (NNS && NNS->getKind() == NestedNameSpecifier::Super) return LookupInSuper(R, NNS->getAsRecordDecl()); else - return LookupQualifiedName(R, LookupCtx); } @@ -2488,43 +2450,32 @@ SourceRange LookupRange = Result.getContextRange(); switch (Result.getAmbiguityKind()) { - case LookupResult::AmbiguousBaseSubobjects: { - CXXBasePaths *Paths = Result.getBasePaths(); - QualType SubobjectType = Paths->front().back().Base->getType(); - Diag(NameLoc, diag::err_ambiguous_member_multiple_subobjects) - << Name << SubobjectType << getAmbiguousPathsDisplayString(*Paths) - << LookupRange; - - DeclContext::lookup_iterator Found = Paths->front().Decls.begin(); - while (isa<CXXMethodDecl>(*Found) && - cast<CXXMethodDecl>(*Found)->isStatic()) - ++Found; - - Diag((*Found)->getLocation(), diag::note_ambiguous_member_found); - break; - } - - case LookupResult::AmbiguousBaseSubobjectTypes: { - Diag(NameLoc, diag::err_ambiguous_member_multiple_subobject_types) - << Name << LookupRange; + case LookupResult::AmbiguousBaseSubobjectResults: { + Diag(NameLoc, + diag::err_ambiguous_member_different_results_in_multiple_subobjects) + << Name << LookupRange; CXXBasePaths *Paths = Result.getBasePaths(); - std::set<const NamedDecl *> DeclsPrinted; + std::set<const CXXRecordDecl *> BasesPrinted; for (CXXBasePaths::paths_iterator Path = Paths->begin(), PathEnd = Paths->end(); Path != PathEnd; ++Path) { - const NamedDecl *D = Path->Decls.front(); - if (!D->isInIdentifierNamespace(Result.getIdentifierNamespace())) + QualType Base = Path->back().Base->getType(); + if (!BasesPrinted.insert(Base->getAsCXXRecordDecl()->getCanonicalDecl()) + .second) continue; - if (DeclsPrinted.insert(D).second) { - if (const auto *TD = dyn_cast<TypedefNameDecl>(D->getUnderlyingDecl())) + for (const NamedDecl *D : Path->Decls) { + if (!D->isInIdentifierNamespace(Result.getIdentifierNamespace())) + continue; + if (const auto *TD = + dyn_cast<TypedefNameDecl>(D->getUnderlyingDecl())) Diag(D->getLocation(), diag::note_ambiguous_member_type_found) - << TD->getUnderlyingType(); + << Base << TD->getUnderlyingType(); else if (const auto *TD = dyn_cast<TypeDecl>(D->getUnderlyingDecl())) Diag(D->getLocation(), diag::note_ambiguous_member_type_found) - << Context.getTypeDeclType(TD); + << Base << Context.getTypeDeclType(TD); else - Diag(D->getLocation(), diag::note_ambiguous_member_found); + Diag(D->getLocation(), diag::note_ambiguous_member_found) << Base; } } break; Index: clang/lib/Sema/SemaAccess.cpp =================================================================== --- clang/lib/Sema/SemaAccess.cpp +++ clang/lib/Sema/SemaAccess.cpp @@ -231,6 +231,13 @@ return namingClass->getCanonicalDecl(); } + /// Update the target to a specific, most-accessible declaration of the + /// entity. + void resolveTarget(NamedDecl *ND) { + setTargetDecl(ND); + DeclaringClass = FindDeclaringClass(ND); + } + private: void initialize() { HasInstanceContext = (isMemberAccess() && @@ -866,6 +873,32 @@ llvm_unreachable("impossible friendship kind"); } +static bool declaresSameType(ASTContext &Ctx, NamedDecl *D1, NamedDecl *D2) { + auto *T1 = dyn_cast<TypeDecl>(D1); + auto *T2 = dyn_cast<TypeDecl>(D2); + return T1 && T2 && + Ctx.hasSameType(Ctx.getTypeDeclType(T1), Ctx.getTypeDeclType(T2)); +} + +/// Find the most accessible redeclaration of the target in RD. +static NamedDecl * +FindBestCorrespondingMember(Sema &S, const CXXRecordDecl *RD, + const AccessTarget &Target) { + NamedDecl *Best = nullptr; + for (NamedDecl *Corresponding : + RD->lookup(Target.getTargetDecl()->getDeclName())) { + // Note that declaresSameEntity doesn't correctly determine whether + // two type declarations declare the same type entity. + if ((!Best || Best->getAccess() > Corresponding->getAccess()) && + (declaresSameEntity(Corresponding->getUnderlyingDecl(), + Target.getTargetDecl()->getUnderlyingDecl()) || + declaresSameType(S.Context, Corresponding->getUnderlyingDecl(), + Target.getTargetDecl()->getUnderlyingDecl()))) + Best = Corresponding; + } + return Best; +} + /// Finds the best path from the naming class to the declaring class, /// taking friend declarations into account. /// @@ -919,27 +952,49 @@ /// /// B is an accessible base of N at R iff ACAB(1) = public. /// -/// \param FinalAccess the access of the "final step", or AS_public if -/// there is no final step. +/// Note that we don't actually know which member was named here, because class +/// scope name lookup can find different sets of declarations of the same +/// entities in different classes. So we also redo the name lookup for the +/// member in each class in case a using-declaration or typedef changed its +/// access along some path. +/// +/// \param Target the access target, updated to the corresponding member found +/// along the best path. /// \return null if friendship is dependent static CXXBasePath *FindBestPath(Sema &S, const EffectiveContext &EC, AccessTarget &Target, - AccessSpecifier FinalAccess, CXXBasePaths &Paths) { // Derive the paths to the desired base. const CXXRecordDecl *Derived = Target.getNamingClass(); - const CXXRecordDecl *Base = Target.getDeclaringClass(); - // FIXME: fail correctly when there are dependent paths. - bool isDerived = Derived->isDerivedFrom(const_cast<CXXRecordDecl*>(Base), - Paths); - assert(isDerived && "derived class not actually derived from base"); - (void) isDerived; + if (Target.isMemberAccess()) { + // Redo class-scope lookup to find the set of possible access paths. + auto DeclaresCorrespondingMember = [&](const CXXBaseSpecifier *Specifier, + CXXBasePath &Path) { + // Because name lookup didn't encounter an ambiguity, we know that this + // lookup will stop at exactly the classes where non-shadowed + // declarations of the found entity were found. + if (NamedDecl *BestMember = FindBestCorrespondingMember( + S, Specifier->getType()->getAsCXXRecordDecl(), Target)) { + Path.BestDecl = BestMember; + return true; + } + return false; + }; + Derived->lookupInBases(DeclaresCorrespondingMember, Paths); + } else { + const CXXRecordDecl *Base = Target.getDeclaringClass(); + bool isDerived = Derived->isDerivedFrom(const_cast<CXXRecordDecl*>(Base), + Paths); + assert(isDerived && "derived class not actually derived from base"); + (void) isDerived; + } + + assert(Paths.begin() != Paths.end() && "no paths?"); CXXBasePath *BestPath = nullptr; - - assert(FinalAccess != AS_none && "forbidden access after declaring class"); + NamedDecl *BestTarget = nullptr; bool AnyDependent = false; @@ -947,28 +1002,54 @@ for (CXXBasePaths::paths_iterator PI = Paths.begin(), PE = Paths.end(); PI != PE; ++PI) { AccessTarget::SavedInstanceContext _ = Target.saveInstanceContext(); + AccessSpecifier PathAccess; + + if (Target.isMemberAccess()) { + // Determine if the declaration at the end of this path is accessible + // from EC when named in the corresponding base class. + const CXXRecordDecl *DeclaringClass = + PI->back().Base->getType()->getAsCXXRecordDecl(); + PathAccess = PI->BestDecl->getAccess(); + switch (HasAccess(S, EC, DeclaringClass, PathAccess, Target)) { + case AR_accessible: + PathAccess = AS_public; + // Future tests are not against members and so do not have + // instance context. + Target.suppressInstanceContext(); + break; + case AR_inaccessible: + break; + case AR_dependent: + AnyDependent = true; + continue; + } + } else { + PathAccess = AS_public; + } // Walk through the path backwards. - AccessSpecifier PathAccess = FinalAccess; CXXBasePath::iterator I = PI->end(), E = PI->begin(); while (I != E) { --I; - assert(PathAccess != AS_none); - // If the declaration is a private member of a base class, there // is no level of friendship in derived classes that can make it // accessible. - if (PathAccess == AS_private) { + if (PathAccess == AS_private) PathAccess = AS_none; - break; - } const CXXRecordDecl *NC = I->Class->getCanonicalDecl(); AccessSpecifier BaseAccess = I->Base->getAccessSpecifier(); PathAccess = std::max(PathAccess, BaseAccess); + if (PathAccess == AS_none) { + if (Target.isMemberAccess()) + continue; + else + break; + } + switch (HasAccess(S, EC, NC, PathAccess, Target)) { case AR_inaccessible: break; case AR_accessible: @@ -984,15 +1065,19 @@ } } - // Note that we modify the path's Access field to the - // friend-modified access. + // Note that we modify the path's Access field to the friend-modified, + // target-specific access to the best corresponding member. if (BestPath == nullptr || PathAccess < BestPath->Access) { BestPath = &*PI; BestPath->Access = PathAccess; + BestTarget = PI->BestDecl; // Short-circuit if we found a public path. - if (BestPath->Access == AS_public) + if (BestPath->Access == AS_public) { + if (Target.isMemberAccess()) + Target.resolveTarget(BestTarget); return BestPath; + } } Next: ; @@ -1006,6 +1091,8 @@ if (AnyDependent) return nullptr; + if (Target.isMemberAccess() && BestPath) + Target.resolveTarget(BestTarget); return BestPath; } @@ -1155,45 +1242,44 @@ // This basically repeats the main algorithm but keeps some more // information. - // The natural access so far. + // We might find the declaration in the naming class itself. In that case, + // we don't need to consider paths. + if (NamedDecl *BestMember = FindBestCorrespondingMember( + S, entity.getEffectiveNamingClass(), entity)) { + entity.resolveTarget(BestMember); + return diagnoseBadDirectAccess(S, EC, entity); + } + + CXXBasePaths paths; + CXXBasePath &path = *FindBestPath(S, EC, entity, paths); + assert(path.Access != AS_public); + + // Re-derive the access to the declaration, if any. AccessSpecifier accessSoFar = AS_public; - - // Check whether we have special rights to the declaring class. if (entity.isMemberAccess()) { - NamedDecl *D = entity.getTargetDecl(); - accessSoFar = D->getAccess(); - const CXXRecordDecl *declaringClass = entity.getDeclaringClass(); + accessSoFar = path.BestDecl->getAccess(); + assert(accessSoFar != AS_none); + const CXXRecordDecl *declaringClass = + path.back().Base->getType()->getAsCXXRecordDecl(); switch (HasAccess(S, EC, declaringClass, accessSoFar, entity)) { - // If the declaration is accessible when named in its declaring - // class, then we must be constrained by the path. + case AR_inaccessible: break; case AR_accessible: accessSoFar = AS_public; entity.suppressInstanceContext(); break; - - case AR_inaccessible: - if (accessSoFar == AS_private || - declaringClass == entity.getEffectiveNamingClass()) - return diagnoseBadDirectAccess(S, EC, entity); - break; - case AR_dependent: llvm_unreachable("cannot diagnose dependent access"); } + if (accessSoFar == AS_private) + return diagnoseBadDirectAccess(S, EC, entity); } - CXXBasePaths paths; - CXXBasePath &path = *FindBestPath(S, EC, entity, accessSoFar, paths); - assert(path.Access != AS_public); - CXXBasePath::iterator i = path.end(), e = path.begin(); CXXBasePath::iterator constrainingBase = i; while (i != e) { --i; - assert(accessSoFar != AS_none && accessSoFar != AS_private); - // Is the entity accessible when named in the deriving class, as // modified by the base specifier? const CXXRecordDecl *derivingClass = i->Class->getCanonicalDecl(); @@ -1212,7 +1298,7 @@ case AR_accessible: accessSoFar = AS_public; entity.suppressInstanceContext(); - constrainingBase = nullptr; + constrainingBase = path.end(); break; case AR_dependent: llvm_unreachable("cannot diagnose dependent access"); @@ -1340,44 +1426,24 @@ } } - AccessTarget::SavedInstanceContext _ = Entity.saveInstanceContext(); - - // We lower member accesses to base accesses by pretending that the - // member is a base class of its declaring class. - AccessSpecifier FinalAccess; - + // If the entity is declared in the naming class, we don't need to consider + // paths. if (Entity.isMemberAccess()) { - // Determine if the declaration is accessible from EC when named - // in its declaring class. - NamedDecl *Target = Entity.getTargetDecl(); - const CXXRecordDecl *DeclaringClass = Entity.getDeclaringClass(); - - FinalAccess = Target->getAccess(); - switch (HasAccess(S, EC, DeclaringClass, FinalAccess, Entity)) { - case AR_accessible: - // Target is accessible at EC when named in its declaring class. - // We can now hill-climb and simply check whether the declaring - // class is accessible as a base of the naming class. This is - // equivalent to checking the access of a notional public - // member with no instance context. - FinalAccess = AS_public; - Entity.suppressInstanceContext(); - break; - case AR_inaccessible: break; - case AR_dependent: return AR_dependent; // see above + if (NamedDecl *BestMember = + FindBestCorrespondingMember(S, NamingClass, Entity)) { + Entity.resolveTarget(BestMember); + // Don't repeat checks we already performed when checking unprivileged + // access above. + if (UnprivilegedAccess != AS_none && + BestMember->getAccess() >= UnprivilegedAccess) + return AR_inaccessible; + return HasAccess(S, EC, NamingClass, BestMember->getAccess(), Entity); } - - if (DeclaringClass == NamingClass) - return (FinalAccess == AS_public ? AR_accessible : AR_inaccessible); - } else { - FinalAccess = AS_public; } - assert(Entity.getDeclaringClass() != NamingClass); - - // Append the declaration's access if applicable. + // Find the most-accessible path to the entity. CXXBasePaths Paths; - CXXBasePath *Path = FindBestPath(S, EC, Entity, FinalAccess, Paths); + CXXBasePath *Path = FindBestPath(S, EC, Entity, Paths); if (!Path) return AR_dependent; Index: clang/include/clang/Sema/Lookup.h =================================================================== --- clang/include/clang/Sema/Lookup.h +++ clang/include/clang/Sema/Lookup.h @@ -76,7 +76,8 @@ enum AmbiguityKind { /// Name lookup results in an ambiguity because multiple /// entities that meet the lookup criteria were found in - /// subobjects of different types. For example: + /// subobjects of different types, and the lookup results + /// were not the same in every type. For example: /// @code /// struct A { void f(int); } /// struct B { void f(double); } @@ -86,21 +87,7 @@ /// // types. overload resolution is not performed. /// } /// @endcode - AmbiguousBaseSubobjectTypes, - - /// Name lookup results in an ambiguity because multiple - /// nonstatic entities that meet the lookup criteria were found - /// in different subobjects of the same type. For example: - /// @code - /// struct A { int x; }; - /// struct B : A { }; - /// struct C : A { }; - /// struct D : B, C { }; - /// int test(D d) { - /// return d.x; // error: 'x' is found in two A subobjects (of B and C) - /// } - /// @endcode - AmbiguousBaseSubobjects, + AmbiguousBaseSubobjectResults, /// Name lookup results in an ambiguity because multiple definitions /// of entity that meet the lookup criteria were found in different @@ -531,17 +518,11 @@ return getResultKind() == Found && isa<TagDecl>(getFoundDecl()); } - /// Make these results show that the name was found in - /// base classes of different types. + /// Make these results show that different lookup results were found in + /// multiple base classes. /// /// The given paths object is copied and invalidated. - void setAmbiguousBaseSubobjectTypes(CXXBasePaths &P); - - /// Make these results show that the name was found in - /// distinct base classes of the same type. - /// - /// The given paths object is copied and invalidated. - void setAmbiguousBaseSubobjects(CXXBasePaths &P); + void setAmbiguousBaseSubobjectResults(CXXBasePaths &P); /// Make these results show that the name was found in /// different contexts and a tag decl was hidden by an ordinary Index: clang/include/clang/Sema/DelayedDiagnostic.h =================================================================== --- clang/include/clang/Sema/DelayedDiagnostic.h +++ clang/include/clang/Sema/DelayedDiagnostic.h @@ -61,8 +61,9 @@ MemberNonce _, CXXRecordDecl *NamingClass, DeclAccessPair FoundDecl, QualType BaseObjectType) : Access(FoundDecl.getAccess()), IsMember(true), - Target(FoundDecl.getDecl()), NamingClass(NamingClass), - BaseObjectType(BaseObjectType), Diag(0, Allocator) {} + Target(FoundDecl.getDecl()->getUnderlyingDecl()), + NamingClass(NamingClass), BaseObjectType(BaseObjectType), + Diag(0, Allocator) {} AccessedEntity(PartialDiagnostic::DiagStorageAllocator &Allocator, BaseNonce _, CXXRecordDecl *BaseClass, @@ -79,6 +80,7 @@ // These apply to member decls... NamedDecl *getTargetDecl() const { return Target; } CXXRecordDecl *getNamingClass() const { return NamingClass; } + void setTargetDecl(NamedDecl *ND) { Target = ND; } // ...and these apply to hierarchy conversions. CXXRecordDecl *getBaseClass() const { Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -8692,13 +8692,12 @@ "of class %1 via virtual base %2 is not allowed">; // C++ member name lookup -def err_ambiguous_member_multiple_subobjects : Error< - "non-static member %0 found in multiple base-class subobjects of type %1:%2">; -def err_ambiguous_member_multiple_subobject_types : Error< - "member %0 found in multiple base classes of different types">; -def note_ambiguous_member_found : Note<"member found by ambiguous name lookup">; +def err_ambiguous_member_different_results_in_multiple_subobjects : Error< + "member %0 found in multiple base classes with differing lookup results">; +def note_ambiguous_member_found : Note< + "member found by ambiguous name lookup in %0">; def note_ambiguous_member_type_found : Note< - "member type %0 found by ambiguous name lookup">; + "member type %1 found by ambiguous name lookup in %0">; def err_ambiguous_reference : Error<"reference to %0 is ambiguous">; def note_ambiguous_candidate : Note<"candidate found by name lookup is %q0">; def err_ambiguous_tag_hiding : Error<"a type named %0 is hidden by a " Index: clang/include/clang/AST/CXXInheritance.h =================================================================== --- clang/include/clang/AST/CXXInheritance.h +++ clang/include/clang/AST/CXXInheritance.h @@ -74,11 +74,16 @@ /// used to indicate a path which permits no legal access. AccessSpecifier Access = AS_public; - CXXBasePath() = default; + /// Additional data stashed on the base path by its consumers. + union { + /// The set of declarations found inside this base class + /// subobject. Used by name lookup. + DeclContext::lookup_result Decls = {}; - /// The set of declarations found inside this base class - /// subobject. - DeclContext::lookup_result Decls; + /// The most-accessible declaration found inside this base class. + /// Used by access checking. + NamedDecl *BestDecl; + }; void clear() { SmallVectorImpl<CXXBasePathElement>::clear(); @@ -190,6 +195,14 @@ using decl_range = llvm::iterator_range<decl_iterator>; + const CXXBasePath *getUniquePath() const { + if (begin() == end()) + return nullptr; + if (std::next(begin()) == end()) + return &*begin(); + return nullptr; + } + /// Determine whether the path from the most-derived type to the /// given base type is ambiguous (i.e., it refers to multiple subobjects of /// the same base type).
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits