From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 27662 invoked by alias); 18 May 2011 17:46:22 -0000 Received: (qmail 27651 invoked by uid 22791); 18 May 2011 17:46:20 -0000 X-SWARE-Spam-Status: No, hits=-6.3 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 18 May 2011 17:46:03 +0000 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p4IHjrXT025590 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 18 May 2011 13:45:54 -0400 Received: from [127.0.0.1] (ovpn-113-120.phx2.redhat.com [10.3.113.120]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p4IHjqk3017558; Wed, 18 May 2011 13:45:53 -0400 Message-ID: <4DD405D0.4090108@redhat.com> Date: Wed, 18 May 2011 18:36:00 -0000 From: Jason Merrill User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110428 Fedora/3.1.10-1.fc14 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: Nathan Froyd CC: gcc-patches@gcc.gnu.org Subject: Re: [PATCH,c++] describe reasons for function template overload resolution failure References: <20110509224933.GR23480@codesourcery.com> <4DC99B34.7070301@redhat.com> <20110516173938.GA779@nightcrawler> In-Reply-To: <20110516173938.GA779@nightcrawler> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2011-05/txt/msg01302.txt.bz2 > 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 S and ARG is S. > 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 (loc, > + " cannot instantiate member function templates to " > + "copy class objects to their class type"); The standardese is misleading here (and is fixed in C++11); you certainly can instantiate a constructor template to copy an object of the same type. The real problem is having a constructor taking a single parameter with the type of the class. > if (TEMPLATE_TYPE_LEVEL (parm) > != template_decl_level (tparm)) > /* The PARM is not one we're trying to unify. Just check > to see if it matches ARG. */ > /* FIXME: What to return here? */ > return (TREE_CODE (arg) == TREE_CODE (parm) > && same_type_p (parm, arg)) ? 0 : 1; unify_type_mismatch seems appropriate here. And unify_success, of course. > if (TEMPLATE_PARM_LEVEL (parm) > != template_decl_level (tparm)) > /* The PARM is not one we're trying to unify. Just check > to see if it matches ARG. */ > return !(TREE_CODE (arg) == TREE_CODE (parm) > && cp_tree_equal (parm, arg)); No diagnostic code here in case of mismatch? > idx = TEMPLATE_PARM_IDX (parm); > targ = TREE_VEC_ELT (INNERMOST_TEMPLATE_ARGS (targs), idx); > > if (targ) > return !cp_tree_equal (targ, arg); Seems like you're missing unify_inconsistency here. > + case ur_parameter_pack_mismatch: > + inform (loc, " template parmeter %qD is not a parameter pack", > + ui->u.mismatch.parm); > + break; This message should indicate that this is a compiler defect > 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 (TREE_CODE (arg_max) != MINUS_EXPR) > return 1; No diagnostic code here? > case TREE_VEC: > { > int i; > if (TREE_CODE (arg) != TREE_VEC) > return 1; > if (TREE_VEC_LENGTH (parm) != TREE_VEC_LENGTH (arg)) > return 1; Or here? > /* An unresolved overload is a nondeduced context. */ > if (type_unknown_p (parm)) > return 0; And this should be unify_success. > 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. Jason