commit: 363d9cf8b9fb665334e166cdaf515bb28dbc2880 Author: Sam James <sam <AT> gentoo <DOT> org> AuthorDate: Sat Jan 11 12:52:36 2025 +0000 Commit: Sam James <sam <AT> gentoo <DOT> org> CommitDate: Sat Jan 11 12:52:36 2025 +0000 URL: https://gitweb.gentoo.org/proj/gcc-patches.git/commit/?id=363d9cf8
15.0.0: refresh C23 "too many/few" arguments patch Signed-off-by: Sam James <sam <AT> gentoo.org> ...ovements-to-too-few-many-arguments-errors.patch | 233 +++++++++++++++++---- 1 file changed, 197 insertions(+), 36 deletions(-) diff --git a/15.0.0/gentoo/77_all_PR118112-c-c-UX-improvements-to-too-few-many-arguments-errors.patch b/15.0.0/gentoo/77_all_PR118112-c-c-UX-improvements-to-too-few-many-arguments-errors.patch index 8688ac1..b1e83b5 100644 --- a/15.0.0/gentoo/77_all_PR118112-c-c-UX-improvements-to-too-few-many-arguments-errors.patch +++ b/15.0.0/gentoo/77_all_PR118112-c-c-UX-improvements-to-too-few-many-arguments-errors.patch @@ -1,11 +1,165 @@ -https://inbox.sourceware.org/gcc-patches/[email protected]/ +https://inbox.sourceware.org/gcc-patches/[email protected]/#t -From b0525913499d305c2116e31bc6505ed67e87cf19 Mon Sep 17 00:00:00 2001 -Message-ID: <b0525913499d305c2116e31bc6505ed67e87cf19.1734659653.git....@gentoo.org> +From 595638c60e39f513301e34afdf14b04247634838 Mon Sep 17 00:00:00 2001 +Message-ID: <595638c60e39f513301e34afdf14b04247634838.1736599921.git....@gentoo.org> From: David Malcolm <[email protected]> -Date: Thu, 19 Dec 2024 18:40:19 -0500 +Date: Fri, 10 Jan 2025 13:47:43 -0500 Subject: [PATCH] c/c++: UX improvements to 'too {few,many} arguments' errors [PR118112] +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +On Thu, 2025-01-09 at 22:28 -0500, David Malcolm wrote: +> On Thu, 2025-01-09 at 21:15 -0500, Jason Merrill wrote: +> > On 1/9/25 7:00 PM, David Malcolm wrote: +> > > On Thu, 2025-01-09 at 14:21 -0500, Jason Merrill wrote: +> > > +> > > Thanks for taking a look... +> > > +> > > > > On 1/9/25 2:11 PM, David Malcolm wrote: +> > > > > +> > > > > @@ -4743,7 +4769,38 @@ convert_arguments (tree typelist, +> > > > > vec<tree, +> > > > > va_gc> **values, tree fndecl, +> > > > > if (typetail && typetail != void_list_node) +> > > > > { +> > > > > if (complain & tf_error) +> > > > > - error_args_num (input_location, fndecl, +> > > > > /*too_many_p=*/false); +> > > > > + { +> > > > > + /* Not enough args. +> > > > > + Determine minimum number of arguments +> > > > > required. */ +> > > > > + int min_expected_num = 0; +> > > > > + bool at_least_p = false; +> > > > > + tree iter = typelist; +> > > > > + while (true) +> > > > > + { +> > > > > + if (!iter) +> > > > > + { +> > > > > + /* Variadic arguments; stop +> > > > > iterating. +> > > > > */ +> > > > > + at_least_p = true; +> > > > > + break; +> > > > > + } +> > > > > + if (iter == void_list_node) +> > > > > + /* End of arguments; stop iterating. */ +> > > > > + break; +> > > > > + if (fndecl && TREE_PURPOSE (iter) +> > > > > + && TREE_CODE (TREE_PURPOSE (iter)) != +> > > > > DEFERRED_PARSE) +> > > > > +> > > > +> > > > Why are you checking DEFERRED_PARSE? That indicates a default +> > > > argument, +> > > > even if it isn't parsed yet. For that case we should get the +> > > > error +> > > > in +> > > > convert_default_arg rather than pretend there's no default +> > > > argument. +> > > +> > > I confess that the check for DEFERRED_PARSE was a rather mindless +> > > copy +> > > and paste by me from the "See if there are default arguments that +> > > can be +> > > used" logic earlier in the function. +> > > +> > > I've removed it in the latest version of the patch. +> > > +> > > > > + { +> > > > > + /* Found a default argument; skip this +> > > > > one when +> > > > > + counting minimum required. */ +> > > > > + at_least_p = true; +> > > > > + iter = TREE_CHAIN (iter); +> > > > > + continue; +> > > > +> > > > We could break here, once you have a default arg the rest of +> > > > the +> > > > parms +> > > > need to have them as well. +> > > +> > > Indeed; I've updated this in the latest version of the patch, so +> > > we break out as soon as we see an arg with a non-null +> > > TREE_PURPOSE. +> > > +> > > > +> > > > > + } +> > > > > + ++min_expected_num; +> > > > > + iter = TREE_CHAIN (iter); +> > > > > + } +> > > > > + error_args_num (input_location, fndecl, +> > > > > + min_expected_num, actual_num, +> > > > > at_least_p); +> > > > > + } +> > > > > return -1; +> > > > > } +> > > +> > > Here's a v3 version of the patch, which is currently going +> > > through +> > > my tester. +> > > +> > > OK for trunk if it passes bootstrap®rtesting? +> > +> > OK. +> +> Thanks. However, it turns out that I may have misspoke, and the v2 +> patch might have been the correct approach: if we bail out on the +> first +> arg with a TREE_PURPOSE then in +> gcc/testsuite/g++.dg/cpp0x/variadic169.C +> +> 1 │ // DR 2233 +> 2 │ // { dg-do compile { target c++11 } } +> 3 │ +> 4 │ template<typename ...T> void f(int n = 0, T ...t); +> 5 │ +> 6 │ int main() +> 7 │ { +> 8 │ f<int>(); // { dg-error "too few arguments to +> function '\[^\n\r\]*'; expected at least 1, have 0" } +> 9 │ } +> +> we instead emit the nonsensical "expected at least 0, have 0": +> +> error: too few arguments to function ‘void f(int, T ...) [with T = +> {int}]’; expected at least 0, have 0 +> 8 | f<int>(); // { dg-error "too few +> arguments to function '\[^\n\r\]*'; expected at least 1, have 0" } +> | ~~~~~~^~ +> ../../src/gcc/testsuite/g++.dg/cpp0x/variadic169.C:4:30: note: +> declared +> here +> 4 | template<typename ...T> void f(int n = 0, T ...t); +> | ^ +> +> whereas with the v2 patch we count the trailing arg after the default +> arg, and we emit "expected at least 1, have 0", which seems correct +> to +> me. +> +> I'm testing a version of the patch that continues to iterate after a +> TREE_PURPOSE (as v2, but but dropping the check on DEFERRED_PARSE). +> +> Thanks +> Dave + +Hi Jason + +Here's an updated version of the patch which drops the check on +DEFERRED_PARSE, but continues to iterate on TREE_PURPOSE, for dealing +with the case of explicit template args where default args are followed +by missing mandatory args (the code above mentions DR777, so I +referenced that). + +Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. + +Are the C++ parts OK for trunk? + +Thanks +Dave Consider this case of a bad call to a callback function (perhaps due to C23 changing the meaning of () in function decls): @@ -82,9 +236,6 @@ s.c:1:6: note: declared here 1 | void callee (const char *, ...); | ^~~~~~ -Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. -OK for trunk? - gcc/c/ChangeLog: PR c/118112 * c-typeck.cc (inform_declaration): Add "function_expr" param and @@ -122,19 +273,19 @@ gcc/testsuite/ChangeLog: Signed-off-by: David Malcolm <[email protected]> --- - gcc/c/c-typeck.cc | 77 ++++++++++++--- - gcc/cp/typeck.cc | 94 ++++++++++++++---- - .../c-c++-common/too-few-arguments.c | 38 ++++++++ - .../c-c++-common/too-many-arguments.c | 96 +++++++++++++++++++ - gcc/testsuite/g++.dg/cpp0x/variadic169.C | 2 +- - gcc/testsuite/g++.dg/modules/macloc-1_c.C | 4 +- - gcc/testsuite/g++.dg/modules/macloc-1_d.C | 4 +- - 7 files changed, 280 insertions(+), 35 deletions(-) + gcc/c/c-typeck.cc | 77 +++++++++++-- + gcc/cp/typeck.cc | 104 ++++++++++++++---- + .../c-c++-common/too-few-arguments.c | 38 +++++++ + .../c-c++-common/too-many-arguments.c | 96 ++++++++++++++++ + gcc/testsuite/g++.dg/cpp0x/variadic169.C | 2 +- + gcc/testsuite/g++.dg/modules/macloc-1_c.C | 4 +- + gcc/testsuite/g++.dg/modules/macloc-1_d.C | 4 +- + 7 files changed, 287 insertions(+), 38 deletions(-) create mode 100644 gcc/testsuite/c-c++-common/too-few-arguments.c create mode 100644 gcc/testsuite/c-c++-common/too-many-arguments.c diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc -index 9756edaae084..fff2bba06954 100644 +index 6e40f7edf02a..cd9290160d7a 100644 --- a/gcc/c/c-typeck.cc +++ b/gcc/c/c-typeck.cc @@ -3737,14 +3737,30 @@ build_function_call (location_t loc, tree function, tree params) @@ -284,7 +435,7 @@ index 9756edaae084..fff2bba06954 100644 return error_args ? -1 : (int) parmnum; diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc -index 964e549a6122..2966931ca8c1 100644 +index 3e0d71102abd..a8580eddd397 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -59,7 +59,7 @@ static tree get_delta_difference (tree, tree, bool, bool, tsubst_flags_t); @@ -296,14 +447,17 @@ index 964e549a6122..2966931ca8c1 100644 static int convert_arguments (tree, vec<tree, va_gc> **, tree, int, tsubst_flags_t); static bool is_std_move_p (tree); -@@ -4533,11 +4533,16 @@ cp_build_function_call_vec (tree function, vec<tree, va_gc> **params, +@@ -4535,11 +4535,19 @@ cp_build_function_call_vec (tree function, vec<tree, va_gc> **params, } /* Subroutine of convert_arguments. - Print an error message about a wrong number of arguments. */ + Print an error message about a wrong number of arguments. + If AT_LEAST_P is true, then EXPECTED_NUM is the minimum number -+ of expected arguments. */ ++ of expected arguments, otherwise EXPECTED_NUM is the exact number ++ of expected arguments. ++ ACTUAL_NUM is the actual number of arguments that were explicitly ++ passed at the callsite (i.e. not counting default arguments). */ static void -error_args_num (location_t loc, tree fndecl, bool too_many_p) @@ -315,7 +469,7 @@ index 964e549a6122..2966931ca8c1 100644 if (fndecl) { auto_diagnostic_group d; -@@ -4548,22 +4553,28 @@ error_args_num (location_t loc, tree fndecl, bool too_many_p) +@@ -4550,22 +4558,28 @@ error_args_num (location_t loc, tree fndecl, bool too_many_p) == DECL_NAME (TYPE_NAME (DECL_CONTEXT (fndecl))))) error_at (loc, too_many_p @@ -353,7 +507,7 @@ index 964e549a6122..2966931ca8c1 100644 if (!DECL_IS_UNDECLARED_BUILTIN (fndecl)) inform (DECL_SOURCE_LOCATION (fndecl), "declared here"); } -@@ -4572,12 +4583,19 @@ error_args_num (location_t loc, tree fndecl, bool too_many_p) +@@ -4574,12 +4588,19 @@ error_args_num (location_t loc, tree fndecl, bool too_many_p) if (c_dialect_objc () && objc_message_selector ()) error_at (loc, too_many_p @@ -378,30 +532,34 @@ index 964e549a6122..2966931ca8c1 100644 } } -@@ -4607,6 +4625,10 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl, +@@ -4609,9 +4630,11 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl, /* Argument passing is always copy-initialization. */ flags |= LOOKUP_ONLYCONVERTING; +- for (i = 0, typetail = typelist; +- i < vec_safe_length (*values); +- i++) + /* Preserve actual number of arguments passed (without counting default + args), in case we need to complain about too many/few. */ -+ int actual_num = vec_safe_length (*values); ++ const unsigned actual_num = vec_safe_length (*values); + - for (i = 0, typetail = typelist; - i < vec_safe_length (*values); - i++) -@@ -4621,7 +4643,10 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl, ++ for (i = 0, typetail = typelist; i < actual_num; i++) + { + tree type = typetail ? TREE_VALUE (typetail) : 0; + tree val = (**values)[i]; +@@ -4623,7 +4646,10 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl, { if (complain & tf_error) { - error_args_num (input_location, fndecl, /*too_many_p=*/true); + /* Too many args. */ -+ int expected_num = i; -+ error_args_num (input_location, fndecl, expected_num, actual_num, -+ false); ++ error_args_num (input_location, fndecl, ++ /*expected_num=*/i, actual_num, ++ /*at_least_p=*/false); return i; } else -@@ -4743,7 +4768,38 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl, +@@ -4745,7 +4771,41 @@ convert_arguments (tree typelist, vec<tree, va_gc> **values, tree fndecl, if (typetail && typetail != void_list_node) { if (complain & tf_error) @@ -423,11 +581,14 @@ index 964e549a6122..2966931ca8c1 100644 + if (iter == void_list_node) + /* End of arguments; stop iterating. */ + break; -+ if (fndecl && TREE_PURPOSE (iter) -+ && TREE_CODE (TREE_PURPOSE (iter)) != DEFERRED_PARSE) ++ if (fndecl && TREE_PURPOSE (iter)) + { + /* Found a default argument; skip this one when -+ counting minimum required. */ ++ counting minimum required, but there might ++ be non-default arguments left to count: ++ after DR777, with explicit template args we can ++ end up with a default argument followed by ++ no default argument. */ + at_least_p = true; + iter = TREE_CHAIN (iter); + continue; @@ -624,7 +785,7 @@ index 282a31c4a2d1..56c001fc3f83 100644 +// { dg-regexp "\[^\n]*macloc-1_d.C:8:6: error: too many arguments to function 'int me@agnes\\(\\)'; expected 0, have 1\nIn module agnes, imported at \[^\n]*macloc-1_d.C:4:\n\[^\n]*macloc-1_a.C:11:12: note: declared here\n\[^\n]*macloc-1_a.C:8:20: note: in definition of macro 'BOB'\n" } +// { dg-regexp "\[^\n]*macloc-1_d.C:9:7: error: too many arguments to function 'void gru@edith\\(\\)'; expected 0, have 1\nIn module edith, imported at \[^\n]*macloc-1_d.C:3:\n\[^\n]*macloc-1_b.C:10:20: note: declared here\n\[^\n]*macloc-1_b.C:6:19: note: in definition of macro 'STUART'\n" } -base-commit: b11e85adbfdb02bc7743098d358a5ea362648ca1 +base-commit: 65286465b94cba6ee3d59edbc771bef0088ac46e -- -2.47.1 +2.48.0
