[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?


-- 
Janne Blomqvist

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: