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)?
-- >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;
--
2.27.0.83.g0313f36c6e
>
> > if (uses_template_parms (type_scope))
> > type_scope = tsubst (type_scope, targs, tf_error, NULL_TREE);
> > /* Make access check error messages point to the location
> > of the use of the typedef. */
> > - iloc_sentinel ils (iter->locus);
> > + iloc_sentinel ils (chk->loc);
> > perform_or_defer_access_check (TYPE_BINFO (type_scope),
> > - type_decl, type_decl,
> > - tf_warning_or_error);
> > + decl, diag_decl, tf_warning_or_error);
> > }
> > }
> > @@ -12070,11 +12063,7 @@ instantiate_class_template_1 (tree type)
> > definitions or default arguments, of the class member functions,
> > member classes, static data members and member templates.... */
> > - /* Some typedefs referenced from within the template code need to be
> > access
> > - checked at template instantiation time, i.e now. These types were
> > - added to the template at parsing time. Let's get those and perform
> > - the access checks then. */
> > - perform_typedefs_access_check (pattern, args);
> > + perform_instantiation_time_access_checks (pattern, args);
> > perform_deferred_access_checks (tf_warning_or_error);
> > pop_nested_class ();
> > maximum_field_alignment = saved_maximum_field_alignment;
> > @@ -25621,12 +25610,8 @@ instantiate_decl (tree d, bool defer_ok, bool
> > expl_inst_class_mem_p)
> > else
> > start_preparsed_function (d, NULL_TREE, SF_PRE_PARSED);
> > - /* Some typedefs referenced from within the template code need to
> > be
> > - access checked at template instantiation time, i.e now. These
> > - types were added to the template at parsing time. Let's get those
> > - and perform the access checks then. */
> > - perform_typedefs_access_check (DECL_TEMPLATE_RESULT (td),
> > - args);
> > + perform_instantiation_time_access_checks (DECL_TEMPLATE_RESULT (td),
> > + args);
> > /* Create substitution entries for the parameters. */
> > register_parameter_specializations (code_pattern, d);
> > diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
> > index bf1d720347a..b94cd88166e 100644
> > --- a/gcc/cp/semantics.c
> > +++ b/gcc/cp/semantics.c
> > @@ -295,12 +295,12 @@ enforce_access (tree basetype_path, tree decl, tree
> > diag_decl,
> > return true;
> > /* Defer this access check until instantiation time. */
> > - qualified_typedef_usage_t typedef_usage;
> > - typedef_usage.typedef_decl = decl;
> > - typedef_usage.context = TREE_TYPE (basetype_path);
> > - typedef_usage.locus = input_location;
> > - vec_safe_push (TI_TYPEDEFS_NEEDING_ACCESS_CHECKING (template_info),
> > - typedef_usage);
> > + deferred_access_check access_check;
> > + access_check.binfo = basetype_path;
> > + access_check.decl = decl;
> > + access_check.diag_decl = diag_decl;
> > + access_check.loc = input_location;
> > + vec_safe_push (TI_DEFERRED_ACCESS_CHECKS (template_info),
> > access_check);
> > return true;
> > }
> >
>
>
