Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill <jason@redhat.com>]

On 12/03/2012 08:27 AM, Dodji Seketeli wrote:
> +	- HAS_EXPANSION_ARG_P: Set to TRUE iff at least one parameter
> +	  pack has got an argument that is an expansion.

The "got" is unnecessary, just "has an argument" is better.

> +   Setup APS, which is an instance of an ARGUMENT_PACK_SELECT tree, so
> +   that it selects the Ith element out of the argument pack ARG_PACK.
> +   If the Ith element is a pack expansion, then just select its
> +   pattern.  Otherwise, select the whole element.

I wonder if, rather than set up a temporary pack at this point, it makes 
sense to look through pack expansions when we use an 
ARGUMENT_PACK_SELECT.  Is there any case where we actually want an 
ARGUMENT_PACK_SELECT to be an expansion?

> +  /* If we have one parameter pack whose matching argument pack is
> +     just what template_parm_to_arg returned when passed the
> +     parameter pack, or if we only have empty arguments ... */
> +  else if (arg_from_pack_level_to_prune || has_empty_arg)
> +    {
> +      /* ... we just return a pack expansion which pattern is PATTERN
> +	 into which ARGS has been substituted.  */
> +      *instantiation_yields_no_list_p = true;
> +    }

I was thinking we wouldn't need to recognize this case specifically, 
that the code following it would work the way we want.  If callers get a 
vector with a single pack expansion element rather than just a pack 
expansion, is that a problem?  Alternately, if len == 1, maybe we should 
always just return the single element.

> +  /* We could not find any argument packs that work, so we'll just
> +     return an unsubstituted pack expansion.  The caller must be
> +     prepared to deal with this.  */
>    if (len < 0)
> -    return error_mark_node;
> +    len = 1;

Why this change?  Why is returning error_mark_node no longer the right 
thing to do?

Jason