public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Tobias Burnus <tobias@codesourcery.com>
To: Jakub Jelinek <jakub@redhat.com>, Florian Weimer <fweimer@redhat.com>
Cc: Richard Earnshaw <Richard.Earnshaw@foss.arm.com>,
	<gcc-patches@gcc.gnu.org>, <fortran@gcc.gnu.org>
Subject: Re: [PATCH] libgfortran: Fix -Wincompatible-pointer-types errors
Date: Tue, 5 Dec 2023 14:35:50 +0100	[thread overview]
Message-ID: <feb41bdf-5a79-438c-8b22-f417a71b68d7@codesourcery.com> (raw)
In-Reply-To: <ZW78YWaxl9JCrqMh@tucnak>

Hi all,

the patch submission looks confusing as the context is a bit unclear
(aarch64 having two integer types?) and the slightly unmotivated 'long'
change (as explained in later emails: used as trick to find all
locations that should be changed and not being part of actually proposed
patch).

However, once this has been disentangled, the patch LGTM, assuming and
being positive that no aarch64 maintainer sees a problem.

Tobias

On 05.12.23 11:33, Jakub Jelinek wrote:
> On Tue, Dec 05, 2023 at 10:46:02AM +0100, Florian Weimer wrote:
>> Presumably the fixes will look like this?
>>
>> diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
>> index db3330060ce..4fcc77dbf83 100644
>> --- a/libgfortran/io/list_read.c
>> +++ b/libgfortran/io/list_read.c
>> @@ -2987,13 +2987,13 @@ nml_read_obj (st_parameter_dt *dtp, namelist_info *nl, index_type offset,
>>          /* If this object has a User Defined procedure, call it.  */
>>          if (nl->dtio_sub != NULL)
>>            {
>> -            int unit = dtp->u.p.current_unit->unit_number;
>> +            GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>>              char iotype[] = "NAMELIST";
>>              gfc_charlen_type iotype_len = 8;
>>              char tmp_iomsg[IOMSG_LEN] = "";
>>              char *child_iomsg;
>>              gfc_charlen_type child_iomsg_len;
>> -            int noiostat;
>> +            GFC_INTEGER_4 noiostat;
>>              int *child_iostat = NULL;
>>              gfc_full_array_i4 vlist;
>>              formatted_dtio dtio_ptr = (formatted_dtio)nl->dtio_sub;
> That seems insufficient.
>
> The following patch makes libgfortran build on i686-linux after hacking up
> --- kinds.h.xx        2023-12-05 00:23:00.133365064 +0100
> +++ kinds.h   2023-12-05 11:19:24.409679808 +0100
> @@ -10,8 +10,8 @@ typedef GFC_INTEGER_2 GFC_LOGICAL_2;
>   #define HAVE_GFC_LOGICAL_2
>   #define HAVE_GFC_INTEGER_2
>
> -typedef int32_t GFC_INTEGER_4;
> -typedef uint32_t GFC_UINTEGER_4;
> +typedef long GFC_INTEGER_4;
> +typedef unsigned long GFC_UINTEGER_4;
>   typedef GFC_INTEGER_4 GFC_LOGICAL_4;
>   #define HAVE_GFC_LOGICAL_4
>   #define HAVE_GFC_INTEGER_4
> in the build dir to emulate what newlib aarch64 is doing:
>
> 2023-12-05  Florian Weimer  <fweimer@redhat.com>
>           Jakub Jelinek  <jakub@redhat.com>
>
>       * io/list_read.c (list_formatted_read_scalar) <case BT_CLASS>:
>       Change types of unit and noiostat to GFC_INTEGER_4 from int, change
>       type of child_iostat from to GFC_INTEGER_4 * from int *, formatting
>       fixes.
>       (nml_read_obj): Likewise.
>       * io/write.c (list_formatted_write_scalar) <case BT_CLASS>: Likewise.
>       (nml_write_obj): Likewise.
>       * io/transfer.c (unformatted_read, unformatted_write): Likewise.
>
> --- libgfortran/io/list_read.c.jj     2023-05-09 00:07:26.161168737 +0200
> +++ libgfortran/io/list_read.c        2023-12-05 11:25:31.837426653 +0100
> @@ -2189,14 +2189,14 @@ list_formatted_read_scalar (st_parameter
>         break;
>       case BT_CLASS:
>         {
> -       int unit = dtp->u.p.current_unit->unit_number;
> +       GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>         char iotype[] = "LISTDIRECTED";
>             gfc_charlen_type iotype_len = 12;
>         char tmp_iomsg[IOMSG_LEN] = "";
>         char *child_iomsg;
>         gfc_charlen_type child_iomsg_len;
> -       int noiostat;
> -       int *child_iostat = NULL;
> +       GFC_INTEGER_4 noiostat;
> +       GFC_INTEGER_4 *child_iostat = NULL;
>         gfc_full_array_i4 vlist;
>
>         GFC_DESCRIPTOR_DATA(&vlist) = NULL;
> @@ -2204,8 +2204,8 @@ list_formatted_read_scalar (st_parameter
>
>         /* Set iostat, intent(out).  */
>         noiostat = 0;
> -       child_iostat = (dtp->common.flags & IOPARM_HAS_IOSTAT) ?
> -                       dtp->common.iostat : &noiostat;
> +       child_iostat = ((dtp->common.flags & IOPARM_HAS_IOSTAT)
> +                       ? dtp->common.iostat : &noiostat);
>
>         /* Set iomsge, intent(inout).  */
>         if (dtp->common.flags & IOPARM_HAS_IOMSG)
> @@ -2987,14 +2987,14 @@ nml_read_obj (st_parameter_dt *dtp, name
>           /* If this object has a User Defined procedure, call it.  */
>           if (nl->dtio_sub != NULL)
>             {
> -             int unit = dtp->u.p.current_unit->unit_number;
> +             GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>               char iotype[] = "NAMELIST";
>               gfc_charlen_type iotype_len = 8;
>               char tmp_iomsg[IOMSG_LEN] = "";
>               char *child_iomsg;
>               gfc_charlen_type child_iomsg_len;
> -             int noiostat;
> -             int *child_iostat = NULL;
> +             GFC_INTEGER_4 noiostat;
> +             GFC_INTEGER_4 *child_iostat = NULL;
>               gfc_full_array_i4 vlist;
>               formatted_dtio dtio_ptr = (formatted_dtio)nl->dtio_sub;
>
> @@ -3006,8 +3006,8 @@ nml_read_obj (st_parameter_dt *dtp, name
>
>               /* Set iostat, intent(out).  */
>               noiostat = 0;
> -             child_iostat = (dtp->common.flags & IOPARM_HAS_IOSTAT) ?
> -                             dtp->common.iostat : &noiostat;
> +             child_iostat = ((dtp->common.flags & IOPARM_HAS_IOSTAT)
> +                             ? dtp->common.iostat : &noiostat);
>
>               /* Set iomsg, intent(inout).  */
>               if (dtp->common.flags & IOPARM_HAS_IOMSG)
> --- libgfortran/io/write.c.jj 2023-09-28 21:49:38.632795791 +0200
> +++ libgfortran/io/write.c    2023-12-05 11:26:27.763627070 +0100
> @@ -1952,14 +1952,14 @@ list_formatted_write_scalar (st_paramete
>         break;
>       case BT_CLASS:
>         {
> -       int unit = dtp->u.p.current_unit->unit_number;
> +       GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>         char iotype[] = "LISTDIRECTED";
>         gfc_charlen_type iotype_len = 12;
>         char tmp_iomsg[IOMSG_LEN] = "";
>         char *child_iomsg;
>         gfc_charlen_type child_iomsg_len;
> -       int noiostat;
> -       int *child_iostat = NULL;
> +       GFC_INTEGER_4 noiostat;
> +       GFC_INTEGER_4 *child_iostat = NULL;
>         gfc_full_array_i4 vlist;
>
>         GFC_DESCRIPTOR_DATA(&vlist) = NULL;
> @@ -1967,8 +1967,8 @@ list_formatted_write_scalar (st_paramete
>
>         /* Set iostat, intent(out).  */
>         noiostat = 0;
> -       child_iostat = (dtp->common.flags & IOPARM_HAS_IOSTAT) ?
> -                       dtp->common.iostat : &noiostat;
> +       child_iostat = ((dtp->common.flags & IOPARM_HAS_IOSTAT)
> +                       ? dtp->common.iostat : &noiostat);
>
>         /* Set iomsge, intent(inout).  */
>         if (dtp->common.flags & IOPARM_HAS_IOMSG)
> @@ -2277,14 +2277,14 @@ nml_write_obj (st_parameter_dt *dtp, nam
>             /* First ext_name => get length of all possible components  */
>             if (obj->dtio_sub != NULL)
>               {
> -               int unit = dtp->u.p.current_unit->unit_number;
> +               GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>                 char iotype[] = "NAMELIST";
>                 gfc_charlen_type iotype_len = 8;
>                 char tmp_iomsg[IOMSG_LEN] = "";
>                 char *child_iomsg;
>                 gfc_charlen_type child_iomsg_len;
> -               int noiostat;
> -               int *child_iostat = NULL;
> +               GFC_INTEGER_4 noiostat;
> +               GFC_INTEGER_4 *child_iostat = NULL;
>                 gfc_full_array_i4 vlist;
>                 formatted_dtio dtio_ptr = (formatted_dtio)obj->dtio_sub;
>
> @@ -2292,8 +2292,8 @@ nml_write_obj (st_parameter_dt *dtp, nam
>
>                 /* Set iostat, intent(out).  */
>                 noiostat = 0;
> -               child_iostat = (dtp->common.flags & IOPARM_HAS_IOSTAT) ?
> -                               dtp->common.iostat : &noiostat;
> +               child_iostat = ((dtp->common.flags & IOPARM_HAS_IOSTAT)
> +                               ? dtp->common.iostat : &noiostat);
>
>                 /* Set iomsg, intent(inout).  */
>                 if (dtp->common.flags & IOPARM_HAS_IOMSG)
> --- libgfortran/io/transfer.c.jj      2023-05-09 00:07:26.162168723 +0200
> +++ libgfortran/io/transfer.c 2023-12-05 11:25:15.756656561 +0100
> @@ -1092,17 +1092,17 @@ unformatted_read (st_parameter_dt *dtp,
>
>     if (type == BT_CLASS)
>       {
> -       int unit = dtp->u.p.current_unit->unit_number;
> +       GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>         char tmp_iomsg[IOMSG_LEN] = "";
>         char *child_iomsg;
>         gfc_charlen_type child_iomsg_len;
> -       int noiostat;
> -       int *child_iostat = NULL;
> +       GFC_INTEGER_4 noiostat;
> +       GFC_INTEGER_4 *child_iostat = NULL;
>
>         /* Set iostat, intent(out).  */
>         noiostat = 0;
> -       child_iostat = (dtp->common.flags & IOPARM_HAS_IOSTAT) ?
> -                       dtp->common.iostat : &noiostat;
> +       child_iostat = ((dtp->common.flags & IOPARM_HAS_IOSTAT)
> +                       ? dtp->common.iostat : &noiostat);
>
>         /* Set iomsg, intent(inout).  */
>         if (dtp->common.flags & IOPARM_HAS_IOMSG)
> @@ -1119,7 +1119,7 @@ unformatted_read (st_parameter_dt *dtp,
>         /* Call the user defined unformatted READ procedure.  */
>         dtp->u.p.current_unit->child_dtio++;
>         dtp->u.p.ufdtio_ptr (dest, &unit, child_iostat, child_iomsg,
> -                           child_iomsg_len);
> +                            child_iomsg_len);
>         dtp->u.p.current_unit->child_dtio--;
>         return;
>       }
> @@ -1222,17 +1222,17 @@ unformatted_write (st_parameter_dt *dtp,
>
>     if (type == BT_CLASS)
>       {
> -       int unit = dtp->u.p.current_unit->unit_number;
> +       GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>         char tmp_iomsg[IOMSG_LEN] = "";
>         char *child_iomsg;
>         gfc_charlen_type child_iomsg_len;
> -       int noiostat;
> -       int *child_iostat = NULL;
> +       GFC_INTEGER_4 noiostat;
> +       GFC_INTEGER_4 *child_iostat = NULL;
>
>         /* Set iostat, intent(out).  */
>         noiostat = 0;
> -       child_iostat = (dtp->common.flags & IOPARM_HAS_IOSTAT) ?
> -                       dtp->common.iostat : &noiostat;
> +       child_iostat = ((dtp->common.flags & IOPARM_HAS_IOSTAT)
> +                       ? dtp->common.iostat : &noiostat);
>
>         /* Set iomsg, intent(inout).  */
>         if (dtp->common.flags & IOPARM_HAS_IOMSG)
> @@ -1249,7 +1249,7 @@ unformatted_write (st_parameter_dt *dtp,
>         /* Call the user defined unformatted WRITE procedure.  */
>         dtp->u.p.current_unit->child_dtio++;
>         dtp->u.p.ufdtio_ptr (source, &unit, child_iostat, child_iomsg,
> -                           child_iomsg_len);
> +                            child_iomsg_len);
>         dtp->u.p.current_unit->child_dtio--;
>         return;
>       }
> @@ -1688,13 +1688,13 @@ formatted_transfer_scalar_read (st_param
>           return;
>         if (require_type (dtp, BT_CLASS, type, f))
>           return;
> -       int unit = dtp->u.p.current_unit->unit_number;
> +       GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>         char dt[] = "DT";
>         char tmp_iomsg[IOMSG_LEN] = "";
>         char *child_iomsg;
>         gfc_charlen_type child_iomsg_len;
> -       int noiostat;
> -       int *child_iostat = NULL;
> +       GFC_INTEGER_4 noiostat;
> +       GFC_INTEGER_4 *child_iostat = NULL;
>         char *iotype;
>         gfc_charlen_type iotype_len = f->u.udf.string_len;
>
> @@ -1709,8 +1709,8 @@ formatted_transfer_scalar_read (st_param
>
>         /* Set iostat, intent(out).  */
>         noiostat = 0;
> -       child_iostat = (dtp->common.flags & IOPARM_HAS_IOSTAT) ?
> -                       dtp->common.iostat : &noiostat;
> +       child_iostat = ((dtp->common.flags & IOPARM_HAS_IOSTAT)
> +                       ? dtp->common.iostat : &noiostat);
>
>         /* Set iomsg, intent(inout).  */
>         if (dtp->common.flags & IOPARM_HAS_IOMSG)
> @@ -2169,13 +2169,13 @@ formatted_transfer_scalar_write (st_para
>       case FMT_DT:
>         if (n == 0)
>           goto need_data;
> -       int unit = dtp->u.p.current_unit->unit_number;
> +       GFC_INTEGER_4 unit = dtp->u.p.current_unit->unit_number;
>         char dt[] = "DT";
>         char tmp_iomsg[IOMSG_LEN] = "";
>         char *child_iomsg;
>         gfc_charlen_type child_iomsg_len;
> -       int noiostat;
> -       int *child_iostat = NULL;
> +       GFC_INTEGER_4 noiostat;
> +       GFC_INTEGER_4 *child_iostat = NULL;
>         char *iotype;
>         gfc_charlen_type iotype_len = f->u.udf.string_len;
>
> @@ -2190,8 +2190,8 @@ formatted_transfer_scalar_write (st_para
>
>         /* Set iostat, intent(out).  */
>         noiostat = 0;
> -       child_iostat = (dtp->common.flags & IOPARM_HAS_IOSTAT) ?
> -                       dtp->common.iostat : &noiostat;
> +       child_iostat = ((dtp->common.flags & IOPARM_HAS_IOSTAT)
> +                       ? dtp->common.iostat : &noiostat);
>
>         /* Set iomsg, intent(inout).  */
>         if (dtp->common.flags & IOPARM_HAS_IOMSG)
>
>
>       Jakub
>
-----------------
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634 München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht München, HRB 106955

  parent reply	other threads:[~2023-12-05 13:36 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-20  9:55 [PATCH v3 00/11] : More warnings as errors by default Florian Weimer
2023-11-20  9:55 ` [PATCH v3 01/11] aarch64: Avoid -Wincompatible-pointer-types warning in Linux unwinder Florian Weimer
2023-11-22 23:24   ` Joseph Myers
2023-11-20  9:55 ` [PATCH v3 02/11] aarch64: Call named function in gcc.target/aarch64/aapcs64/ice_1.c Florian Weimer
2023-11-22 23:25   ` Joseph Myers
2023-11-20  9:55 ` [PATCH v3 03/11] gm2: Add missing declaration of m2pim_M2RTS_Terminate to test Florian Weimer
2023-11-22 23:28   ` Joseph Myers
2023-11-20  9:56 ` [PATCH v3 04/11] Add tests for validating future C permerrors Florian Weimer
2023-11-30 17:31   ` Marek Polacek
2023-11-30 17:37     ` Florian Weimer
2023-11-30 17:39       ` Marek Polacek
2023-11-30 18:25         ` Jakub Jelinek
2023-11-20  9:56 ` [PATCH v3 05/11] c: Turn int-conversion warnings into permerrors Florian Weimer
2023-11-30 19:04   ` Marek Polacek
2023-11-30 19:46     ` Florian Weimer
2023-11-20  9:56 ` [PATCH v3 06/11] c: Turn -Wimplicit-function-declaration into a permerror Florian Weimer
2023-11-30 19:15   ` Marek Polacek
2023-12-01 15:54   ` c: Turn -Wimplicit-function-declaration into a permerror: Fix 'gcc.dg/gnu23-builtins-no-dfp-1.c' (was: [PATCH v3 06/11] c: Turn -Wimplicit-function-declaration into a permerror) Thomas Schwinge
2023-12-03  5:55     ` [committed] Fix gnu23-builtins-no-dfp Jeff Law
2023-12-03  7:41       ` Florian Weimer
2023-12-03 12:23         ` Thomas Schwinge
2023-12-03 20:57           ` Jeff Law
2023-12-05  9:31             ` [v2] c: Turn -Wimplicit-function-declaration into a permerror: Fix 'gcc.dg/gnu23-builtins-no-dfp-1.c' (was: [committed] Fix gnu23-builtins-no-dfp) Thomas Schwinge
2024-04-09 11:40   ` [PATCH v3 06/11] c: Turn -Wimplicit-function-declaration into a permerror Sebastian Huber
2024-04-09 12:10     ` Sam James
2024-04-09 12:26       ` Sebastian Huber
2024-04-09 12:56         ` Sam James
2023-11-20  9:56 ` [PATCH v3 07/11] c: Turn -Wimplicit-int " Florian Weimer
2023-11-30 19:48   ` Marek Polacek
2023-11-20  9:56 ` [PATCH v3 08/11] c: Do not ignore some forms of -Wimplicit-int in system headers Florian Weimer
2023-11-30 19:53   ` Marek Polacek
2023-11-20  9:56 ` [PATCH v3 09/11] c: Turn -Wreturn-mismatch into a permerror Florian Weimer
2023-11-23 17:32   ` Marek Polacek
2023-11-23 18:22     ` Florian Weimer
2023-11-30 16:17       ` Marek Polacek
2023-11-20  9:56 ` [PATCH v3 10/11] c: Turn -Wincompatible-pointer-types " Florian Weimer
2023-11-30 20:47   ` Marek Polacek
2023-11-30 21:02   ` Marek Polacek
2023-11-30 21:11     ` Florian Weimer
2023-11-30 21:15       ` Marek Polacek
2023-11-30 21:23         ` Jakub Jelinek
2023-11-30 21:27           ` Florian Weimer
2023-11-30 21:30             ` Jakub Jelinek
2023-11-30 21:36               ` Marek Polacek
2023-12-10 19:23                 ` Jason Merrill
2023-12-05  9:37   ` Richard Earnshaw
2023-12-05  9:46     ` Florian Weimer
2023-12-05 10:11       ` Richard Earnshaw
2023-12-05 10:33       ` [PATCH] libgfortran: Fix -Wincompatible-pointer-types errors Jakub Jelinek
2023-12-05 10:47         ` Richard Earnshaw
2023-12-05 10:51           ` Jakub Jelinek
2023-12-05 10:57             ` Richard Earnshaw
2023-12-05 10:59               ` Jakub Jelinek
2023-12-05 17:35                 ` Richard Earnshaw
2023-12-05 11:00               ` Florian Weimer
2023-12-05 13:35         ` Tobias Burnus [this message]
2023-12-06 12:04   ` [PATCH v3 10/11] c: Turn -Wincompatible-pointer-types into a permerror Prathamesh Kulkarni
2023-12-06 12:12     ` Florian Weimer
2023-11-20  9:56 ` [PATCH v3 11/11] c: Add new -Wdeclaration-missing-parameter-type permerror Florian Weimer
2023-11-20 19:12   ` Eric Gallager
2023-11-20 19:32     ` Florian Weimer
2023-11-30 21:10   ` Marek Polacek
2023-12-11  9:11     ` Florian Weimer
2023-11-23  0:54 ` [PATCH v3 00/11] : More warnings as errors by default Jeff Law
2023-11-23  1:04   ` Florian Weimer
2023-11-27 20:23     ` Sam James
2023-11-30 21:35       ` Florian Weimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=feb41bdf-5a79-438c-8b22-f417a71b68d7@codesourcery.com \
    --to=tobias@codesourcery.com \
    --cc=Richard.Earnshaw@foss.arm.com \
    --cc=fortran@gcc.gnu.org \
    --cc=fweimer@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).