Hi,
of the various access control issues we have got, this one seems rather
manageable.
It seems to me that in case of nested anonymous aggregates what is
needed to get the access control right is just a bit of recursion, to
completely propagate the access from the outer to the inner aggregates.
The patch seems large but in fact just moves to a recursive helper the
code that currently handles anonymous aggregates in finish_struct_anon.
Patch appears to work well, no regressions etc, I'm not 100% sure we
couldn't simplify a bit the new finish_struct_anon_r.
Thanks!
Paolo.
////////////////////////
/cp
2013-09-04
PR c++/24926
* class.c (finish_struct_anon_r): New.
(finish_struct_anon): Use it.
/testsuite
2013-09-04
PR c++/24926
* g++.dg/parse/access11.C: New.
Index: cp/class.c
===================================================================
--- cp/class.c (revision 202231)
+++ cp/class.c (working copy)
@@ -2773,73 +2773,112 @@ warn_hidden (tree t)
}
}
-/* Check for things that are invalid. There are probably plenty of other
- things we should check for also. */
+/* Recursive helper for finish_struct_anon. */
static void
-finish_struct_anon (tree t)
+finish_struct_anon_r (tree field, bool complain)
{
- tree field;
-
- for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
+ if (DECL_NAME (field) == NULL_TREE
+ && ANON_AGGR_TYPE_P (TREE_TYPE (field)))
{
- if (TREE_STATIC (field))
- continue;
- if (TREE_CODE (field) != FIELD_DECL)
- continue;
+ bool is_union = TREE_CODE (TREE_TYPE (field)) == UNION_TYPE;
+ tree elt = TYPE_FIELDS (TREE_TYPE (field));
+ for (; elt; elt = DECL_CHAIN (elt))
+ {
+ /* We're generally only interested in entities the user
+ declared, but we also find nested classes by noticing
+ the TYPE_DECL that we create implicitly. You're
+ allowed to put one anonymous union inside another,
+ though, so we explicitly tolerate that. We use
+ TYPE_ANONYMOUS_P rather than ANON_AGGR_TYPE_P so that
+ we also allow unnamed types used for defining fields. */
+ if (DECL_ARTIFICIAL (elt)
+ && (!DECL_IMPLICIT_TYPEDEF_P (elt)
+ || TYPE_ANONYMOUS_P (TREE_TYPE (elt))))
+ continue;
- if (DECL_NAME (field) == NULL_TREE
- && ANON_AGGR_TYPE_P (TREE_TYPE (field)))
- {
- bool is_union = TREE_CODE (TREE_TYPE (field)) == UNION_TYPE;
- tree elt = TYPE_FIELDS (TREE_TYPE (field));
- for (; elt; elt = DECL_CHAIN (elt))
+ if (!complain && TREE_STATIC (elt))
+ continue;
+
+ if (TREE_CODE (elt) != FIELD_DECL)
{
- /* We're generally only interested in entities the user
- declared, but we also find nested classes by noticing
- the TYPE_DECL that we create implicitly. You're
- allowed to put one anonymous union inside another,
- though, so we explicitly tolerate that. We use
- TYPE_ANONYMOUS_P rather than ANON_AGGR_TYPE_P so that
- we also allow unnamed types used for defining fields. */
- if (DECL_ARTIFICIAL (elt)
- && (!DECL_IMPLICIT_TYPEDEF_P (elt)
- || TYPE_ANONYMOUS_P (TREE_TYPE (elt))))
- continue;
-
- if (TREE_CODE (elt) != FIELD_DECL)
+ if (complain)
{
if (is_union)
- permerror (input_location, "%q+#D invalid; an anonymous
union can "
+ permerror (input_location,
+ "%q+#D invalid; an anonymous union can "
"only have non-static data members", elt);
else
- permerror (input_location, "%q+#D invalid; an anonymous
struct can "
+ permerror (input_location,
+ "%q+#D invalid; an anonymous struct can "
"only have non-static data members", elt);
- continue;
}
+ continue;
+ }
+ if (complain)
+ {
if (TREE_PRIVATE (elt))
{
if (is_union)
- permerror (input_location, "private member %q+#D in
anonymous union", elt);
+ permerror (input_location,
+ "private member %q+#D in anonymous union",
+ elt);
else
- permerror (input_location, "private member %q+#D in
anonymous struct", elt);
+ permerror (input_location,
+ "private member %q+#D in anonymous struct",
+ elt);
}
else if (TREE_PROTECTED (elt))
{
if (is_union)
- permerror (input_location, "protected member %q+#D in
anonymous union", elt);
+ permerror (input_location,
+ "protected member %q+#D in anonymous union",
+ elt);
else
- permerror (input_location, "protected member %q+#D in
anonymous struct", elt);
+ permerror (input_location,
+ "protected member %q+#D in anonymous struct",
+ elt);
}
+ }
- TREE_PRIVATE (elt) = TREE_PRIVATE (field);
- TREE_PROTECTED (elt) = TREE_PROTECTED (field);
- }
+ TREE_PRIVATE (elt) = TREE_PRIVATE (field);
+ TREE_PROTECTED (elt) = TREE_PROTECTED (field);
+
+ /* Recur inside the anonymous aggregates to handle correctly
+ access control (c++/24926):
+
+ class A {
+ union {
+ union {
+ int i;
+ };
+ };
+ };
+
+ int j=A().i; */
+ finish_struct_anon_r (elt, /*complain=*/false);
}
}
}
+/* Check for things that are invalid. There are probably plenty of other
+ things we should check for also. */
+
+static void
+finish_struct_anon (tree t)
+{
+ for (tree field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
+ {
+ if (TREE_STATIC (field))
+ continue;
+ if (TREE_CODE (field) != FIELD_DECL)
+ continue;
+
+ finish_struct_anon_r (field, /*complain=*/true);
+ }
+}
+
/* Add T to CLASSTYPE_DECL_LIST of current_class_type which
will be used later during class template instantiation.
When FRIEND_P is zero, T can be a static member data (VAR_DECL),
Index: testsuite/g++.dg/parse/access11.C
===================================================================
--- testsuite/g++.dg/parse/access11.C (revision 0)
+++ testsuite/g++.dg/parse/access11.C (working copy)
@@ -0,0 +1,35 @@
+// PR c++/24926
+
+class A {
+ union {
+ int i; // { dg-error "private" }
+ };
+ union {
+ int j; // { dg-error "private" }
+ };
+ union {
+ union {
+ int k; // { dg-error "private" }
+ };
+ union {
+ union {
+ int l; // { dg-error "private" }
+ };
+ union {
+ int m; // { dg-error "private" }
+ union {
+ int n; // { dg-error "private" }
+ int o; // { dg-error "private" }
+ };
+ };
+ };
+ };
+};
+
+int a1 = A().i; // { dg-error "context" }
+int a2 = A().j; // { dg-error "context" }
+int a3 = A().k; // { dg-error "context" }
+int a4 = A().l; // { dg-error "context" }
+int a5 = A().m; // { dg-error "context" }
+int a6 = A().n; // { dg-error "context" }
+int a7 = A().o; // { dg-error "context" }
