Re: [PATCH] PR c++/68795: fix uninitialized close_paren_loc in cp_parser_postfix_expression (v2)

[David Malcolm <dmalcolm at redhat dot com>]

On Mon, 2016-01-11 at 16:51 +0100, Bernd Schmidt wrote:
> On 12/18/2015 08:05 PM, David Malcolm wrote:
> > On Thu, 2015-12-17 at 19:21 +0100, Bernd Schmidt wrote:
> >> On 12/17/2015 07:32 PM, David Malcolm wrote:
> >>> +             if (close_paren_loc)
> >>
> >> close_paren_loc != UNKNOWN_LOCATION - it's very confusing otherwise.
> >>
> >>
> >> Bernd
> >
> > Here's an updated version; the only change since v1 is:
> > -           if (close_paren_loc)
> > +           if (close_paren_loc != UNKNOWN_LOCATION)
> >
> > Have verified the fix in valgrind.  OK for trunk if it still passes
> > bootstrap&regrtest?
> >
> > gcc/cp/ChangeLog:
> > 	* parser.c (cp_parser_postfix_expression): Initialize
> > 	close_paren_loc to UNKNOWN_LOCATION; only use it if
> > 	it has been written to by
> > 	cp_parser_parenthesized_expression_list.
> > 	(cp_parser_postfix_dot_deref_expression): Likewise.
> > 	(cp_parser_parenthesized_expression_list): Document the behavior
> > 	with respect to the CLOSE_PAREN_LOC param.
> 
> I usually like to leave C++ patches to Jason, but I guess I don't need 
> to know the standard inside out for this one.
> 
> Prior to your ealier patch, we had
> 
> protected_set_expr_location (postfix_expression, token->location);
> 
> It looks like after your new patch, we could end up not setting the 
> location on the expr at all if close_paren_loc is still 
> UNKNOWN_LOCATION. I'm guessing this is intentional as the comment update 
> suggests this is an error path, and the cp_expr constructor ensures that 
> we get at least UNKNOWN_LOCATION and not another uninitialized loc.

Yes.

> If I'm correct in all that, the patch is ok.

Thanks.  Committed to trunk as r232238, having verified
bootstrap&regrtest, and re-verified the PR under valgrind.

Dave