Re: [PATCH 1/2] C++: more location wrapper nodes (PR c++/43064, PR c++/43486)

[Thomas Schwinge <thomas@codesourcery.com>]

Hi David!

I will admit that I don't have researched ;-/ what this is actually all
about, and how it's implemented, but...

On Mon,  5 Nov 2018 15:31:08 -0500, David Malcolm <dmalcolm@redhat.com> wrote:
> The C++ frontend gained various location wrapper nodes in r256448 (GCC 8).
> That patch:
>   https://gcc.gnu.org/ml/gcc-patches/2018-01/msg00799.html
> added wrapper nodes around all nodes with !CAN_HAVE_LOCATION_P for:
> 
> * arguments at callsites, and for
> 
> * typeid, alignof, sizeof, and offsetof.
> 
> This is a followup to that patch, adding many more location wrappers
> to the C++ frontend.  It adds location wrappers for nodes with
> !CAN_HAVE_LOCATION_P to:
> 
> * all literal nodes (in cp_parser_primary_expression)
> 
> * all id-expression nodes (in finish_id_expression), except within a
>   decltype.
> 
> * all mem-initializer nodes within a mem-initializer-list
>   (in cp_parser_mem_initializer)
> 
> However, the patch also adds some suppressions: regions in the parser
> for which wrapper nodes will not be created:
> 
> * within a template-parameter-list or template-argument-list (in
>   cp_parser_template_parameter_list and cp_parser_template_argument_list
>   respectively), to avoid encoding the spelling location of the nodes
>   in types.  For example, "array<10>" and "array<10>" are the same type,
>   despite the fact that the two different "10" tokens are spelled in
>   different locations in the source.
> 
> * within a gnu-style attribute (none of are handlers are set up to cope
>   with location wrappers yet)
> 
> * within various OpenMP clauses

... I did wonder why things applicable to OpenMP wouldn't likewise apply
to OpenACC, too?  That is:

> 	(cp_parser_omp_all_clauses): Don't create wrapper nodes within
> 	OpenMP clauses.
> 	(cp_parser_omp_for_loop): Likewise.
> 	(cp_parser_omp_declare_reduction_exprs): Likewise.

> @@ -33939,6 +33968,9 @@ cp_parser_omp_all_clauses (cp_parser *parser, omp_clause_mask mask,
>    bool first = true;
>    cp_token *token = NULL;
>  
> +  /* Don't create location wrapper nodes within OpenMP clauses.  */
> +  auto_suppress_location_wrappers sentinel;
> +
>    while (cp_lexer_next_token_is_not (parser->lexer, CPP_PRAGMA_EOL))
>      {
>        pragma_omp_clause c_kind;
> @@ -35223,6 +35255,10 @@ cp_parser_omp_for_loop (cp_parser *parser, enum tree_code code, tree clauses,
>  	}
>        loc = cp_lexer_consume_token (parser->lexer)->location;
>  
> +      /* Don't create location wrapper nodes within an OpenMP "for"
> +	 statement.  */
> +      auto_suppress_location_wrappers sentinel;
> +
>        matching_parens parens;
>        if (!parens.require_open (parser))
>  	return NULL;
> @@ -37592,6 +37628,8 @@ cp_parser_omp_declare_reduction_exprs (tree fndecl, cp_parser *parser)
>        else
>  	{
>  	  cp_parser_parse_tentatively (parser);
> +	  /* Don't create location wrapper nodes here.  */
> +	  auto_suppress_location_wrappers sentinel;
>  	  tree fn_name = cp_parser_id_expression (parser, /*template_p=*/false,
>  						  /*check_dependency_p=*/true,
>  						  /*template_p=*/NULL,

Shouldn't "cp_parser_oacc_all_clauses" (and "some" other functions?) be
adjusted in the same way?  How would I test that?  (I don't see any
OpenMP test cases added -- I have not yet tried whether any problems
would become apparent when temporarily removing the OpenMP changes cited
above.)


Grüße
 Thomas