Re: Location wrappers vs decls that change type (was Re: [v2 of PATCH 03/14] C++: add location_t wrapper nodes during parsing (minimal impl))

[Nathan Sidwell <nathan at acm dot org>]

On 01/08/2018 12:02 PM, David Malcolm wrote:
> On Fri, 2018-01-05 at 17:20 -0500, David Malcolm wrote:

> Doing so uncovered an issue which I'm not sure how to resolve: it's
> possible for a decl to change type during parsing, after location
> wrappers may have been created, which changes location_wrapper_p on
> those wrappers from true to false.

> Asserting that the only VIEW_CONVERT_EXPR or NON_LVALUE_EXPR seen in
> tsubst_copy and tsubst_copy_and_build are location_wrapper_p leads to
> an ICE on the above code.
>
> What's happening is as follows.  First, in the call:
>
>       6      assign(_S_terminal);
>                     ^~~~~~~~~~~
>
> the VAR_DECL "_S_terminal" gains a VIEW_CONVERT_EXPR location wrapper
> node to express the underline shown above.
>
> Later, during parsing of this init-declarator:
>
>      10  template<typename _CharT>
>      11  const _CharT basic_string<_CharT>::_S_terminal = _CharT();
>                                             ^~~~~~~~~~~
>
>
> ...cp_parser_init_declarator calls start_decl, which calls
> duplicate_decls, merging the "_S_terminal" seen here:
...
> Both "_S_terminal" VAR_DECLs have a "_CharT" TEMPLATE_TYPE_PARM, but
> these types are different tree nodes.

correct. they are not EQ but are EQUAL (same_type_p will be true).

> Hence the type of the first VAR_DECL changes in duplicate_decls here:
>
> 2152	      TREE_TYPE (newdecl) = TREE_TYPE (olddecl) = newtype;
>
> ...changing type to the TEMPLATE_TYPE_PARM of the second VAR_DECL.

> 18306	    case VIEW_CONVERT_EXPR:
> 18307	    case NON_LVALUE_EXPR:
> 18308	      gcc_assert (location_wrapper_p (t));
> 18309	      RETURN (RECUR (TREE_OPERAND (t, 0)));
>
> Assuming I'm correctly understanding the above, I'm not sure what the
> best solution is.
>
> Some ideas:

> * don't add location wrappers if processing a template
>
> * introduce a new tree node for location wrappers (gah)
>
> * something I haven't thought of

Add a flag on the VIEW_CONVERT/NON_LVALUE expr explicitly noting its 
wrapperness (rather than infer it from TREE_TYPE == TREE_TYPE 
(TREE_OPERAND)).  TREE_LANG_FLAG_0 looks available?

nathan


--
Nathan Sidwell