Re: [PATCH,c++] describe reasons for function template overload resolution failure
On 05/26/2011 03:04 PM, Nathan Froyd wrote:
Thanks, this is looking pretty close. A few more issues, mostly to do
with wording of the diagnostics:
@@ -14084,7 +14350,7 @@ fn_type_unification (tree fn,
sarg = tree_cons (NULL_TREE, TREE_TYPE (substed), sarg);
for (i = 0; i nargs sarg; ++i, sarg = TREE_CHAIN (sarg))
if (!same_type_p (args[i], TREE_VALUE (sarg)))
- return 1;
+ return unify_unknown (explain_p);
Here the problem is that substituting in the deduced template arguments
didn't give us the result we were looking for; the diagnostic should
print the substitution result and the desired result.
if (resolve_overloaded_unification
- (tparms, targs, parm, arg, strict, sub_strict))
+ (tparms, targs, parm, arg, strict, sub_strict, explain_p))
continue;
- return 1;
+ return unify_unknown (explain_p);
Here the problem is that deduction is unable to resolve the address of
an overloaded function.
+ inform (input_location,
+ template parameter %qD is not a parameter pack,
+ parm);
+ inform (input_location, please report this as a bug);
Actually, I was wrong; this isn't a bug. 14.8.2.4/8 says, If A was
transformed from a function parameter pack and P is not a parameter
pack, type deduction fails.
But let's print the argument as well in this case.
+unify_ptrmem_cst_mismatch (bool explain_p, tree parm, tree arg)
+{
+ if (explain_p)
+inform (input_location,
+ mismatched pointer-to-memory constants %qE and %qE,
+ parm, arg);
Here the problem is that arg is not a pointer-to-member constant at all
(note also member, not memory).
if (TREE_CODE (arg) != INTEGER_CST)
- return 1;
+ return unify_constant_mismatch (explain_p, parm, arg);
This could end up complaining that arg isn't constant when the problem
is that it's the wrong kind of constant.
You might combine unify_ptrmem_cst_mismatch, unify_constant_mismatch,
and unify_constant_unequal to just say that arg is not a constant equal
to parm. Or just use unify_template_argument_mismatch.
+unify_expression_unequal (bool explain_p, tree parm, tree arg)
+{
+ if (explain_p)
+inform (input_location, %qE is not equal to %qE, parm, arg);
This should say equivalent rather than equal.
+ portions of parameter pack %qT could not be deduced,
The problem here is that portions of the pack were deduced to different
values, like unify_inconsistency but with packs. Let's print out the
conflicting values.
+ method type %qT is not a valid template argument, arg);
member function type
+unify_too_many_parameters (bool explain_p, int have, int wanted)
+unify_too_few_parameters (bool explain_p, int have, int wanted)
s/parameters/arguments/
if (strict != DEDUCE_EXACT
can_convert_arg (parm, type, TYPE_P (arg) ? NULL_TREE : arg,
flags))
continue;
return unify_arg_conversion (explain_p, parm, type, arg);
In the DEDUCE_EXACT case, let's use unify_type_mismatch.
!template_template_parm_bindings_ok_p
(DECL_INNERMOST_TEMPLATE_PARMS (fn), targs))
return unify_inconsistent_template_template_parameters (explain_p);
Here the problem is that the template parameters of a template template
argument are inconsistent with other deduced template arguments, as
illustrated in the example just above this snippet. Ideally here we'd
print the parameter types of the template template argument and the
parameter types of the template template parameter after substituting in
the other template arguments. I'm not going to insist on that now, but
let's be a bit more verbose than inconsistent template template
parameters.
TREE_CODE (tparm) != TYPE_DECL)
|| (TREE_CODE (parm) == TEMPLATE_TEMPLATE_PARM
TREE_CODE (tparm) != TEMPLATE_DECL))
- return 1;
+ return unify_template_parameter_mismatch (explain_p,
+ parm, tparm);
Looking more closely, I have no idea how this could happen. It seems to
hypothesize a template type parameter that isn't a type parameter, or a
template template parameter that isn't a template template parameter.
Let's try asserting that this can't happen.
converted_args
= (coerce_template_parms (tparms, explicit_targs, NULL_TREE, tf_none,
/*require_all_args=*/false,
/*use_default_args=*/false));
if (converted_args == error_mark_node)
- return 1;
+ {
+ /* Run it for diagnostics. */
+ if (explain_p)
+ coerce_template_parms (tparms, explicit_targs, NULL_TREE,
+ tf_warning_or_error,
+
Re: [PATCH,c++] describe reasons for function template overload resolution failure
On 05/27/2011 04:22 PM, Nathan Froyd wrote: + /* The PARM is not one we're trying to unify. Just check +to see if it matches ARG. */ + int result = !(TREE_CODE (arg) == TREE_CODE (parm) + cp_tree_equal (parm, arg)); + if (result) + unify_template_parameter_mismatch (explain_p, parm, tparm); This should be unify_template_argument_mismatch (explain_p, parm, arg); So we can drop unify_template_parameter_mismatch. OK with that change. Jason
Re: [PATCH,c++] describe reasons for function template overload resolution failure
On 05/25/2011 02:15 PM, Nathan Froyd wrote: The patch below implements just such an idea. The only twist is that the `explain' parameter is actually a `location_t *' so that when we provide explanations that aren't produced via tf_warning_or_error blocks, the explanations are attached to the template itself rather than the call, as would be done with input_location. Requests for non-explanation are accomplished with a NULL argument. I'm not totally attached to this; it's almost too cute, and since tf_warning_or_error messsages tend to be attached to the call anyway, it's arguably inconsistent. Yes, I think let's drop this bit and just use the call location. Ideally, for unification failures we'd give the source location of the problematic argument (which we don't always have because it might be a plain DECL) and for substitution failures we'd give the source location of the expression that substitution failed on (for which we could adjust input_location in tsubst_*). I didn't see what you intended by reconstructing the arguments, so I implemented the more straightforward saving. I was thinking that we could try to add the candidate again, but this is fine. Jason
Re: [PATCH,c++] describe reasons for function template overload resolution failure
Thanks for the background; I will keep the principle in mind. IMHO, in
a case like this where we're logically printing one diagnostic (one
error and then some number of explanatory notes) keeping all the logic
for the diagnostic centralized makes more sense.
I understand, but that means we have to create a whole data structure to
try and preserve information about the failure, and either having to
duplicate every possible error or give less informative messages. I
feel even more strongly about this after looking more closely at your patch.
+case ur_invalid:
+ inform (loc,
+ template argument deduction attempted with invalid input);
+ break;
In ur_invalid cases, we should have had an earlier error message
already, so giving an extra message here seems kind of redundant.
+ types %qT and %qT differ in their qualifiers,
Let's say ...have incompatible cv-qualifiers, since some differences
are OK.
+ inform (loc, variable-sized array type %qT is not permitted,
...is not a valid template argument
+ inform (loc, %qT is not derived from %qT,
This could be misleading, since we can also fail when the deduction is
ambiguous.
+ inform (loc, %qE is not a valid pointer-to-member of type %qT,
This needs to say pointer-to-member constant, not just
pointer-to-member.
+case ur_parameter_deduction_failure:
+ inform (loc, couldn't deduce template argument %qD, ui-u.parm);
+ break;
It seems like you're using this both for cases where unification
succeeded but just didn't produce template arguments for all parameters,
and for cases where unification failed for some reason; this message
should only apply to the first case.
if (TREE_PURPOSE (TREE_VEC_ELT (tparms, i)))
{
tree parm = TREE_VALUE (TREE_VEC_ELT (tparms, i));
tree arg = TREE_PURPOSE (TREE_VEC_ELT (tparms, i));
arg = tsubst_template_arg (arg, targs, tf_none, NULL_TREE);
arg = convert_template_argument (parm, arg, targs, tf_none,
i, NULL_TREE, ui);
if (arg == error_mark_node)
return unify_parameter_deduction_failure (ui, parm);
In this case, the problem is that we tried to use the default template
argument but it didn't work for some reason; we should say that, not
just say we didn't deduce something, or the users will say but there's
a default argument!.
In this case, we should do the substitution again with
tf_warning_or_error so the user can see what the problem actually is,
not just say that there was some unspecified problem.
- return 2;
+ return unify_parameter_deduction_failure (ui, tparm);
This seems like the only place we actually want to use
unify_parameter_deduction_failure.
/* Check for mixed types and values. */
if ((TREE_CODE (parm) == TEMPLATE_TYPE_PARM
TREE_CODE (tparm) != TYPE_DECL)
|| (TREE_CODE (parm) == TEMPLATE_TEMPLATE_PARM
TREE_CODE (tparm) != TEMPLATE_DECL))
return unify_parameter_deduction_failure (ui, parm);
This is a type/template mismatch issue that deserves a more helpful
diagnostic.
/* ARG must be constructed from a template class or a template
template parameter. */
if (TREE_CODE (arg) != BOUND_TEMPLATE_TEMPLATE_PARM
!CLASSTYPE_SPECIALIZATION_OF_PRIMARY_TEMPLATE_P (arg))
return unify_parameter_deduction_failure (ui, parm);
This is saying that we can't deduce a template from a non-template type.
/* If the argument deduction results is a METHOD_TYPE,
then there is a problem.
METHOD_TYPE doesn't map to any real C++ type the result of
the deduction can not be of that type. */
if (TREE_CODE (arg) == METHOD_TYPE)
return unify_parameter_deduction_failure (ui, parm);
Like with the VLA case, the problem here is deducing something that
isn't a valid template type argument.
/* We haven't deduced the type of this parameter yet. Try again
later. */
return unify_success (ui);
else
return unify_parameter_deduction_failure (ui, parm);
Here the problem is a type mismatch between parm and arg for a non-type
template argument.
/* Perhaps PARM is something like SU and ARG is Sint.
Then, we should unify `int' and `U'. */
t = arg;
else
/* There's no chance of unification succeeding. */
return unify_parameter_deduction_failure (ui, parm);
This should be type_mismatch.
case FIELD_DECL:
case TEMPLATE_DECL:
/* Matched cases are handled by the ARG == PARM test above. */
return unify_parameter_deduction_failure (ui, parm);
Another case where we should talk about the arg/parm mismatch.
+ case rr_invalid_copy:
+ inform
Re: [PATCH,c++] describe reasons for function template overload resolution failure
On 05/18/2011 01:45 PM, Jason Merrill wrote:
Thanks for the background; I will keep the principle in mind. IMHO, in
a case like this where we're logically printing one diagnostic (one
error and then some number of explanatory notes) keeping all the logic
for the diagnostic centralized makes more sense.
I understand, but that means we have to create a whole data structure to try
and preserve information about the failure, and either having to duplicate
every possible error or give less informative messages. I feel even more
strongly about this after looking more closely at your patch.
Thank you for the review. I'll go back and try things the way you suggest;
before I go off and do that, I've taken your comments to mean that:
- fn_type_unification/type_unification_real and associated callers should take
a boolean `explain' parameter, which is normally false;
- failed calls to fn_type_unification should save the arguments for the call
for future explanation;
- printing diagnostic messages should call fn_type_unification with the saved
arguments and a true `explain' parameter.
This is similar to passing `struct unification_info' and really only involves
shuffling code from call.c into the unify_* functions in pt.c and some minor
changes to the rejection_reason code in call.c. The only wrinkle I see is
that in cases like these:
if (TREE_PURPOSE (TREE_VEC_ELT (tparms, i)))
{
tree parm = TREE_VALUE (TREE_VEC_ELT (tparms, i));
tree arg = TREE_PURPOSE (TREE_VEC_ELT (tparms, i));
arg = tsubst_template_arg (arg, targs, tf_none, NULL_TREE);
arg = convert_template_argument (parm, arg, targs, tf_none,
i, NULL_TREE, ui);
if (arg == error_mark_node)
return unify_parameter_deduction_failure (ui, parm);
In this case, the problem is that we tried to use the default template
argument but it didn't work for some reason; we should say that, not just say
we didn't deduce something, or the users will say but there's a default
argument!.
In this case, we should do the substitution again with tf_warning_or_error so
the user can see what the problem actually is, not just say that there was
some unspecified problem.
if (coerce_template_parms (parm_parms,
full_argvec,
TYPE_TI_TEMPLATE (parm),
tf_none,
/*require_all_args=*/true,
/*use_default_args=*/false, ui)
== error_mark_node)
return 1;
Rather than pass ui down into coerce_template_parms we should just note when
it fails and run it again at diagnostic time.
converted_args
= (coerce_template_parms (tparms, explicit_targs, NULL_TREE, tf_none,
/*require_all_args=*/false,
/*use_default_args=*/false, ui));
if (converted_args == error_mark_node)
return 1;
Here too.
if (fntype == error_mark_node)
return unify_substitution_failure (ui);
And this should remember the arguments so we can do the tsubst again at
diagnostic time.
and other bits of pt.c, I'm interpreting your suggestions to mean that
tf_warning_or_error should be passed if `explain' is true. That doesn't seem
like the best interface for diagnostics, as we'll get:
foo.cc:105:40 error: no matching function for call to bar (...)
foo.cc:105:40 note: candidates are:
bar.hh:7000:30 note: bar (...)
bar.hh:7000:30 note: [some reason]
bar.hh:4095:63 note: bar (...)
bar.hh:... error: [some message from tf_warning_or_error code]
I'm not sure that the last location there will necessary be the same as the
one that's printed for the declaration. I think I'll punt on that issue for
the time being until we see how the diagnostics work out. There's also the
matter of the error vs. note diagnostic. I think it'd be nicer to keep the
conformity of a note for all the explanations; the only way I see to do that
is something like:
- Add a tf_note flag; pass it at all appropriate call sites when explaining
things;
- Add a tf_issue_diagnostic flag that's the union of tf_{warning,error,note};
- Change code that looks like:
if (complain tf_warning_or_error)
error (STUFF);
to something like:
if (complain tf_issue_diagnostic)
emit_diagnostic (complain tf_note ? DK_NOTE : DK_ERROR, STUFF);
passing input_location if we're not already passing a location.
That involves a lot of code churn. (Not a lot if you just modified the
functions above, but with this scheme, you'd have to call instantiate_template
again from the diagnostic code, and I assume you'd want to call that with
tf_note as well, which means hitting a lot more code.) I don't see a better
way
Re: [PATCH,c++] describe reasons for function template overload resolution failure
On 05/18/2011 03:00 PM, Nathan Froyd wrote:
Thank you for the review. I'll go back and try things the way you suggest;
before I go off and do that, I've taken your comments to mean that:
- fn_type_unification/type_unification_real and associated callers should take
a boolean `explain' parameter, which is normally false;
- failed calls to fn_type_unification should save the arguments for the call
for future explanation;
- printing diagnostic messages should call fn_type_unification with the saved
arguments and a true `explain' parameter.
Yes, that's what I had in mind. Though I think you can reconstruct the
arguments rather than save them.
...
bar.hh:4095:63 note: bar (...)
bar.hh:... error: [some message from tf_warning_or_error code]
I'm not sure that the last location there will necessary be the same as the
one that's printed for the declaration. I think I'll punt on that issue for
the time being until we see how the diagnostics work out. There's also the
matter of the error vs. note diagnostic. I think it'd be nicer to keep the
conformity of a note for all the explanations
Nicer, yes, but I think that's a secondary concern after usefulness of
the actual message. In similar cases I've introduced the errors with
another message like %qD is implicitly deleted because the default
definition would be ill-formed:
Or, in this case, deduction failed because substituting the template
arguments would be ill-formed:
; the only way I see to do that
is something like:
- Add a tf_note flag; pass it at all appropriate call sites when explaining
things;
- Add a tf_issue_diagnostic flag that's the union of tf_{warning,error,note};
- Change code that looks like:
if (complain tf_warning_or_error)
error (STUFF);
to something like:
if (complain tf_issue_diagnostic)
emit_diagnostic (complain tf_note ? DK_NOTE : DK_ERROR,STUFF);
passing input_location if we're not already passing a location.
That involves a lot of code churn. (Not a lot if you just modified the
functions above, but with this scheme, you'd have to call instantiate_template
again from the diagnostic code, and I assume you'd want to call that with
tf_note as well, which means hitting a lot more code.) I don't see a better
way to keep the diagnostics uniform, but I might be making things too
complicated; did you have a different idea of how to implement what you were
suggesting?
That all makes sense, but I'd put it in a follow-on patch. And wrap the
complexity in a cp_error function that takes a complain parameter and
either gives no message, a note, or an error depending.
Jason
Re: [PATCH,c++] describe reasons for function template overload resolution failure
On 05/09/2011 06:49 PM, Nathan Froyd wrote: The patch below is an updated version of: http://gcc.gnu.org/ml/libstdc++/2011-02/msg9.html Sorry I didn't respond to that message. In general, my preference is to have diagnostics collocated with the tests that lead to them, and just run through the same code a second time if we decide that we want to give a diagnostic. For instance, that's what I did in joust and synthesized_method_walk. Saving information ahead of time about why we reject a particular candidate creates overhead that is useless in the vast majority of cases, since most compiles succeed. That said, I'm not going to reject this patch over this issue, but please keep this principle in mind with future diagnostic improvement patches. +case ur_invalid_arg: +case ur_invalid_parm: +case ur_invalid_init_list: +case ur_invalid_template_parm: + inform (loc, an error occurred during template argument deduction); These are all cases of bailing out when we encounter an error_mark_node that indicates an earlier error from an ill-formed expression or whatever. I don't see any reason to have separate cases for them, and the message is misleading; the error didn't occur during deduction, it occurred before we got to deduction. We just can't do deduction on ill-formed inputs. +case ur_type_mismatch: +case ur_pointer_mismatch: +case ur_reference_mismatch: + inform (loc,mismatched types %qT and %qT, Why make these different? They are all cases of the argument and parameter types not having the same form. +case ur_expanding_pack_to_fixed_arg_list: This is never generated anywhere. And the message in coerce_template_parms is a sorry to indicate that this is a bug in G++, not a problem with the user's code. unify_failure (struct unification_info *ui, tree parm) ... return unify_failure (ui); This doesn't compile. Were you testing a previous version of the patch? I guess I'll stop reviewing the patch now and wait for an update. Jason
