[libgfortran] Replace implicit conversions between enums in io/transfer.c by explicit casts.

Message ID CAAgBjM=ipUaAG5Cf9i6UThGwYvyzrvnALhLq8tjyZWkU8wF0zQ@mail.gmail.com
State New
Headers show
Series
  • [libgfortran] Replace implicit conversions between enums in io/transfer.c by explicit casts.
Related show

Commit Message

Prathamesh Kulkarni Sept. 12, 2017, 11:38 a.m.
Hi,
I am working on patch for PR78736
(https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00011.html),
which adds a new warning -Wenum-conversion to C front-end to warn for
implicit conversion between different enums.
The warning in that patch triggered on io/transfer.c for following
implicit conversions:
i) Implicit conversion from unit_mode to file_mode
ii) Implicit conversion from unit_sign_s to unit_sign.

I was wondering if the warning for above implicit conversions would be
correct since unit_mode
and file_mode are different enums and similarly unit_sign_s and
unit_sign are different enums ?
Or are these warnings false positives ?

The attached patch makes the conversion explicit to silence the warnings.
Bootstrap+tested on x86_64-unknown-linux-gnu.
Does the patch look OK ?

Thanks,
Prathamesh
2017-09-12  Prathamesh Kulkarni  <prathamesh.kulkarni@linaro.org>

libgfortran/
	* io/transfer.c (current_mode): Cast FORM_UNSPECIFIED to file_mode.
	(formatted_transfer_scalar_read): Cast SIGN_S, SIGN_SS, SIGN_SP to
	unit_sign.
	(formatted_transfer_scalar_write): Likewise.

Comments

Prathamesh Kulkarni Sept. 25, 2017, 5:12 p.m. | #1
On 12 September 2017 at 17:08, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> Hi,
> I am working on patch for PR78736
> (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00011.html),
> which adds a new warning -Wenum-conversion to C front-end to warn for
> implicit conversion between different enums.
> The warning in that patch triggered on io/transfer.c for following
> implicit conversions:
> i) Implicit conversion from unit_mode to file_mode
> ii) Implicit conversion from unit_sign_s to unit_sign.
>
> I was wondering if the warning for above implicit conversions would be
> correct since unit_mode
> and file_mode are different enums and similarly unit_sign_s and
> unit_sign are different enums ?
> Or are these warnings false positives ?
>
> The attached patch makes the conversion explicit to silence the warnings.
> Bootstrap+tested on x86_64-unknown-linux-gnu.
> Does the patch look OK ?
ping https://gcc.gnu.org/ml/fortran/2017-09/msg00036.html

Thanks,
Prathamesh
>
> Thanks,
> Prathamesh
Janne Blomqvist Sept. 26, 2017, 6:53 a.m. | #2
On Mon, Sep 25, 2017 at 8:12 PM, Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
> On 12 September 2017 at 17:08, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
>> Hi,
>> I am working on patch for PR78736
>> (https://gcc.gnu.org/ml/gcc-patches/2017-09/msg00011.html),
>> which adds a new warning -Wenum-conversion to C front-end to warn for
>> implicit conversion between different enums.
>> The warning in that patch triggered on io/transfer.c for following
>> implicit conversions:
>> i) Implicit conversion from unit_mode to file_mode
>> ii) Implicit conversion from unit_sign_s to unit_sign.
>>
>> I was wondering if the warning for above implicit conversions would be
>> correct since unit_mode
>> and file_mode are different enums and similarly unit_sign_s and
>> unit_sign are different enums ?
>> Or are these warnings false positives ?
>>
>> The attached patch makes the conversion explicit to silence the warnings.
>> Bootstrap+tested on x86_64-unknown-linux-gnu.
>> Does the patch look OK ?
> ping https://gcc.gnu.org/ml/fortran/2017-09/msg00036.html

Hi,

based on a quick look, it does seem like your patch has managed to
flush out some potential bugs. As you say, those enums are different,
so I don't think your patch is the correct fix.

Jerry, you're probably the one most familiar with formatted IO, what
do you think?

Patch

diff --git a/libgfortran/io/transfer.c b/libgfortran/io/transfer.c
index 529637061b1..3307d213bb7 100644
--- a/libgfortran/io/transfer.c
+++ b/libgfortran/io/transfer.c
@@ -196,7 +196,7 @@  current_mode (st_parameter_dt *dtp)
 {
   file_mode m;
 
-  m = FORM_UNSPECIFIED;
+  m = (file_mode) FORM_UNSPECIFIED;
 
   if (dtp->u.p.current_unit->flags.access == ACCESS_DIRECT)
     {
@@ -1671,17 +1671,17 @@  formatted_transfer_scalar_read (st_parameter_dt *dtp, bt type, void *p, int kind
 
 	case FMT_S:
 	  consume_data_flag = 0;
-	  dtp->u.p.sign_status = SIGN_S;
+	  dtp->u.p.sign_status = (unit_sign) SIGN_S;
 	  break;
 
 	case FMT_SS:
 	  consume_data_flag = 0;
-	  dtp->u.p.sign_status = SIGN_SS;
+	  dtp->u.p.sign_status = (unit_sign) SIGN_SS;
 	  break;
 
 	case FMT_SP:
 	  consume_data_flag = 0;
-	  dtp->u.p.sign_status = SIGN_SP;
+	  dtp->u.p.sign_status = (unit_sign) SIGN_SP;
 	  break;
 
 	case FMT_BN:
@@ -2130,17 +2130,17 @@  formatted_transfer_scalar_write (st_parameter_dt *dtp, bt type, void *p, int kin
 
 	case FMT_S:
 	  consume_data_flag = 0;
-	  dtp->u.p.sign_status = SIGN_S;
+	  dtp->u.p.sign_status = (unit_sign) SIGN_S;
 	  break;
 
 	case FMT_SS:
 	  consume_data_flag = 0;
-	  dtp->u.p.sign_status = SIGN_SS;
+	  dtp->u.p.sign_status = (unit_sign) SIGN_SS;
 	  break;
 
 	case FMT_SP:
 	  consume_data_flag = 0;
-	  dtp->u.p.sign_status = SIGN_SP;
+	  dtp->u.p.sign_status = (unit_sign) SIGN_SP;
 	  break;
 
 	case FMT_BN: