From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1772 invoked by alias); 18 May 2011 19:00:22 -0000 Received: (qmail 1737 invoked by uid 22791); 18 May 2011 19:00:20 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 18 May 2011 19:00:05 +0000 Received: (qmail 27952 invoked from network); 18 May 2011 19:00:05 -0000 Received: from unknown (HELO ?192.168.1.78?) (froydnj@127.0.0.2) by mail.codesourcery.com with ESMTPA; 18 May 2011 19:00:05 -0000 Message-ID: <4DD41733.2020501@codesourcery.com> Date: Wed, 18 May 2011 20:04:00 -0000 From: Nathan Froyd User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0a2) Gecko/20110505 Thunderbird/3.3a4pre MIME-Version: 1.0 To: Jason Merrill 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> <4DD405D0.4090108@redhat.com> In-Reply-To: <4DD405D0.4090108@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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/msg01315.txt.bz2 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 (); to something like: if (complain & tf_issue_diagnostic) emit_diagnostic (complain & tf_note ? DK_NOTE : DK_ERROR, ); 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? -Nathan