On Wed, Nov 29, 2023 at 12:23:46PM -0500, Patrick Palka wrote:
> On Wed, 29 Nov 2023, Marek Polacek wrote:
> 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > Now that I'm posting this patch, I think you'll probably want me to use
> > ba_any unconditionally.  That works too; g++.dg/tc1/dr52.C just needs
> > a trivial testsuite tweak:
> >   'C' is not an accessible base of 'X'
> > v.
> >   'C' is an inaccessible base of 'X'
> > We should probably unify those messages...
> > 
> > -- >8 --
> > Given
> > 
> >   struct A { constexpr static int a = 0; };
> >   struct B : A {};
> >   struct C : A {};
> >   struct D : B, C {};
> > 
> > we give the "'A' is an ambiguous base of 'D'" error for
> > 
> >   D{}.A::a;
> > 
> > which seems wrong: 'a' is a static data member so there is only one copy
> > so it can be unambiguously referred to even if there are multiple A
> > objects.  clang++/MSVC/icx agree.
> > 
> >     PR c++/112744
> > 
> > gcc/cp/ChangeLog:
> > 
> >     * typeck.cc (finish_class_member_access_expr): When accessing
> >     a static data member, use ba_any for lookup_base.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> >     * g++.dg/lookup/scoped11.C: New test.
> >     * g++.dg/lookup/scoped12.C: New test.
> >     * g++.dg/lookup/scoped13.C: New test.
> > ---
> >  gcc/cp/typeck.cc                       | 21 ++++++++++++++++++---
> >  gcc/testsuite/g++.dg/lookup/scoped11.C | 14 ++++++++++++++
> >  gcc/testsuite/g++.dg/lookup/scoped12.C | 14 ++++++++++++++
> >  gcc/testsuite/g++.dg/lookup/scoped13.C | 14 ++++++++++++++
> >  4 files changed, 60 insertions(+), 3 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/lookup/scoped11.C
> >  create mode 100644 gcc/testsuite/g++.dg/lookup/scoped12.C
> >  create mode 100644 gcc/testsuite/g++.dg/lookup/scoped13.C
> > 
> > diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> > index e995fb6ddd7..c4de8bb2616 100644
> > --- a/gcc/cp/typeck.cc
> > +++ b/gcc/cp/typeck.cc
> > @@ -3476,7 +3476,7 @@ finish_class_member_access_expr (cp_expr object, tree 
> > name, bool template_p,
> >                        name, scope);
> >               return error_mark_node;
> >             }
> > -         
> > +
> >           if (TREE_SIDE_EFFECTS (object))
> >             val = build2 (COMPOUND_EXPR, TREE_TYPE (val), object, val);
> >           return val;
> > @@ -3493,9 +3493,24 @@ finish_class_member_access_expr (cp_expr object, 
> > tree name, bool template_p,
> >           return error_mark_node;
> >         }
> >  
> > +     /* NAME may refer to a static data member, in which case there is
> > +        one copy of the data member that is shared by all the objects of
> > +        the class.  So NAME can be unambiguously referred to even if
> > +        there are multiple indirect base classes containing NAME.  */
> > +     const base_access ba = [scope, name] ()
> > +       {
> > +         if (identifier_p (name))
> > +           {
> > +             tree m = lookup_member (scope, name, /*protect=*/0,
> > +                                     /*want_type=*/false, tf_none);
> > +             if (!m || VAR_P (m))
> > +               return ba_any;
> 
> I wonder if we want to return ba_check_bit instead of ba_any so that we
> still check access of the selected base?

That would certainly make sense to me.  I didn't do that because
I'd not seen ba_check_bit being used except as part of ba_check,
but that may not mean much.

So either I can tweak the lambda to return ba_check_bit rather
than ba_any or use ba_check_bit unconditionally.  Any opinions on that?

>   struct A { constexpr static int a = 0; };
>   struct D : private A {};
> 
>   void f() {
>     D{}.A::a; // #1 GCC (and Clang) currently rejects
>   }
> 
>   template<class T>
>   void g() {
>     D{}.T::a; // #2 GCC currently rejects, Clang accepts?!
>   }
> 
>   template void g<A>();

Thanks for looking at the patch and the testcase.  I'll add it.

> > +           }
> > +         return ba_check;
> > +       } ();
> > +
> >       /* Find the base of OBJECT_TYPE corresponding to SCOPE.  */
> > -     access_path = lookup_base (object_type, scope, ba_check,
> > -                                NULL, complain);
> > +     access_path = lookup_base (object_type, scope, ba, NULL, complain);
> >       if (access_path == error_mark_node)
> >         return error_mark_node;
> >       if (!access_path)
> > diff --git a/gcc/testsuite/g++.dg/lookup/scoped11.C 
> > b/gcc/testsuite/g++.dg/lookup/scoped11.C
> > new file mode 100644
> > index 00000000000..be743522fce
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/lookup/scoped11.C
> > @@ -0,0 +1,14 @@
> > +// PR c++/112744
> > +// { dg-do compile }
> > +
> > +struct A { const static int a = 0; };
> > +struct B : A {};
> > +struct C : A {};
> > +struct D : B, C {};
> > +
> > +int main()
> > +{
> > +  D d;
> > +  (void) d.a;
> > +  (void) d.A::a;
> > +}
> > diff --git a/gcc/testsuite/g++.dg/lookup/scoped12.C 
> > b/gcc/testsuite/g++.dg/lookup/scoped12.C
> > new file mode 100644
> > index 00000000000..ffa145598fd
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/lookup/scoped12.C
> > @@ -0,0 +1,14 @@
> > +// PR c++/112744
> > +// { dg-do compile }
> > +
> > +class A { const static int a = 0; };
> > +struct B : A {};
> > +struct C : A {};
> > +struct D : B, C {};
> > +
> > +int main()
> > +{
> > +  D d;
> > +  (void) d.a;        // { dg-error "private" }
> > +  (void) d.A::a;  // { dg-error "private" }
> > +}
> > diff --git a/gcc/testsuite/g++.dg/lookup/scoped13.C 
> > b/gcc/testsuite/g++.dg/lookup/scoped13.C
> > new file mode 100644
> > index 00000000000..970e1aa833e
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/lookup/scoped13.C
> > @@ -0,0 +1,14 @@
> > +// PR c++/112744
> > +// { dg-do compile }
> > +
> > +struct A { const static int a = 0; };
> > +struct B : A {};
> > +struct C : A {};
> > +struct D : B, C {};
> > +
> > +int main()
> > +{
> > +  D d;
> > +  (void) d.x;        // { dg-error ".struct D. has no member named .x." }
> > +  (void) d.A::x;  // { dg-error ".struct A. has no member named .x." }
> > +}
> > 
> > base-commit: 3d104d93a7011146b0870ab160613147adb8d9b3
> > -- 
> > 2.42.0
> > 
> > 
> 

Marek

Reply via email to