This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Variadic templates, fourth revision (1/2)


Looks pretty good overall, but some things I notice:

Do we really need make_ith_pack_parameter_name? We can't just use the sub-parameters without naming them?

Why does find_parameter_packs_r look into template template parameters? I wouldn't expect naming a template template parameter which happens to have a parameter pack to qualify as naming a parameter pack.

The BASELINK, *_ARGUMENT_PACK, and CAST_EXPR cases seem like they belong in walk_tree rather than a helper function.

The "coarse granularity" you mention in the comment of check_for_bare_parameter_packs is full-expression (if we're dealing with an expression).

The way current_template_args makes a pack to hold the expansion seems a bit odd; I think I understand why it would simplify things, but can't you just treat an expansion as a pack in the appropriate places?

In coerce_template_parms, use the term "expansion" in the error message instead of "unpack".

In for_each_template_parm_r: again, the new code to handle pack and expansion should be in walk_tree rather than here.

The static variable outer_packs in tsubst_pack_expansion has to go; it isn't recursion-safe in case we need to push into another instantiation. Any information like this should be tracked in struct language_function.
Why is it be necessary, anyway? Surely any packs expanded by a nested expansion would be gone by the time we start expanding the outer expansion.


Another case of "unpack" in the error message.

tsubst_copy: "illegel" should be "invalid"

There are some coding conventions issues in your code: a few parentheses at the end of a line; in a ?: there should be a space before the ? and it should not be at the end of a line.

Why are ARGUMENT_PACK_INCOMPLETE_P and ARGUMENT_PACK_EXPLICIT_ARGS set on the ARGUMENT_PACK_ARGS rather than the pack itself?

The check at the end of more_specialized_fn seems useless; deduction should already have given the same result.

I'm not quite done reviewing the patch, but I wanted to send this off rather than wait yet another day.

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]