public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Defer access checking when processing bases [PR82613]
@ 2021-01-18  5:31 Patrick Palka
  2021-01-19 21:07 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: Patrick Palka @ 2021-01-18  5:31 UTC (permalink / raw)
  To: gcc-patches

When parsing the base-clause of a class declaration, we need to defer
access checking until the entire base-clause has been seen, so that
access can be properly checked relative to the scope of the class with
all its bases attached.  This allows us to accept the declaration of
struct D from Example 2 of [class.access.general] (access12.C below).

Similarly when substituting into the base-clause of a class template,
which is the subject of PR82613.

Bootstrapped and regtested on x86_64-pc-linxu-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

	PR c++/82613
	* parser.c (cp_parser_class_head): Defer access checking when
	parsing the base-clause until all bases are seen and attached
	to the class type.
	* pt.c (instantiate_class_template): Likewise when substituting
	into dependent bases.

gcc/testsuite/ChangeLog:

	PR c++/82613
	* g++.dg/parse/access12.C: New test.
	* g++.dg/template/access35.C: New test.
---
 gcc/cp/parser.c                          | 30 ++++++++++++++----------
 gcc/cp/pt.c                              | 16 ++++++-------
 gcc/testsuite/g++.dg/parse/access12.C    | 24 +++++++++++++++++++
 gcc/testsuite/g++.dg/template/access35.C | 26 ++++++++++++++++++++
 4 files changed, 75 insertions(+), 21 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/parse/access12.C
 create mode 100644 gcc/testsuite/g++.dg/template/access35.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 88c6e2648cb..57843cd65c6 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -25578,19 +25578,11 @@ cp_parser_class_head (cp_parser* parser,
 
      is valid.  */
 
-  /* Get the list of base-classes, if there is one.  */
+  /* Get the list of base-classes, if there is one.  Defer access checking
+     until the entire list has been seen, as per [class.access.general].  */
+  push_deferring_access_checks (dk_deferred);
   if (cp_lexer_next_token_is (parser->lexer, CPP_COLON))
-    {
-      /* PR59482: enter the class scope so that base-specifiers are looked
-	 up correctly.  */
-      if (type)
-	pushclass (type);
-      bases = cp_parser_base_clause (parser);
-      /* PR59482: get out of the previously pushed class scope so that the
-	 subsequent pops pop the right thing.  */
-      if (type)
-	popclass ();
-    }
+    bases = cp_parser_base_clause (parser);
   else
     bases = NULL_TREE;
 
@@ -25599,6 +25591,20 @@ cp_parser_class_head (cp_parser* parser,
   if (type && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
     xref_basetypes (type, bases);
 
+  /* Now that all bases have been seen and attached to the class, check
+     accessibility of the types named in the base-clause.  This must be
+     done relative to the class scope, so that we accept e.g.
+
+       class A { protected: struct B {}; };
+       struct C : A::B, A {}; // OK: A::B is accessible from C
+
+     as per [class.access.general].  */
+  if (type)
+    pushclass (type);
+  pop_to_parent_deferring_access_checks ();
+  if (type)
+    popclass ();
+
  done:
   /* Leave the scope given by the nested-name-specifier.  We will
      enter the class scope itself while processing the members.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a82324d23be..d5d3d2fd040 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11825,17 +11825,14 @@ instantiate_class_template_1 (tree type)
 	      || COMPLETE_OR_OPEN_TYPE_P (TYPE_CONTEXT (type)));
 
   base_list = NULL_TREE;
+  /* Defer access checking while we substitute into the types named in
+     the base-clause.  */
+  push_deferring_access_checks (dk_deferred);
   if (BINFO_N_BASE_BINFOS (pbinfo))
     {
       tree pbase_binfo;
-      tree pushed_scope;
       int i;
 
-      /* We must enter the scope containing the type, as that is where
-	 the accessibility of types named in dependent bases are
-	 looked up from.  */
-      pushed_scope = push_scope (CP_TYPE_CONTEXT (type));
-
       /* Substitute into each of the bases to determine the actual
 	 basetypes.  */
       for (i = 0; BINFO_BASE_ITERATE (pbinfo, i, pbase_binfo); i++)
@@ -11877,9 +11874,6 @@ instantiate_class_template_1 (tree type)
 
       /* The list is now in reverse order; correct that.  */
       base_list = nreverse (base_list);
-
-      if (pushed_scope)
-	pop_scope (pushed_scope);
     }
   /* Now call xref_basetypes to set up all the base-class
      information.  */
@@ -11897,6 +11891,10 @@ instantiate_class_template_1 (tree type)
      class, except we also need to push the enclosing classes.  */
   push_nested_class (type);
 
+  /* Now check accessibility of the types named in its base-clause,
+     relative to the scope of the class.  */
+  pop_to_parent_deferring_access_checks ();
+
   /* Now members are processed in the order of declaration.  */
   for (member = CLASSTYPE_DECL_LIST (pattern);
        member; member = TREE_CHAIN (member))
diff --git a/gcc/testsuite/g++.dg/parse/access12.C b/gcc/testsuite/g++.dg/parse/access12.C
new file mode 100644
index 00000000000..6963e0eb089
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/access12.C
@@ -0,0 +1,24 @@
+// Example 2 of [class.access.general]
+// { dg-do compile }
+
+class A {
+  typedef int I;    // private member
+  I f();
+  friend I g(I);
+  static I x;
+  template<int> struct Q;
+  template<int> friend struct R;
+protected:
+    struct B { };
+};
+
+A::I A::f() { return 0; }
+A::I g(A::I p = A::x);
+A::I g(A::I p) { return 0; }
+A::I A::x = 0;
+// FIXME: We reject these two declarations because access checking of A::I
+// is not performed in the scope of the class being declared.
+// template<A::I> struct A::Q { };
+// template<A::I> struct R { };
+
+struct D: A::B, A { };
diff --git a/gcc/testsuite/g++.dg/template/access35.C b/gcc/testsuite/g++.dg/template/access35.C
new file mode 100644
index 00000000000..dcd2c229b35
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/access35.C
@@ -0,0 +1,26 @@
+// PR c++/82613
+// { dg-do compile }
+
+struct Empty {};
+
+template <typename T>
+struct B;
+
+struct A
+{
+    friend struct B<A>;
+
+private:
+    typedef Empty BaseForB;
+};
+
+template <typename T>
+struct B : T::BaseForB
+{
+};
+
+int main()
+{
+    B<A> test;
+    return 0;
+}
-- 
2.30.0.155.g66e871b664


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] c++: Defer access checking when processing bases [PR82613]
  2021-01-18  5:31 [PATCH] c++: Defer access checking when processing bases [PR82613] Patrick Palka
@ 2021-01-19 21:07 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2021-01-19 21:07 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 1/18/21 12:31 AM, Patrick Palka wrote:
> When parsing the base-clause of a class declaration, we need to defer
> access checking until the entire base-clause has been seen, so that
> access can be properly checked relative to the scope of the class with
> all its bases attached.  This allows us to accept the declaration of
> struct D from Example 2 of [class.access.general] (access12.C below).
> 
> Similarly when substituting into the base-clause of a class template,
> which is the subject of PR82613.
> 
> Bootstrapped and regtested on x86_64-pc-linxu-gnu, does this look OK for
> trunk?

OK.

> gcc/cp/ChangeLog:
> 
> 	PR c++/82613
> 	* parser.c (cp_parser_class_head): Defer access checking when
> 	parsing the base-clause until all bases are seen and attached
> 	to the class type.
> 	* pt.c (instantiate_class_template): Likewise when substituting
> 	into dependent bases.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/82613
> 	* g++.dg/parse/access12.C: New test.
> 	* g++.dg/template/access35.C: New test.
> ---
>   gcc/cp/parser.c                          | 30 ++++++++++++++----------
>   gcc/cp/pt.c                              | 16 ++++++-------
>   gcc/testsuite/g++.dg/parse/access12.C    | 24 +++++++++++++++++++
>   gcc/testsuite/g++.dg/template/access35.C | 26 ++++++++++++++++++++
>   4 files changed, 75 insertions(+), 21 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/parse/access12.C
>   create mode 100644 gcc/testsuite/g++.dg/template/access35.C
> 
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 88c6e2648cb..57843cd65c6 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -25578,19 +25578,11 @@ cp_parser_class_head (cp_parser* parser,
>   
>        is valid.  */
>   
> -  /* Get the list of base-classes, if there is one.  */
> +  /* Get the list of base-classes, if there is one.  Defer access checking
> +     until the entire list has been seen, as per [class.access.general].  */
> +  push_deferring_access_checks (dk_deferred);
>     if (cp_lexer_next_token_is (parser->lexer, CPP_COLON))
> -    {
> -      /* PR59482: enter the class scope so that base-specifiers are looked
> -	 up correctly.  */
> -      if (type)
> -	pushclass (type);
> -      bases = cp_parser_base_clause (parser);
> -      /* PR59482: get out of the previously pushed class scope so that the
> -	 subsequent pops pop the right thing.  */
> -      if (type)
> -	popclass ();
> -    }
> +    bases = cp_parser_base_clause (parser);
>     else
>       bases = NULL_TREE;
>   
> @@ -25599,6 +25591,20 @@ cp_parser_class_head (cp_parser* parser,
>     if (type && cp_lexer_next_token_is (parser->lexer, CPP_OPEN_BRACE))
>       xref_basetypes (type, bases);
>   
> +  /* Now that all bases have been seen and attached to the class, check
> +     accessibility of the types named in the base-clause.  This must be
> +     done relative to the class scope, so that we accept e.g.
> +
> +       class A { protected: struct B {}; };
> +       struct C : A::B, A {}; // OK: A::B is accessible from C
> +
> +     as per [class.access.general].  */
> +  if (type)
> +    pushclass (type);
> +  pop_to_parent_deferring_access_checks ();
> +  if (type)
> +    popclass ();
> +
>    done:
>     /* Leave the scope given by the nested-name-specifier.  We will
>        enter the class scope itself while processing the members.  */
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index a82324d23be..d5d3d2fd040 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -11825,17 +11825,14 @@ instantiate_class_template_1 (tree type)
>   	      || COMPLETE_OR_OPEN_TYPE_P (TYPE_CONTEXT (type)));
>   
>     base_list = NULL_TREE;
> +  /* Defer access checking while we substitute into the types named in
> +     the base-clause.  */
> +  push_deferring_access_checks (dk_deferred);
>     if (BINFO_N_BASE_BINFOS (pbinfo))
>       {
>         tree pbase_binfo;
> -      tree pushed_scope;
>         int i;
>   
> -      /* We must enter the scope containing the type, as that is where
> -	 the accessibility of types named in dependent bases are
> -	 looked up from.  */
> -      pushed_scope = push_scope (CP_TYPE_CONTEXT (type));
> -
>         /* Substitute into each of the bases to determine the actual
>   	 basetypes.  */
>         for (i = 0; BINFO_BASE_ITERATE (pbinfo, i, pbase_binfo); i++)
> @@ -11877,9 +11874,6 @@ instantiate_class_template_1 (tree type)
>   
>         /* The list is now in reverse order; correct that.  */
>         base_list = nreverse (base_list);
> -
> -      if (pushed_scope)
> -	pop_scope (pushed_scope);
>       }
>     /* Now call xref_basetypes to set up all the base-class
>        information.  */
> @@ -11897,6 +11891,10 @@ instantiate_class_template_1 (tree type)
>        class, except we also need to push the enclosing classes.  */
>     push_nested_class (type);
>   
> +  /* Now check accessibility of the types named in its base-clause,
> +     relative to the scope of the class.  */
> +  pop_to_parent_deferring_access_checks ();
> +
>     /* Now members are processed in the order of declaration.  */
>     for (member = CLASSTYPE_DECL_LIST (pattern);
>          member; member = TREE_CHAIN (member))
> diff --git a/gcc/testsuite/g++.dg/parse/access12.C b/gcc/testsuite/g++.dg/parse/access12.C
> new file mode 100644
> index 00000000000..6963e0eb089
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/parse/access12.C
> @@ -0,0 +1,24 @@
> +// Example 2 of [class.access.general]
> +// { dg-do compile }
> +
> +class A {
> +  typedef int I;    // private member
> +  I f();
> +  friend I g(I);
> +  static I x;
> +  template<int> struct Q;
> +  template<int> friend struct R;
> +protected:
> +    struct B { };
> +};
> +
> +A::I A::f() { return 0; }
> +A::I g(A::I p = A::x);
> +A::I g(A::I p) { return 0; }
> +A::I A::x = 0;
> +// FIXME: We reject these two declarations because access checking of A::I
> +// is not performed in the scope of the class being declared.
> +// template<A::I> struct A::Q { };
> +// template<A::I> struct R { };
> +
> +struct D: A::B, A { };
> diff --git a/gcc/testsuite/g++.dg/template/access35.C b/gcc/testsuite/g++.dg/template/access35.C
> new file mode 100644
> index 00000000000..dcd2c229b35
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/access35.C
> @@ -0,0 +1,26 @@
> +// PR c++/82613
> +// { dg-do compile }
> +
> +struct Empty {};
> +
> +template <typename T>
> +struct B;
> +
> +struct A
> +{
> +    friend struct B<A>;
> +
> +private:
> +    typedef Empty BaseForB;
> +};
> +
> +template <typename T>
> +struct B : T::BaseForB
> +{
> +};
> +
> +int main()
> +{
> +    B<A> test;
> +    return 0;
> +}
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-01-19 21:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-18  5:31 [PATCH] c++: Defer access checking when processing bases [PR82613] Patrick Palka
2021-01-19 21:07 ` Jason Merrill

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).