Re: [RFA] Implement DW_TAG_*_parameter_pack

[Jason Merrill <jason at redhat dot com>]

On 09/16/2009 02:04 AM, Dodji Seketeli wrote:
> +  /* Gets the list of parameter packs of a generic function instantiation.
> +     The returned list is a TREE_LIST in which each TREE_PURPOSE contains
> +     a PARM_DECL that is a function parameter pack.
> +     Returns NULL if the function has no parameter pack.  */
> +  tree (*get_generic_function_parameter_packs) (const_tree);

This hook isn't ever used, so it can go away.

> +    if (! function_parameter_pack_p (param_decl)
> +       || ! function_parameter_pack_p (pack))
> +      return false;

Is FUNCTION_PARAMETER_PACK_P really true for a PARM_DECL generated from 
a function parameter pack?  That seems wrong.

> +         /* Elements of a function argument pack should not have
> +            a DW_AT_name attribute.  */
> +         if (!lang_hooks.function_parameter_pack_p (node))
> +           add_name_and_src_coords_attributes (parm_die, node);

Here too.  And in any case, for this test I'd rather use a flag passed 
in like you do for generic_parameter_die.

> +      if (generic_decl_parm)
> +      else
> +       /* When generating DIEs, generate the unspecified_parameters DIE
> +          instead if we come across the arg "__builtin_va_alist" */

Why not check for __builtin_va_alist in templates, too?  Can we keep 
this as a single loop rather than two?

And a few formatting nits:

> +function_parameter_expanded_from_pack_p (tree param_decl, tree pack)
> +{
> +

Unnecessary blank line.

> +    /* OK, this is a hack. We know that if PARAM_DECL is expanded from PACK,
> +       its name (build with make_ith_pack_parameter_name is the strin "<foo>#N"
> +       where <foo> is the name of PACK and N is a number.
> +       So here, we just check that naming pattern. Is there a better way ?  */

Typo "string", extra space before ?

And no, there isn't currently a better way.

> +       && IDENTIFIER_LENGTH (DECL_NAME (pack))
> +          < IDENTIFIER_LENGTH (DECL_NAME (param_decl))
> +       && IDENTIFIER_POINTER
> +           (DECL_NAME (param_decl))[IDENTIFIER_LENGTH (DECL_NAME (pack))]
> +          == '#')

Add parens to enforce indentation, i.e.

&& (IDENTIFIER_LENGTH (DECL_NAME (pack))
    < IDENTIFIER_LENGTH (DECL_NAME (param_decl)))

Jason