On Fri, Dec 14, 2018 at 6:05 PM Alexandre Oliva <aol...@redhat.com> wrote:
>
> On Dec 14, 2018, Jason Merrill <ja...@redhat.com> wrote:
>
> > Let's move the initialization of "fields" inside the 'then' block here
> > with the initialization of "cvquals", rather than clear it in the
> > 'else'.
>
> We'd still have to NULL-initialize it somewhere, so I'd rather just move
> the entire loop into the conditional, and narrow the scope of variables
> only used within the loop, like this. The full patch below is very hard
> to read because of the reindentation, so here's a diff -b.
>
> diff --git a/gcc/cp/method.c b/gcc/cp/method.c
> index fd023e200538..17404a65b0fd 100644
> --- a/gcc/cp/method.c
> +++ b/gcc/cp/method.c
> @@ -675,12 +675,9 @@ do_build_copy_constructor (tree fndecl)
> }
> else
> {
> - tree fields = TYPE_FIELDS (current_class_type);
> tree member_init_list = NULL_TREE;
> - int cvquals = cp_type_quals (TREE_TYPE (parm));
> int i;
> tree binfo, base_binfo;
> - tree init;
> vec<tree, va_gc> *vbases;
>
> /* Initialize all the base-classes with the parameter converted
> @@ -704,15 +701,18 @@ do_build_copy_constructor (tree fndecl)
> inh, member_init_list);
> }
>
> - for (; fields; fields = DECL_CHAIN (fields))
> + if (!inh)
> + {
> + int cvquals = cp_type_quals (TREE_TYPE (parm));
> +
> + for (tree fields = TYPE_FIELDS (current_class_type);
> + fields; fields = DECL_CHAIN (fields))
> {
> tree field = fields;
> tree expr_type;
>
> if (TREE_CODE (field) != FIELD_DECL)
> continue;
> - if (inh)
> - continue;
>
> expr_type = TREE_TYPE (field);
> if (DECL_NAME (field))
> @@ -742,7 +742,7 @@ do_build_copy_constructor (tree fndecl)
> expr_type = cp_build_qualified_type (expr_type, quals);
> }
>
> - init = build3 (COMPONENT_REF, expr_type, parm, field, NULL_TREE);
> + tree init = build3 (COMPONENT_REF, expr_type, parm, field,
> NULL_TREE);
> if (move_p && !TYPE_REF_P (expr_type)
> /* 'move' breaks bit-fields, and has no effect for scalars.
> */
> && !scalarish_type_p (expr_type))
> @@ -751,6 +751,8 @@ do_build_copy_constructor (tree fndecl)
>
> member_init_list = tree_cons (field, init, member_init_list);
> }
> + }
> +
> finish_mem_initializers (member_init_list);
> }
> }
> @@ -891,6 +893,7 @@ synthesize_method (tree fndecl)
>
> /* Reset the source location, we might have been previously
> deferred, and thus have saved where we were first needed. */
> + if (!DECL_INHERITED_CTOR (fndecl))
> DECL_SOURCE_LOCATION (fndecl)
> = DECL_SOURCE_LOCATION (TYPE_NAME (DECL_CONTEXT (fndecl)));
>
>
> Is this OK too? (pending regstrapping)
Yes, thanks.
Jason
