On Sat, Jan 13, 2018 at 12:10:22PM +0100, Jakub Jelinek wrote:
> On Fri, Jan 12, 2018 at 01:13:17PM -0500, Jason Merrill wrote:
> > On Thu, Jan 11, 2018 at 5:11 PM, Paolo Carlini <paolo.carl...@oracle.com>
> > wrote:
> > > On 11/01/2018 21:33, Jason Merrill wrote:
> > >> On 01/10/2018 06:50 PM, Paolo Carlini wrote:
> > >>>
> > >>> thus the below is a rather "dull" solution at the level of
> > >>> cplus_decl_attributes itself: cp_check_const_attributes is tweaked to
> > >>> check
> > >>> for error_mark_node at each outer iteration
> > >>
> > >> This shouldn't be necessary; we should have returned error_mark_node for
> > >> the attribute list rather than append it to something else, in which case
> > >> the existing check for attributes == error_mark_node would have done the
> > >> job.
> > >
> > > Indeed. That seems the most straightforward way to approach the issue.
> > > Thanks.
> > >
> > > In grokdeclarator we could either explicitly assign error_mark_node to
> > > *attrlist (instead of chaining) or simply drop the erroneous
> > > declarator->std_attributes. Both solutions appear to work fine in
> > > practice,
> > > pass testing.
> >
> > Hmm, I think dropping the attributes is reasonable for grokdeclarator
> > to do as error-recovery, similarly to how it discards an ill-formed
> > exception-specification. But let's assert seen_error() in that case.
>
> PR83824 is simiar to this PR, just in a different spot, but again:
> 13406 decl_specs->attributes
> 13407 = chainon (decl_specs->attributes,
> 13408 attrs);
> in parser.c and decl_specs->attributes is error_mark_node and attrs
> is error_mark_node too, yet seen_error () is false. The reason for that
> is that we are doing tentative parsing here, so e.g.
> if (!parens.require_close (parser))
> return error_mark_node;
> doesn't report any error but just returns error_mark_node.
> 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);
> or we can not add error_mark_node attributes at all, but we can't assert
> seen_error () in that case; perhaps track in a side boolean in decl_specs
> whether attributes is errorneous.
Or we could not add those error_mark_nodes and
gcc_assert (seen_error () || cp_parser_error_occurred (parser));
Jakub
