[gentoo-commits] proj/gcc-patches:master commit in: 15.0.0/gentoo/

["Sam James" <sam@gentoo.org>]

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/20241219234019.722392-1-dmalcolm@redhat.com/
+https://inbox.sourceware.org/gcc-patches/f34fdf57159821af93c7da1643edfa4796a77ad4.camel@redhat.com/#t
 
-From b0525913499d305c2116e31bc6505ed67e87cf19 Mon Sep 17 00:00:00 2001
-Message-ID: <b0525913499d305c2116e31bc6505ed67e87cf19.1734659653.git.sam@gentoo.org>
+From 595638c60e39f513301e34afdf14b04247634838 Mon Sep 17 00:00:00 2001
+Message-ID: <595638c60e39f513301e34afdf14b04247634838.1736599921.git.sam@gentoo.org>
 From: David Malcolm <dmalcolm@redhat.com>
-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&regrtesting?
+> >
+> > 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 <dmalcolm@redhat.com>
 ---
- 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,
  }
  \f
  /* 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