On 6/15/20 2:59 PM, Patrick Palka wrote:
On Mon, 15 Jun 2020, Jason Merrill wrote:

On 6/5/20 5:16 PM, Patrick Palka wrote:
The previous patch mostly avoided making any changes that had no
functional impact, such as adjusting now-outdated comments and
performing renamings.  Such changes have been consolidated to this
followup patch for easier review.

The main change here is that we now reuse struct deferred_access_check
as the element type of the vector TI_TYPEDEFS_NEEDING_ACCESS_CHECKING
(now renamed to TI_DEFERRED_ACCESS_CHECKS, since it now may contain all
kinds of access checks).

gcc/cp/ChangeLog:

        PR c++/41437
        PR c++/47346
        * cp-tree.h (qualified_typedef_usage_s): Delete.
        (qualified_typedef_usage_t): Delete.
        (deferred_access_check): Move up in file.
        (tree_template_info::typedefs_needing_access_checking): Delete.
        (tree_template_info::deferred_access_checks): New field.
        (TI_TYPEDEFS_NEEDING_ACCESS_CHECKING): Rename to ...
        (TI_DEFERRED_ACCESS_CHECKS): ... this, and adjust accordingly.
        * pt.c (perform_typedefs_access_check): Rename to ...
        (perform_instantiation_time_access_checks): ... this, and adjust
        accordingly.  Remove unnecessary tree tests.
        (instantiate_class_template_1): Adjust accordingly.
        (instantiate_decl): Likewise.
        * semantics.c (enforce_access): Adjust to build up a
        deferred_access_check.
---
   gcc/cp/cp-tree.h   | 58 +++++++++++++++----------------------------
   gcc/cp/pt.c        | 61 +++++++++++++++++-----------------------------
   gcc/cp/semantics.c | 12 ++++-----
   3 files changed, 48 insertions(+), 83 deletions(-)

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 771d51cc283..50d83add458 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1449,27 +1449,6 @@ struct GTY (()) tree_lambda_expr
     int discriminator;
   };
   -/* A (typedef,context,usage location) triplet.
-   It represents a typedef used through a
-   context at a given source location.
-   e.g.
-   struct foo {
-     typedef int myint;
-   };
-
-   struct bar {
-    foo::myint v; // #1<-- this location.
-   };
-
-   In bar, the triplet will be (myint, foo, #1).
-   */
-struct GTY(()) qualified_typedef_usage_s {
-  tree typedef_decl;
-  tree context;
-  location_t locus;
-};
-typedef struct qualified_typedef_usage_s qualified_typedef_usage_t;
-
   /* Non-zero if this template specialization has access violations that
      should be rechecked when the function is instantiated outside argument
      deduction.  */
@@ -1489,11 +1468,24 @@ typedef struct qualified_typedef_usage_s
qualified_typedef_usage_t;
   #define TINFO_VAR_DECLARED_CONSTINIT(NODE) \
     (TREE_LANG_FLAG_2 (TEMPLATE_INFO_CHECK (NODE)))
   +/* The representation of a deferred access check.  */
+
+struct GTY(()) deferred_access_check {
+  /* The base class in which the declaration is referenced. */
+  tree binfo;
+  /* The declaration whose access must be checked.  */
+  tree decl;
+  /* The declaration that should be used in the error message.  */
+  tree diag_decl;
+  /* The location of this access.  */
+  location_t loc;
+};
+
   struct GTY(()) tree_template_info {
     struct tree_base base;
     tree tmpl;
     tree args;
-  vec<qualified_typedef_usage_t, va_gc> *typedefs_needing_access_checking;
+  vec<deferred_access_check, va_gc> *deferred_access_checks;
   };
     // Constraint information for a C++ declaration. Constraint information
is
@@ -3532,14 +3524,15 @@ struct GTY(()) lang_decl {
     ? int_cst_value (NON_DEFAULT_TEMPLATE_ARGS_COUNT (NODE)) \
     : TREE_VEC_LENGTH (INNERMOST_TEMPLATE_ARGS (NODE))
   #endif
-/* The list of typedefs - used in the template - that need
-   access checking at template instantiation time.
+
+/* The list of access checks that were deferred during parsing
+   which need to be performed at template instantiation time.
        FIXME this should be associated with the TEMPLATE_DECL, not the
      TEMPLATE_INFO.  */
-#define TI_TYPEDEFS_NEEDING_ACCESS_CHECKING(NODE) \
+#define TI_DEFERRED_ACCESS_CHECKS(NODE) \
     ((struct tree_template_info*)TEMPLATE_INFO_CHECK \
-     (NODE))->typedefs_needing_access_checking
+     (NODE))->deferred_access_checks
     /* We use TREE_VECs to hold template arguments.  If there is only one
      level of template arguments, then the TREE_VEC contains the
@@ -7090,19 +7083,6 @@ extern int shared_member_p
(tree);
   extern bool any_dependent_bases_p (tree = current_nonlambda_class_type
());
   extern bool maybe_check_overriding_exception_spec (tree, tree);
   -/* The representation of a deferred access check.  */
-
-struct GTY(()) deferred_access_check {
-  /* The base class in which the declaration is referenced. */
-  tree binfo;
-  /* The declaration whose access must be checked.  */
-  tree decl;
-  /* The declaration that should be used in the error message.  */
-  tree diag_decl;
-  /* The location of this access.  */
-  location_t loc;
-};
-
   /* in semantics.c */
   extern void push_deferring_access_checks     (deferring_kind);
   extern void resume_deferring_access_checks   (void);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index be319c50783..142392224c6 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -215,7 +215,7 @@ static bool
any_template_arguments_need_structural_equality_p (tree);
   static bool dependent_type_p_r (tree);
   static tree tsubst_copy      (tree, tree, tsubst_flags_t, tree);
   static tree tsubst_decl (tree, tree, tsubst_flags_t);
-static void perform_typedefs_access_check (tree tmpl, tree targs);
+static void perform_instantiation_time_access_checks (tree, tree);
   static tree listify (tree);
   static tree listify_autos (tree, tree);
   static tree tsubst_template_parm (tree, tree, tsubst_flags_t);
@@ -11502,46 +11502,39 @@ apply_late_template_attributes (tree *decl_p, tree
attributes, int attr_flags,
       }
   }
   -/* Perform (or defer) access check for typedefs that were referenced
-   from within the template TMPL code.
-   This is a subroutine of instantiate_decl and instantiate_class_template.
-   TMPL is the template to consider and TARGS is the list of arguments of
-   that template.  */
+/* The template TMPL is being instantiated with the template arguments
TARGS.
+   Perform the access checks that we deferred during parsing of the
+   template.  */
     static void
-perform_typedefs_access_check (tree tmpl, tree targs)
+perform_instantiation_time_access_checks (tree tmpl, tree targs)
   {
     unsigned i;
-  qualified_typedef_usage_t *iter;
+  deferred_access_check *chk;
   -  if (!tmpl
-      || (!CLASS_TYPE_P (tmpl)
-         && TREE_CODE (tmpl) != FUNCTION_DECL))
+  if (!CLASS_TYPE_P (tmpl) && TREE_CODE (tmpl) != FUNCTION_DECL)
       return;
   -  if (vec<qualified_typedef_usage_t, va_gc> *tdefs
-      = TI_TYPEDEFS_NEEDING_ACCESS_CHECKING (get_template_info (tmpl)))
-    FOR_EACH_VEC_ELT (*tdefs, i, iter)
+  if (vec<deferred_access_check, va_gc> *access_checks
+      = TI_DEFERRED_ACCESS_CHECKS (get_template_info (tmpl)))
+    FOR_EACH_VEC_ELT (*access_checks, i, chk)
         {
-       tree type_decl = iter->typedef_decl;
-       tree type_scope = iter->context;
-
-       if (!type_decl || !type_scope || !CLASS_TYPE_P (type_scope))
-         continue;
-
-       if (uses_template_parms (type_decl)
-           || (TREE_CODE (type_decl) == FIELD_DECL
-               && uses_template_parms (DECL_CONTEXT (type_decl))))
-         type_decl = tsubst_copy (type_decl, targs, tf_error, NULL_TREE);
+       tree decl = chk->decl;
+       tree diag_decl = chk->diag_decl;
+       tree type_scope = TREE_TYPE (chk->binfo);
+
+       if (uses_template_parms (decl)
+           || (TREE_CODE (decl) == FIELD_DECL
+               && uses_template_parms (DECL_CONTEXT (decl))))
+         decl = tsubst_copy (decl, targs, tf_error, NULL_TREE);

Can decl still be dependent?

Hmm, indeed it looks like not, now that we don't defer accesses of
dependently-scoped decls and instead assume that access will be checked
during substitution.  The following supplemental patch passes 'make
check-c++', does the original patch and this one look OK to commit after
a full bootstrap and regtest (and smoke testing)?

OK.


-- >8 --

Subject: [PATCH] c++: TI_DEFERRED_ACCESS_CHECKS and dependent decls

This adds an assert to enforce_access to verify that we don't explicitly
defer access checks of dependent decls -- we should instead be
re-checking the access of such a decl after tsubst'ing into it.

gcc/cp/ChangeLog:

        * pt.c (perform_instantiation_time_access_checks): Don't tsubst
        into decl.
        * semantics.c (enforce_access): Verify that decl is not
        dependent.
---
  gcc/cp/pt.c        | 4 ----
  gcc/cp/semantics.c | 6 ++++++
  2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 73f5935a4dc..efc69d5f81c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -11532,10 +11532,6 @@ perform_instantiation_time_access_checks (tree tmpl, 
tree targs)
        tree diag_decl = chk->diag_decl;
        tree type_scope = TREE_TYPE (chk->binfo);
- if (uses_template_parms (decl)
-           || (TREE_CODE (decl) == FIELD_DECL
-               && uses_template_parms (DECL_CONTEXT (decl))))
-         decl = tsubst_copy (decl, targs, tf_error, NULL_TREE);
        if (uses_template_parms (type_scope))
          type_scope = tsubst (type_scope, targs, tf_error, NULL_TREE);
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index a04b0aa1d9b..fa059ab6c8e 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -294,6 +294,12 @@ enforce_access (tree basetype_path, tree decl, tree 
diag_decl,
             assume it'll be accessible at instantiation time.  */
          return true;
+ /* Checking access of a dependent decl should be done again after
+          tsubst'ing into it rather than explicitly deferring here.  */
+       gcc_assert (!uses_template_parms (decl));
+       if (TREE_CODE (decl) == FIELD_DECL)
+         gcc_assert (!uses_template_parms (DECL_CONTEXT (decl)));
+
        /* Defer this access check until instantiation time.  */
        deferred_access_check access_check;
        access_check.binfo = basetype_path;


Reply via email to