Bug 91593 - Implicit enum conversions in libgfortran/io/transfer.c
Summary: Implicit enum conversions in libgfortran/io/transfer.c
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libfortran (show other bugs)
Version: 10.0
: P3 normal
Target Milestone: ---
Assignee: Jerry DeLisle
URL:
Keywords: diagnostic
Depends on:
Blocks:
 
Reported: 2019-08-29 02:50 UTC by prathamesh3492
Modified: 2019-10-19 15:49 UTC (History)
3 users (show)

See Also:
Host: x86_64-unknown-linux-gnu
Target: x86_64-unknown-linux-gnu
Build: x86_64-unknown-linux-gnu
Known to work:
Known to fail:
Last reconfirmed: 2019-09-04 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description prathamesh3492 2019-08-29 02:50:50 UTC
Hi,
I added a patch for Wenum-conversion (PR78736), that exposes some implicit enum conversions in libgfortran/io/transfer.c:

./../../gcc/libgfortran/io/transfer.c: In function ‘current_mode’:
../../../gcc/libgfortran/io/transfer.c:206:5: warning: implicit conversion from ‘enum <anonymous>’ to ‘file_mode’ {aka ‘enum <anonymous>’} [-Wenum-conversion]
  206 |   m = FORM_UNSPECIFIED;
      |     ^
../../../gcc/libgfortran/io/transfer.c: In function ‘formatted_transfer_scalar_read’:
../../../gcc/libgfortran/io/transfer.c:1730:25: warning: implicit conversion from ‘enum <anonymous>’ to ‘unit_sign’ {aka ‘enum <anonymous>’} [-Wenum-conversion]
 1730 |    dtp->u.p.sign_status = SIGN_S;
      |                         ^
../../../gcc/libgfortran/io/transfer.c:1735:25: warning: implicit conversion from ‘enum <anonymous>’ to ‘unit_sign’ {aka ‘enum <anonymous>’} [-Wenum-conversion]
 1735 |    dtp->u.p.sign_status = SIGN_SS;
      |                         ^
../../../gcc/libgfortran/io/transfer.c:1740:25: warning: implicit conversion from ‘enum <anonymous>’ to ‘unit_sign’ {aka ‘enum <anonymous>’} [-Wenum-conversion]
 1740 |    dtp->u.p.sign_status = SIGN_SP;
      |                         ^
./../../gcc/libgfortran/io/transfer.c: In function ‘formatted_transfer_scalar_write’:
../../../gcc/libgfortran/io/transfer.c:2189:25: warning: implicit conversion from ‘enum <anonymous>’ to ‘unit_sign’ {aka ‘enum <anonymous>’} [-Wenum-conversion]
 2189 |    dtp->u.p.sign_status = SIGN_S;
      |                         ^
./../../gcc/libgfortran/io/transfer.c:2194:25: warning: implicit conversion from ‘enum <anonymous>’ to ‘unit_sign’ {aka ‘enum <anonymous>’} [-Wenum-conversion]
 2194 |    dtp->u.p.sign_status = SIGN_SS;
      |                         ^
../../../gcc/libgfortran/io/transfer.c:2199:25: warning: implicit conversion from ‘enum <anonymous>’ to ‘unit_sign’ {aka ‘enum <anonymous>’} [-Wenum-conversion]
2199 |    dtp->u.p.sign_status = SIGN_SP;
     |                         ^

AFAIU, the warnings are correct in this case since the enums are different
and thus there's an implicit conversion from one enum type to another ?

Thanks,
Prathamesh
Comment 1 prathamesh3492 2019-08-29 02:59:19 UTC
Patch for PR78736 that triggers the warnings:
https://gcc.gnu.org/ml/gcc-patches/2019-08/msg01938.html

Thanks,
Prathamesh
Comment 2 Jeffrey A. Law 2019-09-03 19:52:06 UTC
And ideally once this is fixed we would reevaluate if -Wenum-conversion should be part of -Wall vs -Wextra.
Comment 3 Thomas Koenig 2019-09-03 22:19:28 UTC
Jerry, I am away from my computer at the moment.

Does zhis ring a bell?
Comment 4 Jerry DeLisle 2019-09-04 02:05:45 UTC
(In reply to Thomas Koenig from comment #3)
> Jerry, I am away from my computer at the moment.
> 
> Does zhis ring a bell?

Does not specifically ring a bell, but I am very familair with the code and would be happy to fix it.
Comment 5 Jerry DeLisle 2019-09-06 17:56:30 UTC
Looking back at the code I see we are translating from Front-end to run time regarding whether or not we actually are going to show the positive sign or not or suppress it the sign altogether in floating point writes. In this process we actually have three different enumumerators defined. I am going to analyze this a bit more since it "seems" that one ought to be able to do this all in one step at the right place.  There is a comment about keeping things alligned so as to not break the ABI, so I would bet that as the standards evolved we had to evolve this code a bit. I will see what I can come up with that might make more sense and not break the ABI. (other than just doing a cast)
Comment 6 Jerry DeLisle 2019-09-28 19:15:27 UTC
Author: jvdelisle
Date: Sat Sep 28 19:14:47 2019
New Revision: 276255

URL: https://gcc.gnu.org/viewcvs?rev=276255&root=gcc&view=rev
Log:
2019-09-28  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/91593
	* io/io.h: Add gcc_unreachable().
	* io/transfer.c (file_mode, current_mode,
	formatted_transfer_scalar_read, formatted_transfer_scalar_write,
	pre_position, next_record_r, next_record_w): Add and use
	FORMATTED_UNSPECIFIED to enumeration.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/io.h
    trunk/libgfortran/io/transfer.c
Comment 7 Jerry DeLisle 2019-10-02 02:35:46 UTC
Author: jvdelisle
Date: Wed Oct  2 02:35:14 2019
New Revision: 276439

URL: https://gcc.gnu.org/viewcvs?rev=276439&root=gcc&view=rev
Log:
2019-10-01  Jerry DeLisle  <jvdelisle@gcc.gnu.org>

	PR libfortran/91593
	* io/read.c (read_decimal): Cast constant to size_t to turn off
	a bogus warning.
	* io/write.c (btoa_big): Use memset in lieu of setting the null
	byte in a string buffer to turn off a bogus warning.

Modified:
    trunk/libgfortran/ChangeLog
    trunk/libgfortran/io/read.c
    trunk/libgfortran/io/write.c
Comment 8 Eric Gallager 2019-10-19 05:40:20 UTC
(In reply to Jerry DeLisle from comment #7)
> Author: jvdelisle
> Date: Wed Oct  2 02:35:14 2019
> New Revision: 276439
> 
> URL: https://gcc.gnu.org/viewcvs?rev=276439&root=gcc&view=rev
> Log:
> 2019-10-01  Jerry DeLisle  <jvdelisle@gcc.gnu.org>
> 
> 	PR libfortran/91593
> 	* io/read.c (read_decimal): Cast constant to size_t to turn off
> 	a bogus warning.
> 	* io/write.c (btoa_big): Use memset in lieu of setting the null
> 	byte in a string buffer to turn off a bogus warning.
> 
> Modified:
>     trunk/libgfortran/ChangeLog
>     trunk/libgfortran/io/read.c
>     trunk/libgfortran/io/write.c

Did this fix it?
Comment 9 Jerry DeLisle 2019-10-19 15:49:13 UTC
As far as I can tell, fixed on trunk.