On Sat, Jan 13, 2018 at 7:59 AM, Paolo Carlini <paolo.carl...@oracle.com> wrote:
> Hi Jakub, all,
>
>> On 13 Jan 2018, at 12:32, Jakub Jelinek <ja...@redhat.com> wrote:
>>
>>> On Sat, Jan 13, 2018 at 12:12:02PM +0100, Jakub Jelinek wrote:
>>> Or we could not add those error_mark_nodes and
>>> gcc_assert (seen_error () || cp_parser_error_occurred (parser));
>>
>> This fixes the testcase:
>> --- gcc/cp/parser.c.jj 2018-01-11 18:58:48.386391801 +0100
>> +++ gcc/cp/parser.c 2018-01-13 12:17:20.545347195 +0100
>> @@ -13403,6 +13403,9 @@ cp_parser_decl_specifier_seq (cp_parser*
>> }
>> }
>>
>> + if (attrs == error_mark_node)
>> + gcc_assert (seen_error () || cp_parser_error_occurred (parser));
>> + else
>> decl_specs->attributes
>> = chainon (decl_specs->attributes,
>> attrs);
>> but there are 13 chainon calls like this in parser.c. Shouldn't we introduce
>> a helper function for this, perhaps:
>>
>> void
>> attr_chainon (cp_parser *parser, tree *attrs, tree attr)
>> {
>> /* Don't add error_mark_node to the chain, it can't be chained.
>> Assert this is during error recovery. */
>> if (attr == error_mark_node)
>> gcc_assert (seen_error () || cp_parser_error_occurred (parser));
>> else
>> *attrs = chainon (*attrs, attr);
>> }
>>
>> and change all affected spots, like
>>
>> attr_chainon (parser, &decl_specs->attributes, attrs);
>>
>> ?
>
> First, thanks for your messages. Personally, at this late time for 8, I vote
> for something like my most recent grokdeclarator fix and yours above for
> 83824. Then, for 9, or even 8.2, the more encompassing change for all those
> chainons. Please both of you let me know how shall we proceed, I could
> certainly take care of the latter too from now on. Thanks again!
Let's go ahead with your patch to grokdeclarator. In the parser,
let's do what Jakub is suggesting here:
> So, either we want to go with what Paolo posted even in this case,
> i.e. turn decl_specs->attributes into error_mark_node if attrs
> is error_mark_node, and don't chainon anything to it if decl_specs->attributes
> is already error_mark_node, e.g. something like:
> if (attrs == error_mark_node || decl_specs->attributes == error_mark_node)
> decl_specs->attributes = error_mark_node;
> else
> decl_specs->attributes = chainon (decl_specs->attributes, attrs);
without any assert. Putting this logic in an attr_chainon function sounds good.
Jason
