On 11/29/23 17:01, Marek Polacek wrote:
On Wed, Nov 29, 2023 at 03:28:44PM -0500, Jason Merrill wrote:
On 11/29/23 12:43, Marek Polacek wrote:
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?
The relevant passage seems to be
https://eel.is/c++draft/class.access.base#6
after DR 52, which seems to have clarified that the pointer conversion only
applies to non-static members.
struct A { constexpr static int a = 0; };
struct D : private A {};
void f() {
D{}.A::a; // #1 GCC (and Clang) currently rejects
}
I see that MSVC also rejects it, while EDG accepts.
https://eel.is/c++draft/class.access.base#5.1 seems to say that a is
accessible when named in A.
https://eel.is/c++draft/expr.ref#7 also only constrains references to
non-static members.
But first we need to look up A in D, and A's injected-class-name looked up
as a member of D is not accessible; it's private, and f() is not a friend,
and we correctly complain about that.
If we avoid the lookup of A in D with
D{}.::A::a;
clang accepts it, which is consistent with accepting the template version,
and seems correct.
So, I think ba_any is what we want here.
Wow, that is not intuitive (to me at least). So I had it right but
only by accident.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >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.
The rationale for using ba_any is explained at
<https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638676.html>.
I'd prefer not to cite the mailing list for rationales.
To summarize:
[class.access.base] requires conversion to a unique base subobject for
non-static data members, but it does not require that the base be unique
or accessible for static data members.
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.
* g++.dg/lookup/scoped14.C: New test.
* g++.dg/lookup/scoped15.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 ++++++++++++++
gcc/testsuite/g++.dg/lookup/scoped14.C | 14 ++++++++++++++
gcc/testsuite/g++.dg/lookup/scoped15.C | 20 ++++++++++++++++++++
6 files changed, 94 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
create mode 100644 gcc/testsuite/g++.dg/lookup/scoped14.C
create mode 100644 gcc/testsuite/g++.dg/lookup/scoped15.C
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 0839d0a4167..bf8ffaa7e75 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -3467,7 +3467,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;
@@ -3484,9 +3484,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 || shared_member_p (m))
+ return ba_any;
+ }
+ 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." }
+}
diff --git a/gcc/testsuite/g++.dg/lookup/scoped14.C
b/gcc/testsuite/g++.dg/lookup/scoped14.C
new file mode 100644
index 00000000000..141aa0d2b1a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/scoped14.C
@@ -0,0 +1,14 @@
+// PR c++/112744
+// { dg-do compile { target c++11 } }
+
+struct A { int a = 0; };
+struct B : A {};
+struct C : A {};
+struct D : B, C {};
+
+int main()
+{
+ D d;
+ (void) d.a; // { dg-error "request for member .a. is ambiguous" }
+ (void) d.A::a; // { dg-error ".A. is an ambiguous base of .D." }
+}
diff --git a/gcc/testsuite/g++.dg/lookup/scoped15.C
b/gcc/testsuite/g++.dg/lookup/scoped15.C
new file mode 100644
index 00000000000..d450a41a617
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lookup/scoped15.C
@@ -0,0 +1,20 @@
+// PR c++/112744
+// { dg-do compile { target c++11 } }
+
+struct A { constexpr static int a = 0; };
+struct D : private A {};
+
+// See <https://gcc.gnu.org/pipermail/gcc-patches/2023-November/638676.html>
+// for rationale.
The injected-class-name of A is private when named in D, but if A is
named some other way, there is no requirement in [class.access.base] for
static data members that it be an accessible base.
OK with those comment adjustments.
Jason
