From: Patrick Palka <ppalka@redhat.com>
To: Marek Polacek <polacek@redhat.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>, Jason Merrill <jason@redhat.com>
Subject: Re: [PATCH] c++: wrong ambiguity in accessing static field [PR112744]
Date: Wed, 29 Nov 2023 12:23:46 -0500 (EST) [thread overview]
Message-ID: <fc07bd10-8cc8-9dfa-339f-19d595de29c1@idea> (raw)
In-Reply-To: <20231129154512.428173-1-polacek@redhat.com>
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?
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>();
> + }
> + 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
>
>
next prev parent reply other threads:[~2023-11-29 17:23 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-29 15:45 Marek Polacek
2023-11-29 17:23 ` Patrick Palka [this message]
2023-11-29 17:43 ` Marek Polacek
2023-11-29 20:28 ` Jason Merrill
2023-11-29 22:01 ` [PATCH v2] " Marek Polacek
2023-11-30 16:42 ` Jason Merrill
2023-11-30 21:44 ` Marek Polacek
2023-11-29 18:58 ` [PATCH] " Jason Merrill
2023-11-29 21:59 ` Marek Polacek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fc07bd10-8cc8-9dfa-339f-19d595de29c1@idea \
--to=ppalka@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=jason@redhat.com \
--cc=polacek@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).