[llvm-branch-commits] [clang] 1db60c1 - Remove redundant check for access in the conversion from the naming

Richard Smith via llvm-branch-commits llvm-branch-commits at lists.llvm.org
Sun Nov 29 19:26:35 PST 2020


Author: Richard Smith
Date: 2020-11-29T19:21:59-08:00
New Revision: 1db60c1307ac2e24796047c39d09bf400c22e531

URL: https://github.com/llvm/llvm-project/commit/1db60c1307ac2e24796047c39d09bf400c22e531
DIFF: https://github.com/llvm/llvm-project/commit/1db60c1307ac2e24796047c39d09bf400c22e531.diff

LOG: Remove redundant check for access in the conversion from the naming
class to the declaring class in a class member access.

This check does not appear to be backed by any rule in the standard (the
rule in question was likely removed over the years), and only ever
produces duplicate diagnostics. (It's also not meaningful because there
isn't a unique declaring class after the resolution of core issue 39.)

Added: 
    

Modified: 
    clang/lib/Sema/SemaExpr.cpp
    clang/test/CXX/class.access/class.access.base/p1.cpp
    clang/test/CXX/class.access/class.access.base/p5.cpp
    clang/test/CXX/class.access/class.friend/p1.cpp
    clang/test/CXX/class.access/class.protected/p1.cpp
    clang/test/CXX/class.access/p4.cpp
    clang/test/CXX/drs/dr0xx.cpp
    clang/test/CXX/drs/dr1xx.cpp
    clang/test/CXX/drs/dr2xx.cpp
    clang/test/CXX/drs/dr3xx.cpp
    clang/test/SemaCXX/anonymous-union.cpp
    clang/test/SemaCXX/conversion-function.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d25d91223826..88dab26f2e3b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -2818,21 +2818,24 @@ Sema::LookupInObjCMethod(LookupResult &Lookup, Scope *S,
 
 /// Cast a base object to a member's actual type.
 ///
-/// Logically this happens in three phases:
+/// There are two relevant checks:
 ///
-/// * First we cast from the base type to the naming class.
-///   The naming class is the class into which we were looking
-///   when we found the member;  it's the qualifier type if a
-///   qualifier was provided, and otherwise it's the base type.
+/// C++ [class.access.base]p7:
 ///
-/// * Next we cast from the naming class to the declaring class.
-///   If the member we found was brought into a class's scope by
-///   a using declaration, this is that class;  otherwise it's
-///   the class declaring the member.
+///   If a class member access operator [...] is used to access a non-static
+///   data member or non-static member function, the reference is ill-formed if
+///   the left operand [...] cannot be implicitly converted to a pointer to the
+///   naming class of the right operand.
 ///
-/// * Finally we cast from the declaring class to the "true"
-///   declaring class of the member.  This conversion does not
-///   obey access control.
+/// C++ [expr.ref]p7:
+///
+///   If E2 is a non-static data member or a non-static member function, the
+///   program is ill-formed if the class of which E2 is directly a member is an
+///   ambiguous base (11.8) of the naming class (11.9.3) of E2.
+///
+/// Note that the latter check does not consider access; the access of the
+/// "real" base class is checked as appropriate when checking the access of the
+/// member name.
 ExprResult
 Sema::PerformObjectMemberConversion(Expr *From,
                                     NestedNameSpecifier *Qualifier,
@@ -2956,45 +2959,10 @@ Sema::PerformObjectMemberConversion(Expr *From,
     }
   }
 
-  bool IgnoreAccess = false;
-
-  // If we actually found the member through a using declaration, cast
-  // down to the using declaration's type.
-  //
-  // Pointer equality is fine here because only one declaration of a
-  // class ever has member declarations.
-  if (FoundDecl->getDeclContext() != Member->getDeclContext()) {
-    assert(isa<UsingShadowDecl>(FoundDecl));
-    QualType URecordType = Context.getTypeDeclType(
-                           cast<CXXRecordDecl>(FoundDecl->getDeclContext()));
-
-    // We only need to do this if the naming-class to declaring-class
-    // conversion is non-trivial.
-    if (!Context.hasSameUnqualifiedType(FromRecordType, URecordType)) {
-      assert(IsDerivedFrom(FromLoc, FromRecordType, URecordType));
-      CXXCastPath BasePath;
-      if (CheckDerivedToBaseConversion(FromRecordType, URecordType,
-                                       FromLoc, FromRange, &BasePath))
-        return ExprError();
-
-      QualType UType = URecordType;
-      if (PointerConversions)
-        UType = Context.getPointerType(UType);
-      From = ImpCastExprToType(From, UType, CK_UncheckedDerivedToBase,
-                               VK, &BasePath).get();
-      FromType = UType;
-      FromRecordType = URecordType;
-    }
-
-    // We don't do access control for the conversion from the
-    // declaring class to the true declaring class.
-    IgnoreAccess = true;
-  }
-
   CXXCastPath BasePath;
   if (CheckDerivedToBaseConversion(FromRecordType, DestRecordType,
                                    FromLoc, FromRange, &BasePath,
-                                   IgnoreAccess))
+                                   /*IgnoreAccess=*/true))
     return ExprError();
 
   return ImpCastExprToType(From, DestType, CK_UncheckedDerivedToBase,

diff  --git a/clang/test/CXX/class.access/class.access.base/p1.cpp b/clang/test/CXX/class.access/class.access.base/p1.cpp
index 88e2688406c7..2205b60ba781 100644
--- a/clang/test/CXX/class.access/class.access.base/p1.cpp
+++ b/clang/test/CXX/class.access/class.access.base/p1.cpp
@@ -62,7 +62,7 @@ namespace test1 {
   private: int priv; static int spriv; // expected-note 8 {{declared private here}}
   };
 
-  class Test : protected Base { // expected-note 6 {{declared protected here}} expected-note 8 {{constrained by protected inheritance here}}
+  class Test : protected Base { // expected-note 3 {{declared protected here}} expected-note 8 {{constrained by protected inheritance here}}
     void test() {
       pub++;
       spub++;
@@ -81,11 +81,11 @@ namespace test1 {
   };
 
   void test(Test *t) {
-    t->pub++; // expected-error {{protected member}} expected-error {{protected base class}}
+    t->pub++; // expected-error {{protected member}}
     t->spub++; // expected-error {{protected member}}
-    t->prot++; // expected-error {{protected member}} expected-error {{protected base class}}
+    t->prot++; // expected-error {{protected member}}
     t->sprot++; // expected-error {{protected member}}
-    t->priv++; // expected-error {{private member}} expected-error {{protected base class}}
+    t->priv++; // expected-error {{private member}}
     t->spriv++; // expected-error {{private member}}
 
     // Two possible errors here: one for Base, one for the member
@@ -118,7 +118,7 @@ namespace test2 {
     static int spriv; // expected-note 4 {{declared private here}}
   };
 
-  class Test : private Base { // expected-note 6 {{declared private here}} \
+  class Test : private Base { // expected-note 3 {{declared private here}} \
                               // expected-note 10 {{constrained by private inheritance here}}
     void test() {
       pub++;
@@ -138,11 +138,11 @@ namespace test2 {
   };
 
   void test(Test *t) {
-    t->pub++; // expected-error {{private member}} expected-error {{private base class}}
+    t->pub++; // expected-error {{private member}}
     t->spub++; // expected-error {{private member}}
-    t->prot++; // expected-error {{private member}} expected-error {{private base class}}
+    t->prot++; // expected-error {{private member}}
     t->sprot++; // expected-error {{private member}}
-    t->priv++; // expected-error {{private member}} expected-error {{private base class}}
+    t->priv++; // expected-error {{private member}}
     t->spriv++; // expected-error {{private member}}
 
     t->Base::pub++; // expected-error {{private member}} expected-error {{private base class}}

diff  --git a/clang/test/CXX/class.access/class.access.base/p5.cpp b/clang/test/CXX/class.access/class.access.base/p5.cpp
index 5b08a8619921..acf230f2be92 100644
--- a/clang/test/CXX/class.access/class.access.base/p5.cpp
+++ b/clang/test/CXX/class.access/class.access.base/p5.cpp
@@ -88,10 +88,10 @@ namespace test4 {
   };
 
   class Y : public X { };
-  class Z : protected Y { }; // expected-note 2 {{constrained by protected inheritance here}}
+  class Z : protected Y { }; // expected-note {{constrained by protected inheritance here}}
 
   void X::f(Z *p) {
-    p->field = 0; // expected-error {{cannot cast 'test4::Z' to its protected base class 'test4::X'}} expected-error {{'field' is a private member of 'test4::X'}}
+    p->field = 0; // expected-error {{'field' is a private member of 'test4::X'}}
   }
 }
 

diff  --git a/clang/test/CXX/class.access/class.friend/p1.cpp b/clang/test/CXX/class.access/class.friend/p1.cpp
index b335b0a8c884..aaba34b864b3 100644
--- a/clang/test/CXX/class.access/class.friend/p1.cpp
+++ b/clang/test/CXX/class.access/class.friend/p1.cpp
@@ -128,7 +128,7 @@ namespace test2 {
     X *getPrev() { return Prev; } // expected-note{{member is declared here}}
   };
 
-  class ilist_node : private ilist_half_node { // expected-note {{declared private here}} expected-note {{constrained by private inheritance here}}
+  class ilist_node : private ilist_half_node { // expected-note {{constrained by private inheritance here}}
     friend struct ilist_walker;
     X *Next;
     X *getNext() { return Next; } // expected-note {{declared private here}}
@@ -143,8 +143,7 @@ namespace test2 {
 
   struct ilist_walker_bad {
     static X *getPrev(X *N) { return N->getPrev(); } // \
-    // expected-error {{'getPrev' is a private member of 'test2::ilist_half_node'}} \
-    // expected-error {{cannot cast 'test2::X' to its private base class 'test2::ilist_half_node'}}
+    // expected-error {{'getPrev' is a private member of 'test2::ilist_half_node'}}
 
     static X *getNext(X *N) { return N->getNext(); } // \
     // expected-error {{'getNext' is a private member of 'test2::ilist_node'}}

diff  --git a/clang/test/CXX/class.access/class.protected/p1.cpp b/clang/test/CXX/class.access/class.protected/p1.cpp
index 825447ef1605..6992c4e87d2b 100644
--- a/clang/test/CXX/class.access/class.protected/p1.cpp
+++ b/clang/test/CXX/class.access/class.protected/p1.cpp
@@ -9,9 +9,9 @@ namespace test0 {
   };
   class B : public A {
   };
-  class C : protected A { // expected-note {{declared}}
+  class C : protected A {
   };
-  class D : private B { // expected-note 3 {{constrained}}
+  class D : private B { // expected-note 2 {{constrained}}
   };
 
   void test(A &a) {
@@ -23,11 +23,11 @@ namespace test0 {
     (void) b.sx; // expected-error {{'sx' is a protected member}}
   }
   void test(C &c) {
-    (void) c.x; // expected-error {{'x' is a protected member}} expected-error {{protected base class}}
+    (void) c.x; // expected-error {{'x' is a protected member}}
     (void) c.sx; // expected-error {{'sx' is a protected member}}
   }
   void test(D &d) {
-    (void) d.x; // expected-error {{'x' is a private member}} expected-error {{private base class}}
+    (void) d.x; // expected-error {{'x' is a private member}}
     (void) d.sx; // expected-error {{'sx' is a private member}}
   }
 }
@@ -145,7 +145,7 @@ namespace test4 {
   class B : public A {
     static void test(C&);
   };
-  class C : protected A { // expected-note 4 {{constrained}} expected-note 3 {{declared}}
+  class C : protected A { // expected-note 4 {{constrained}}
     static void test(C&);
   };
   class D : private B {
@@ -153,13 +153,11 @@ namespace test4 {
   };
 
   void A::test(C &c) {
-    (void) c.x;  // expected-error {{'x' is a protected member}} \
-                 // expected-error {{protected base class}}
+    (void) c.x;  // expected-error {{'x' is a protected member}}
     (void) c.sx; // expected-error {{'sx' is a protected member}}
   }
   void B::test(C &c) {
-    (void) c.x;  // expected-error {{'x' is a protected member}} \
-                 // expected-error {{protected base class}}
+    (void) c.x;  // expected-error {{'x' is a protected member}}
     (void) c.sx; // expected-error {{'sx' is a protected member}}
   }
   void C::test(C &c) {
@@ -167,8 +165,7 @@ namespace test4 {
     (void) c.sx;
   }
   void D::test(C &c) {
-    (void) c.x;  // expected-error {{'x' is a protected member}} \
-                 // expected-error {{protected base class}}
+    (void) c.x;  // expected-error {{'x' is a protected member}}
     (void) c.sx; // expected-error {{'sx' is a protected member}}
   }
 }
@@ -186,23 +183,20 @@ namespace test5 {
   class C : protected A {
     static void test(D&);
   };
-  class D : private B { // expected-note 9 {{constrained}}
+  class D : private B { // expected-note 6 {{constrained}}
     static void test(D&);
   };
 
   void A::test(D &d) {
-    (void) d.x;  // expected-error {{'x' is a private member}} \
-                 // expected-error {{cannot cast}}
+    (void) d.x;  // expected-error {{'x' is a private member}}
     (void) d.sx; // expected-error {{'sx' is a private member}}
   }
   void B::test(D &d) {
-    (void) d.x;  // expected-error {{'x' is a private member}} \
-                 // expected-error {{cannot cast}}
+    (void) d.x;  // expected-error {{'x' is a private member}}
     (void) d.sx; // expected-error {{'sx' is a private member}}
   }
   void C::test(D &d) {
-    (void) d.x;  // expected-error {{'x' is a private member}} \
-                 // expected-error {{cannot cast}}
+    (void) d.x;  // expected-error {{'x' is a private member}}
     (void) d.sx; // expected-error {{'sx' is a private member}}
   }
   void D::test(D &d) {
@@ -337,7 +331,7 @@ namespace test9 {
   };
 
   class C : protected B { // expected-note {{declared}} \
-                          // expected-note 9 {{constrained}}
+                          // expected-note 7 {{constrained}}
   };
 
   class D : public A {
@@ -357,14 +351,12 @@ namespace test9 {
     }
 
     static void test(C &c) {
-      c.foo();    // expected-error {{'foo' is a protected member}} \
-                  // expected-error {{cannot cast}}
+      c.foo();    // expected-error {{'foo' is a protected member}}
       c.A::foo(); // expected-error {{'A' is a protected member}} \
                   // expected-error {{cannot cast}}
       c.B::foo(); // expected-error {{'B' is a protected member}} \
                   // expected-error {{cannot cast}}
-      c.C::foo(); // expected-error {{'foo' is a protected member}} \
-                  // expected-error {{cannot cast}}
+      c.C::foo(); // expected-error {{'foo' is a protected member}}
     }
 
     static void test(D &d) {

diff  --git a/clang/test/CXX/class.access/p4.cpp b/clang/test/CXX/class.access/p4.cpp
index adc8bbe29583..5664f726f167 100644
--- a/clang/test/CXX/class.access/p4.cpp
+++ b/clang/test/CXX/class.access/p4.cpp
@@ -270,15 +270,12 @@ namespace test4 {
     operator Public(); // expected-note 2{{member is declared here}}
   };
 
-  class Derived1 : private Base { // expected-note 2 {{declared private here}} \
-                                  // expected-note {{constrained by private inheritance}}
+  class Derived1 : private Base { // expected-note {{constrained by private inheritance}}
     Private test1() { return *this; } // expected-error {{'operator Private' is a private member}}
     Public test2() { return *this; }
   };
-  Private test1(Derived1 &d) { return d; } // expected-error {{'operator Private' is a private member}} \
-                                           // expected-error {{cannot cast 'test4::Derived1' to its private base class}}
-  Public test2(Derived1 &d) { return d; } // expected-error {{cannot cast 'test4::Derived1' to its private base class}} \
-                                          // expected-error {{'operator Public' is a private member}}
+  Private test1(Derived1 &d) { return d; } // expected-error {{'operator Private' is a private member}}
+  Public test2(Derived1 &d) { return d; } // expected-error {{'operator Public' is a private member}}
 
 
   class Derived2 : public Base {
@@ -288,14 +285,12 @@ namespace test4 {
   Private test1(Derived2 &d) { return d; } // expected-error {{'operator Private' is a private member}}
   Public test2(Derived2 &d) { return d; }
 
-  class Derived3 : private Base { // expected-note {{constrained by private inheritance here}} \
-                                  // expected-note {{declared private here}}
+  class Derived3 : private Base { // expected-note {{constrained by private inheritance here}}
   public:
     operator Private();
   };
   Private test1(Derived3 &d) { return d; }
-  Public test2(Derived3 &d) { return d; } // expected-error {{'operator Public' is a private member of 'test4::Base'}} \
-                                          // expected-error {{cannot cast 'test4::Derived3' to its private base class}}
+  Public test2(Derived3 &d) { return d; } // expected-error {{'operator Public' is a private member of 'test4::Base'}}
 
   class Derived4 : public Base {
   public:

diff  --git a/clang/test/CXX/drs/dr0xx.cpp b/clang/test/CXX/drs/dr0xx.cpp
index 8799215ef3b5..01c400184801 100644
--- a/clang/test/CXX/drs/dr0xx.cpp
+++ b/clang/test/CXX/drs/dr0xx.cpp
@@ -118,10 +118,10 @@ namespace dr9 { // dr9: yes
     int m; // expected-note {{here}}
     friend int R1();
   };
-  struct N : protected B { // expected-note 2{{protected}}
+  struct N : protected B { // expected-note {{protected}}
     friend int R2();
   } n;
-  int R1() { return n.m; } // expected-error {{protected base class}} expected-error {{protected member}}
+  int R1() { return n.m; } // expected-error {{protected member}}
   int R2() { return n.m; }
 }
 
@@ -204,10 +204,10 @@ namespace dr16 { // dr16: yes
     void f(); // expected-note {{here}}
     friend class C;
   };
-  class B : A {}; // expected-note 4{{here}}
+  class B : A {}; // expected-note 3{{here}}
   class C : B {
     void g() {
-      f(); // expected-error {{private member}} expected-error {{private base}}
+      f(); // expected-error {{private member}}
       A::f(); // expected-error {{private member}} expected-error {{private base}}
     }
   };

diff  --git a/clang/test/CXX/drs/dr1xx.cpp b/clang/test/CXX/drs/dr1xx.cpp
index 3b16fe0a7708..e9278a8c4e8a 100644
--- a/clang/test/CXX/drs/dr1xx.cpp
+++ b/clang/test/CXX/drs/dr1xx.cpp
@@ -528,7 +528,7 @@ namespace dr142 { // dr142: yes
     void f();
   };
   void DD::f() {
-    mi = 3; // expected-error {{private base class}} expected-error {{private member}}
+    mi = 3; // expected-error {{private member}}
     si = 3; // expected-error {{private member}}
     B b_old; // expected-error {{private member}}
     dr142::B b;

diff  --git a/clang/test/CXX/drs/dr2xx.cpp b/clang/test/CXX/drs/dr2xx.cpp
index 125caad55495..8cf4a19d91e4 100644
--- a/clang/test/CXX/drs/dr2xx.cpp
+++ b/clang/test/CXX/drs/dr2xx.cpp
@@ -917,13 +917,13 @@ namespace dr280 { // dr280: yes
     operator f2*(); // expected-note {{candidate}}
     operator f3*(); // expected-note {{candidate}}
   };
-  struct D : private A, B { // expected-note 2{{here}}
+  struct D : private A, B { // expected-note {{here}}
     operator f2*(); // expected-note {{candidate}}
   } d;
   struct E : C, D {} e;
   void g() {
     d(); // ok, public
-    d(0); // expected-error {{private member of 'dr280::A'}} expected-error {{private base class 'dr280::A'}}
+    d(0); // expected-error {{private member of 'dr280::A'}}
     d(0, 0); // ok, suppressed by member in D
     d(0, 0, 0); // expected-error {{private member of 'dr280::B'}}
     e(); // expected-error {{ambiguous}}

diff  --git a/clang/test/CXX/drs/dr3xx.cpp b/clang/test/CXX/drs/dr3xx.cpp
index 06aa885d236e..5eb6e0512629 100644
--- a/clang/test/CXX/drs/dr3xx.cpp
+++ b/clang/test/CXX/drs/dr3xx.cpp
@@ -1109,9 +1109,9 @@ namespace dr385 { // dr385: yes
   void h(B b) { b.f(); }
 
   struct D { int n; }; // expected-note {{member}}
-  struct E : protected D {}; // expected-note 2{{protected}}
+  struct E : protected D {}; // expected-note {{protected}}
   struct F : E { friend int i(E); };
-  int i(E e) { return e.n; } // expected-error {{protected base}} expected-error {{protected member}}
+  int i(E e) { return e.n; } // expected-error {{protected member}}
 }
 
 namespace dr387 { // dr387: yes

diff  --git a/clang/test/SemaCXX/anonymous-union.cpp b/clang/test/SemaCXX/anonymous-union.cpp
index 9a8843999481..27dd2f0083b8 100644
--- a/clang/test/SemaCXX/anonymous-union.cpp
+++ b/clang/test/SemaCXX/anonymous-union.cpp
@@ -209,9 +209,9 @@ namespace PR8326 {
 
 namespace PR16630 {
   struct A { union { int x; float y; }; }; // expected-note {{member is declared here}}
-  struct B : private A { using A::x; } b; // expected-note 2 {{private}}
+  struct B : private A { using A::x; } b; // expected-note {{private}}
   void foo () {
     b.x = 10;
-    b.y = 0; // expected-error {{cannot cast 'struct B' to its private base class 'PR16630::A'}} expected-error {{'y' is a private member of 'PR16630::A'}}
+    b.y = 0; // expected-error {{'y' is a private member of 'PR16630::A'}}
   }
 }

diff  --git a/clang/test/SemaCXX/conversion-function.cpp b/clang/test/SemaCXX/conversion-function.cpp
index fdd603c98f43..c18948d9b15c 100644
--- a/clang/test/SemaCXX/conversion-function.cpp
+++ b/clang/test/SemaCXX/conversion-function.cpp
@@ -348,7 +348,7 @@ namespace rdar8018274 {
   };
 
   void test2(UeberDerived ud) {
-    int i = ud; // expected-error{{ambiguous conversion from derived class 'rdar8018274::SuperDerived' to base class 'rdar8018274::Base'}}
+    int i = ud; // expected-error{{ambiguous conversion from derived class 'rdar8018274::UeberDerived' to base class 'rdar8018274::Base'}}
   }
 
   struct Base2 {


        


More information about the llvm-branch-commits mailing list