Hi David,

On Sat, Oct 11, 2025 at 07:17:18PM -0400, David Malcolm wrote:
> On Sun, 2025-09-28 at 10:09 +0200, Alejandro Colomar wrote:
> > Not sure about moving the definition of ptr_spec.
> > 
> > Signed-off-by: Alejandro Colomar <[email protected]>
> > ---
> >  gcc/c-family/c-warn.cc | 582 +++++++++++++++++++++------------------
> > --
> >  1 file changed, 294 insertions(+), 288 deletions(-)
> 
> Hi Alejandro.
> 
> Thanks for the patch.  I think it would have helped if you had written
> a ChangeLog entry for this to make the intent of the refactoring clear
> within the commit.  Am I right in thinking the commit message should
> read something like:
> 
> BEGIN SUGGESTION...
> 
> No functional change intended
> 
> gcc/c-family/ChangeLog:
>       * c-warn.cc (warn_parms_array_mismatch): Split out body of
>         per-pair in parameter lists iteration into...
>         (warn_parm_array_mismatch): ...this new function.
> 
> 
> ...END SUGGESTION
>
> ?

Thanks!  That sounds good.  I'll add it for the next revision of the
patch set.

> That said, I too am not sure about the ptr_spec definition; should it
> stay in the parent function?

I didn't fully understand how the ptr_spec definition is working here,
so if anyone knows, please help.  I was especially puzzled that it was
being reused without any clearing after each use, which was a bit weird.

I prefer it in the child function, because it makes it clear that we
don't want to reuse anything.  However, if anyone knows, please review
that part thoroughly.

> Hope this is constructive.
> Dave

Yup.  Thanks!


Have a lovely night!
Alex

-- 
<https://www.alejandro-colomar.es>
Use port 80 (that is, <...:80/>).

Attachment: signature.asc
Description: PGP signature

Reply via email to