From patchwork Wed Nov 17 20:46:33 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Nathan Froyd X-Patchwork-Id: 71619 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 71B9CB71A4 for ; Thu, 18 Nov 2010 07:46:52 +1100 (EST) Received: (qmail 11442 invoked by alias); 17 Nov 2010 20:46:50 -0000 Received: (qmail 11424 invoked by uid 22791); 17 Nov 2010 20:46:47 -0000 X-SWARE-Spam-Status: No, hits=-0.8 required=5.0 tests=AWL, BAYES_40, TW_CX, 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, 17 Nov 2010 20:46:39 +0000 Received: (qmail 2200 invoked from network); 17 Nov 2010 20:46:34 -0000 Received: from unknown (HELO codesourcery.com) (froydnj@127.0.0.2) by mail.codesourcery.com with ESMTPA; 17 Nov 2010 20:46:34 -0000 Date: Wed, 17 Nov 2010 15:46:33 -0500 From: Nathan Froyd To: gcc-patches@gcc.gnu.org Cc: jason@redhat.com, mark@codesourcery.com, aaw@gcc.gnu.org Subject: [RFC,c++] fix PR 45329, provide information on overload resolution Message-ID: <20101117204631.GE24469@nightcrawler> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.20 (2009-06-14) X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org PR 45329 is a problem with GCC's overload resolution diagnostics. For the testcase: struct S { int x; int y; }; int foo(int x, int y) { return x+y; } int foo(const S &s) { return s.x + s.y; } int foo2(int x) { foo(x); } gcc prints: ./tst.cc: In function ‘int foo2(int)’: ./tst.cc:19:8: error: no matching function for call to ‘foo(int&)’ ./tst.cc:7:5: note: candidates are: int foo(int, int) ./tst.cc:12:5: note: int foo(const S&) and the bug reporter indicates it would be helpful if GCC provided some information about why the candidate functions did not match. The patch below addresses this. At each point where we decide that a candidate function loses, we record a rejection reason. When we print the candidates, we also print out the reasons, if any. This scheme only really deals with the "no matching function for" case; I can try to piece something together for the ambiguous match case, though that will be a bit harder. We don't keep track of multiple rejection reasons. It would be a trivial modification to do so if people felt that was worthwhile. There are two points at which I have not added rejection reasons. One of them--convert_class_to_reference--I have described in a comment. The second one is in add_function_candidate: /* Kludge: When looking for a function from a subobject while generating an implicit copy/move constructor/operator=, don't consider anything that takes (a reference to) an unrelated type. See c++/44909. */ else if (parmlist && ((flags & LOOKUP_SPECULATIVE) || (current_function_decl && DECL_DEFAULTED_FN (current_function_decl)))) { if (DECL_CONSTRUCTOR_P (fn)) i = 1; else if (DECL_ASSIGNMENT_OPERATOR_P (fn) && DECL_OVERLOADED_OPERATOR_P (fn) == NOP_EXPR) i = 2; else i = 0; if (i && len == i) { parmnode = chain_index (i-1, parmlist); if (!reference_related_p (non_reference (TREE_VALUE (parmnode)), ctype)) viable = 0; } I am clueless about what a decent reason is. Comments welcome. For the testcase above, a modified g++ prints: pr45329.C: In function 'int foo2(int)': pr45329.C:26:7: error: no matching function for call to 'foo(int&)' cc1plus: note: candidates are: pr45329.C:7:5: note: int foo(int, int) pr45329.C:7:5: note: candidate expects 2 arguments, 1 provided pr45329.C:19:5: note: int foo(const S&) pr45329.C:19:5: note: no known conversion for argument 1 from 'int' to 'const S&' which is a little friendlier. Part of the reason for asking for comments here is that moving the diagnostics around as above will require the modification of a bajillion testcases; before I embark on that, I'd like comments on the approach and whether the formatting seems reasonable. I moved the formatting around because leaving the formatting as before resulted in: pr45329.C: In function 'int foo2(int)': pr45329.C:26:7: error: no matching function for call to 'foo(int&)' pr45329.C:7:5: note: candidates are: int foo(int, int) pr45329.C:7:5: note: candidates are: candidate expects 2 arguments, 1 provided pr45329.C:19:5: note: int foo(const S&) pr45329.C:19:5: note: no known conversion for argument 1 from 'int' to 'const S&' which IMHO takes up a lot of space for little gain. I suppose one obvious improvement to the proposed format might be something like: pr45329.C: In function 'int foo2(int)': pr45329.C:26:7: error: no matching function for call to 'foo(int&)' cc1plus: note: candidates are: pr45329.C:7:5: note: int foo(int, int) pr45329.C:note: candidate expects 2 arguments, 1 provided pr45329.C:19:5: note: int foo(const S&) pr45329.C: note: no known conversion for argument 1 from 'int' to 'const S&' though I wouldn't be thrilled about dg-message matching that. I have gone through testlogs from check-g++ and the messages look reasonable. Comments? Concerns? Ollie, you reported the bug; what do you think about the changes? -Nathan gcc/cp/ PR c++/45329 * cp-tree.h (location_at): Declare. * error.c (location_at): Export. * call.c (struct conversion): Document bad_p field. (enum rejection_reason_code): Define. (struct rejection_reason): Define. (struct z_candidate): Add `reason' field. (add_candidate): Add `reason' parameter. Store it in CAND. (alloc_rejection, arity_rejection, arg_conversion_rejection): New functions. (bad_arg_conversion_rejection): New function. (convert_class_to_reference): Add comment. (add_function_candidate): Record rejection reason and pass it to add_candidate. (add_conv_candidate, build_builtin_candidate): Likewise. (add_template_candidate_real): Likewise. (print_z_candidate): Print CAND->REASON if it exists. Adjust diagnostic strings. (print_z_candidates): Adjust calling sequence for print_z_candidate. Print header line directly. (build_user_type_conversion_1): Add reason for rejection to CAND. diff --git a/gcc/cp/call.c b/gcc/cp/call.c index eb7247d..c69126d 100644 --- a/gcc/cp/call.c +++ b/gcc/cp/call.c @@ -84,6 +84,9 @@ struct conversion { BOOL_BITFIELD user_conv_p : 1; BOOL_BITFIELD ellipsis_p : 1; BOOL_BITFIELD this_p : 1; + /* True if this conversion would be permitted with a bending of + language standards, e.g. disregarding pointer qualifiers or + converting integers to pointers. */ BOOL_BITFIELD bad_p : 1; /* If KIND is ck_ref_bind ck_base_conv, true to indicate that a temporary should be created to hold the result of the @@ -129,6 +132,7 @@ struct conversion { static struct obstack conversion_obstack; static bool conversion_obstack_initialized; +struct rejection_reason; static struct z_candidate * tourney (struct z_candidate *); static int equal_functions (tree, tree); @@ -190,7 +194,7 @@ static conversion *maybe_handle_ref_bind (conversion **); static void maybe_handle_implicit_object (conversion **); static struct z_candidate *add_candidate (struct z_candidate **, tree, tree, const VEC(tree,gc) *, size_t, - conversion **, tree, tree, int); + conversion **, tree, tree, int, struct rejection_reason *); static tree source_type (conversion *); static void add_warning (struct z_candidate *, struct z_candidate *); static bool reference_compatible_p (tree, tree); @@ -416,6 +420,45 @@ struct candidate_warning { candidate_warning *next; }; +/* Information for providing diagnostics about why overloading failed. */ + +enum rejection_reason_code { + rr_none, + rr_arity, + rr_arg_conversion, + rr_bad_arg_conversion +}; + +struct rejection_reason { + enum rejection_reason_code code; + union { + /* Information about an arity mismatch. */ + struct { + /* The expected number of arguments. */ + int expected; + /* The actual number of arguments in the call. */ + int actual; + /* Whether the call was a varargs call. */ + bool call_varargs_p; + } arity; + /* Information about an argument conversion mismatch. */ + struct { + /* The index of the argument, 0-based. */ + int n_arg; + /* The type of the actual argument. */ + tree from_type; + /* The type of the formal argument. */ + tree to_type; + } conversion; + /* Same, but for bad argument conversions. */ + struct { + int n_arg; + tree from_type; + tree to_type; + } bad_conversion; + } u; +}; + struct z_candidate { /* The FUNCTION_DECL that will be called if this candidate is selected by overload resolution. */ @@ -437,6 +480,7 @@ struct z_candidate { type. */ conversion *second_conv; int viable; + struct rejection_reason *reason; /* If FN is a member function, the binfo indicating the path used to qualify the name of FN at the call site. This path is used to determine whether or not FN is accessible if it is selected by @@ -518,6 +562,46 @@ conversion_obstack_alloc (size_t n) return p; } +/* Allocate rejection reasons. */ + +static struct rejection_reason * +alloc_rejection (enum rejection_reason_code code) +{ + struct rejection_reason *p; + p = (struct rejection_reason *) conversion_obstack_alloc (sizeof *p); + p->code = code; + return p; +} + +static struct rejection_reason * +arity_rejection (int expected, int actual) +{ + struct rejection_reason *r = alloc_rejection (rr_arity); + r->u.arity.expected = expected; + r->u.arity.actual = actual; + return r; +} + +static struct rejection_reason * +arg_conversion_rejection (int n_arg, tree from, tree to) +{ + struct rejection_reason *r = alloc_rejection (rr_arg_conversion); + r->u.conversion.n_arg = n_arg; + r->u.conversion.from_type = from; + r->u.conversion.to_type = to; + return r; +} + +static struct rejection_reason * +bad_arg_conversion_rejection (int n_arg, tree from, tree to) +{ + struct rejection_reason *r = alloc_rejection (rr_bad_arg_conversion); + r->u.bad_conversion.n_arg = n_arg; + r->u.bad_conversion.from_type = from; + r->u.bad_conversion.to_type = to; + return r; +} + /* Dynamically allocate a conversion. */ static conversion * @@ -1149,6 +1233,10 @@ convert_class_to_reference (tree reference_type, tree s, tree expr, int flags) if (TREE_CODE (t2) != REFERENCE_TYPE || !reference_compatible_p (t, TREE_TYPE (t2))) { + /* No need to set cand->reason here; this is most likely + an ambiguous match. If it's not, either this candidate + will win, or we will have identified a reason for it + losing already. */ cand->viable = 0; } else @@ -1562,7 +1650,7 @@ add_candidate (struct z_candidate **candidates, tree fn, tree first_arg, const VEC(tree,gc) *args, size_t num_convs, conversion **convs, tree access_path, tree conversion_path, - int viable) + int viable, struct rejection_reason *reason) { struct z_candidate *cand = (struct z_candidate *) conversion_obstack_alloc (sizeof (struct z_candidate)); @@ -1575,12 +1663,28 @@ add_candidate (struct z_candidate **candidates, cand->access_path = access_path; cand->conversion_path = conversion_path; cand->viable = viable; + cand->reason = reason; cand->next = *candidates; *candidates = cand; return cand; } +/* Return the number of remaining arguments in the parameter list + beginning with ARG. */ + +static int +remaining_arguments (tree arg) +{ + int n; + + for (n = 0; arg != NULL_TREE && arg != void_list_node; + arg = TREE_CHAIN (arg)) + n++; + + return n; +} + /* Create an overload candidate for the function or method FN called with the argument list FIRST_ARG/ARGS and add it to CANDIDATES. FLAGS is passed on to implicit_conversion. @@ -1603,6 +1707,7 @@ add_function_candidate (struct z_candidate **candidates, tree orig_first_arg = first_arg; int skip; int viable = 1; + struct rejection_reason *reason = NULL; /* At this point we should not see any functions which haven't been explicitly declared, except for friend functions which will have @@ -1643,11 +1748,19 @@ add_function_candidate (struct z_candidate **candidates, } if (i < len && parmnode) - viable = 0; + { + int remaining = remaining_arguments (parmnode); + viable = 0; + reason = arity_rejection (i + remaining, len); + } /* Make sure there are default args for the rest of the parms. */ else if (!sufficient_parms_p (parmnode)) - viable = 0; + { + int remaining = remaining_arguments (parmnode); + viable = 0; + reason = arity_rejection (i + remaining, len); + } /* Kludge: When looking for a function from a subobject while generating an implicit copy/move constructor/operator=, don't consider anything @@ -1684,7 +1797,7 @@ add_function_candidate (struct z_candidate **candidates, for (i = 0; i < len; ++i) { - tree arg, argtype; + tree arg, argtype, to_type; conversion *t; int is_this; @@ -1751,11 +1864,13 @@ add_function_candidate (struct z_candidate **candidates, t = implicit_conversion (parmtype, argtype, arg, /*c_cast_p=*/false, lflags); + to_type = parmtype; } else { t = build_identity_conv (argtype, arg); t->ellipsis_p = true; + to_type = argtype; } if (t && is_this) @@ -1765,16 +1880,20 @@ add_function_candidate (struct z_candidate **candidates, if (! t) { viable = 0; + reason = arg_conversion_rejection (i, argtype, to_type); break; } if (t->bad_p) - viable = -1; + { + viable = -1; + reason = bad_arg_conversion_rejection (i, argtype, to_type); + } } out: return add_candidate (candidates, fn, orig_first_arg, args, len, convs, - access_path, conversion_path, viable); + access_path, conversion_path, viable, reason); } /* Create an overload candidate for the conversion function FN which will @@ -1798,6 +1917,7 @@ add_conv_candidate (struct z_candidate **candidates, tree fn, tree obj, int i, len, viable, flags; tree parmlist, parmnode; conversion **convs; + struct rejection_reason *reason; for (parmlist = totype; TREE_CODE (parmlist) != FUNCTION_TYPE; ) parmlist = TREE_TYPE (parmlist); @@ -1808,6 +1928,7 @@ add_conv_candidate (struct z_candidate **candidates, tree fn, tree obj, parmnode = parmlist; viable = 1; flags = LOOKUP_IMPLICIT; + reason = NULL; /* Don't bother looking up the same type twice. */ if (*candidates && (*candidates)->fn == totype) @@ -1815,7 +1936,7 @@ add_conv_candidate (struct z_candidate **candidates, tree fn, tree obj, for (i = 0; i < len; ++i) { - tree arg, argtype; + tree arg, argtype, convert_type = NULL_TREE; conversion *t; if (i == 0) @@ -1828,17 +1949,24 @@ add_conv_candidate (struct z_candidate **candidates, tree fn, tree obj, argtype = lvalue_type (arg); if (i == 0) - t = implicit_conversion (totype, argtype, arg, /*c_cast_p=*/false, - flags); + { + t = implicit_conversion (totype, argtype, arg, /*c_cast_p=*/false, + flags); + convert_type = totype; + } else if (parmnode == void_list_node) break; else if (parmnode) - t = implicit_conversion (TREE_VALUE (parmnode), argtype, arg, - /*c_cast_p=*/false, flags); + { + t = implicit_conversion (TREE_VALUE (parmnode), argtype, arg, + /*c_cast_p=*/false, flags); + convert_type = TREE_VALUE (parmnode); + } else { t = build_identity_conv (argtype, arg); t->ellipsis_p = true; + convert_type = argtype; } convs[i] = t; @@ -1846,7 +1974,10 @@ add_conv_candidate (struct z_candidate **candidates, tree fn, tree obj, break; if (t->bad_p) - viable = -1; + { + viable = -1; + reason = bad_arg_conversion_rejection (i, argtype, convert_type); + } if (i == 0) continue; @@ -1856,13 +1987,21 @@ add_conv_candidate (struct z_candidate **candidates, tree fn, tree obj, } if (i < len) - viable = 0; + { + int remaining = remaining_arguments (parmnode); + viable = 0; + reason = arity_rejection (i + remaining, len); + } if (!sufficient_parms_p (parmnode)) - viable = 0; + { + int remaining = remaining_arguments (parmnode); + viable = 0; + reason = arity_rejection (i + remaining, len); + } return add_candidate (candidates, totype, first_arg, arglist, len, convs, - access_path, conversion_path, viable); + access_path, conversion_path, viable, reason); } static void @@ -1875,6 +2014,7 @@ build_builtin_candidate (struct z_candidate **candidates, tree fnname, size_t num_convs; int viable = 1, i; tree types[2]; + struct rejection_reason *reason = NULL; types[0] = type1; types[1] = type2; @@ -1902,9 +2042,13 @@ build_builtin_candidate (struct z_candidate **candidates, tree fnname, viable = 0; /* We need something for printing the candidate. */ t = build_identity_conv (types[i], NULL_TREE); + reason = arg_conversion_rejection (i, argtypes[i], types[i]); } else if (t->bad_p) - viable = 0; + { + viable = 0; + reason = bad_arg_conversion_rejection (i, argtypes[i], types[i]); + } convs[i] = t; } @@ -1918,14 +2062,18 @@ build_builtin_candidate (struct z_candidate **candidates, tree fnname, if (t) convs[0] = t; else - viable = 0; + { + viable = 0; + reason = arg_conversion_rejection (0, argtypes[2], + boolean_type_node); + } } add_candidate (candidates, fnname, /*first_arg=*/NULL_TREE, /*args=*/NULL, num_convs, convs, /*access_path=*/NULL_TREE, /*conversion_path=*/NULL_TREE, - viable); + viable, reason); } static bool @@ -2577,6 +2725,7 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl, struct z_candidate *cand; int i; tree fn; + struct rejection_reason *reason = NULL; /* We don't do deduction on the in-charge parameter, the VTT parameter or 'this'. */ @@ -2695,7 +2844,7 @@ add_template_candidate_real (struct z_candidate **candidates, tree tmpl, return cand; fail: return add_candidate (candidates, tmpl, first_arg, arglist, nargs, NULL, - access_path, conversion_path, 0); + access_path, conversion_path, 0, reason); } @@ -2816,38 +2965,73 @@ equal_functions (tree fn1, tree fn2) static void print_z_candidate (const char *msgstr, struct z_candidate *candidate) { + const char *msg = (msgstr == NULL + ? "" + : ACONCAT ((msgstr, " ", NULL))); + if (TREE_CODE (candidate->fn) == IDENTIFIER_NODE) { if (candidate->num_convs == 3) - inform (input_location, "%s %D(%T, %T, %T) ", msgstr, candidate->fn, + inform (input_location, "%s%D(%T, %T, %T) ", msg, candidate->fn, candidate->convs[0]->type, candidate->convs[1]->type, candidate->convs[2]->type); else if (candidate->num_convs == 2) - inform (input_location, "%s %D(%T, %T) ", msgstr, candidate->fn, + inform (input_location, "%s%D(%T, %T) ", msg, candidate->fn, candidate->convs[0]->type, candidate->convs[1]->type); else - inform (input_location, "%s %D(%T) ", msgstr, candidate->fn, + inform (input_location, "%s%D(%T) ", msg, candidate->fn, candidate->convs[0]->type); } else if (TYPE_P (candidate->fn)) - inform (input_location, "%s %T ", msgstr, candidate->fn); + inform (input_location, "%s%T ", msg, candidate->fn); else if (candidate->viable == -1) - inform (input_location, "%s %+#D ", msgstr, candidate->fn); + inform (input_location, "%s%+#D ", msg, candidate->fn); else if (DECL_DELETED_FN (STRIP_TEMPLATE (candidate->fn))) - inform (input_location, "%s %+#D ", msgstr, candidate->fn); + inform (input_location, "%s%+#D ", msg, candidate->fn); else - inform (input_location, "%s %+#D", msgstr, candidate->fn); + inform (input_location, "%s%+#D", msg, candidate->fn); + /* Give the user some information about why this candidate failed. */ + if (candidate->reason != NULL) + { + struct rejection_reason *r = candidate->reason; + location_t loc = location_of (candidate->fn); + + switch (r->code) + { + case rr_arity: + inform_n (loc, r->u.arity.expected, + "candidate expects %d argument, %d provided", + "candidate expects %d arguments, %d provided", + r->u.arity.expected, r->u.arity.actual); + break; + case rr_arg_conversion: + inform (loc, + "no known conversion for argument %d from %qT to %qT", + r->u.conversion.n_arg+1, r->u.conversion.from_type, + r->u.conversion.to_type); + break; + case rr_bad_arg_conversion: + inform (loc, + "no known conversion for argument %d from %qT to %qT", + r->u.bad_conversion.n_arg+1, r->u.bad_conversion.from_type, + r->u.bad_conversion.to_type); + break; + case rr_none: + default: + /* This candidate didn't have any issues or we failed to + handle a particular code. Either way... */ + gcc_unreachable (); + } + } } static void print_z_candidates (struct z_candidate *candidates) { - const char *str; struct z_candidate *cand1; struct z_candidate **cand2; - char *spaces; if (!candidates) return; @@ -2889,14 +3073,10 @@ print_z_candidates (struct z_candidate *candidates) } } - str = candidates->next ? _("candidates are:") : _("candidate is:"); - spaces = NULL; + inform (UNKNOWN_LOCATION, + candidates->next ? _("candidates are:") : _("candidate is:")); for (; candidates; candidates = candidates->next) - { - print_z_candidate (spaces ? spaces : str, candidates); - spaces = spaces ? spaces : get_spaces (str); - } - free (spaces); + print_z_candidate (NULL, candidates); } /* USER_SEQ is a user-defined conversion sequence, beginning with a @@ -3134,9 +3314,18 @@ build_user_type_conversion_1 (tree totype, tree expr, int flags) cand->second_conv = ics; if (!ics) - cand->viable = 0; + { + tree rettype = TREE_TYPE (TREE_TYPE (cand->fn)); + cand->viable = 0; + cand->reason = arg_conversion_rejection (-1, rettype, totype); + } else if (cand->viable == 1 && ics->bad_p) - cand->viable = -1; + { + tree rettype = TREE_TYPE (TREE_TYPE (cand->fn)); + cand->viable = -1; + cand->reason + = bad_arg_conversion_rejection (-1, rettype, totype); + } } } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 67f4f93..792c6db 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -4907,6 +4907,7 @@ extern void print_instantiation_context (void); extern void maybe_warn_variadic_templates (void); extern void maybe_warn_cpp0x (cpp0x_warn_str str); extern bool pedwarn_cxx98 (location_t, int, const char *, ...) ATTRIBUTE_GCC_DIAG(3,4); +extern location_t location_of (tree); /* in except.c */ extern void init_exception_processing (void); diff --git a/gcc/cp/error.c b/gcc/cp/error.c index 1560fc6..49e6341 100644 --- a/gcc/cp/error.c +++ b/gcc/cp/error.c @@ -98,7 +98,6 @@ static void cp_print_error_function (diagnostic_context *, diagnostic_info *); static bool cp_printer (pretty_printer *, text_info *, const char *, int, bool, bool, bool); -static location_t location_of (tree); void init_error (void) @@ -2459,7 +2458,7 @@ lang_decl_name (tree decl, int v, bool translate) /* Return the location of a tree passed to %+ formats. */ -static location_t +location_t location_of (tree t) { if (TREE_CODE (t) == PARM_DECL && DECL_CONTEXT (t))