This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C++ Patch] [PR c++/88146] do not crash synthesizing inherited ctor(...)


On Fri, Dec 14, 2018 at 6:05 PM Alexandre Oliva <aoliva@redhat.com> wrote:
>
> On Dec 14, 2018, Jason Merrill <jason@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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]