On 26 August 2017 at 04:15, Joseph Myers <jos...@codesourcery.com> wrote:
> On Tue, 11 Jul 2017, Prathamesh Kulkarni wrote:
>
>> On 13 June 2017 at 01:47, Joseph Myers <jos...@codesourcery.com> wrote:
>> > This is OK with one fix:
>> >
>> >> +C ObjC Var(warn_enum_conversion) Init(0) Warning LangEnabledBy(C
>> >> Objc,Wall)
>> >
>> > I believe the LangEnabledBy arguments are case-sensitive, so you need to
>> > have ObjC not Objc there for it to work correctly. (*.opt parsing isn't
>> > very good at detecting typos and giving errors rather than silently
>> > ignoring things.)
>> Hi,
>> Sorry for the late response, I was on a vacation.
>> The attached patch is rebased and bootstrap+tested on
>> x86_64-unknown-linux-gnu.
>> I have modified it slightly to not warn for enums with different names
>> but having same value ranges.
>> For eg:
>> enum e1 { e1_1, e1_2 };
>> enum e2 { e2_1, e2_2 };
>>
>> enum e1 x = e2_1;
>> With this version, there would be no warning for the above assignment
>> since both e1 and e2 have
>> same value ranges. Is that OK ?
>
> I don't really think that's a good heuristic. I see no particular reason
> to think that just because two enums have the same set of values, an
> implicit conversion between them is actually deliberate - corresponding
> values have the same semantics in some sense - rather than reflecting an
> underlying confusion in the code. (You could have different levels of the
> warning - and only warn in this case at a higher level - but I don't
> really think that makes sense either for this particular warning.)
Thanks for the suggestion, I have reverted this change in the attached patch.
>
>> The patch has following fallouts in the testsuite:
>>
>> a) libgomp:
>> I initially assume it was a false positive because I thought enum
>> gomp_schedule_type
>> and enum omp_sched_t have same value-ranges but it looks like omp_sched_t
>> has range [1, 4] while gomp_schedule_type has range [0, 4] with one
>> extra element.
>> Is the warning then correct for this case ?
>>
>> b) libgfortran:
>> i) Implicit conversion from unit_mode to file_mode
>> ii) Implicit conversion from unit_sign_s to unit_sign.
>> I suppose the warning is OK for these cases since unit_mode, file_mode
>> have different value-ranges and similarly for unit_sign_s, unit_sign ?
>
> If it's an implicit conversion between different enum types, the warning
> is correct. The more important question for the review is: is it correct
> to replace the implicit conversion by an explicit one? That is, for each
> value in the source type, what enumerator of the destination type has the
> same value, and do the semantics match in whatever way is required for the
> code in question?
Well, for instance unit_sign in libgfortran/io.h is defined as:
typedef enum
{ SIGN_PROCDEFINED, SIGN_SUPPRESS, SIGN_PLUS, SIGN_UNSPECIFIED }
unit_sign;
and unit_sign_s is defined:
typedef enum
{ SIGN_S, SIGN_SS, SIGN_SP }
unit_sign_s;
Since the enum types are different, I assume warnings for implicit
conversion from unit_sign_s to unit_sign would be correct ?
And since unit_sign_s is a subset of unit_sign in terms of
value-ranges, I assume replacing implicit by explicit conversion would
be OK ?
Thanks,
Prathamesh
>
> Also note s/valeu/value/ in the documentation.
>
> --
> Joseph S. Myers
> jos...@codesourcery.com
2017-10-01 Prathamesh Kulkarni <prathamesh.kulka...@linaro.org>
* doc/invoke.texi: Document -Wenum-conversion.
* c-family/c.opt (Wenum-conversion): New option.
* c/c-typeck.c (convert_for_assignment): Handle Wenum-conversion.
libgomp/
* icv.c (omp_set_schedule): Cast kind to enum gomp_schedule_type.
(omp_get_schedule): Cast icv->run_sched_var to enum omp_sched_t
before.
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.
testsuite/
* gcc.dg/Wenum-conversion.c: New test-case.
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 3435fe90cca..23a5d350bad 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -500,6 +500,10 @@ Wenum-compare
C ObjC C++ ObjC++ Var(warn_enum_compare) Init(-1) Warning LangEnabledBy(C
ObjC,Wall || Wc++-compat)
Warn about comparison of different enum types.
+Wenum-conversion
+C ObjC Var(warn_enum_conversion) Init(0) Warning LangEnabledBy(C ObjC,Wall)
+Warn about implicit conversion of enum types.
+
Werror
C ObjC C++ ObjC++
; Documented in common.opt
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 135dd9d665c..a46de0438e6 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -6341,6 +6341,20 @@ convert_for_assignment (location_t location, location_t
expr_loc, tree type,
}
}
+ if (warn_enum_conversion)
+ {
+ tree checktype = origtype != NULL_TREE ? origtype : rhstype;
+ if (checktype != error_mark_node
+ && TREE_CODE (checktype) == ENUMERAL_TYPE
+ && TREE_CODE (type) == ENUMERAL_TYPE
+ && TYPE_MAIN_VARIANT (checktype) != TYPE_MAIN_VARIANT (type))
+ {
+ gcc_rich_location loc (location);
+ warning_at_rich_loc (&loc, OPT_Wenum_conversion, "implicit conversion
from"
+ " %qT to %qT", checktype, type);
+ }
+ }
+
if (TYPE_MAIN_VARIANT (type) == TYPE_MAIN_VARIANT (rhstype))
return rhs;
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 64363e54a00..e6d2159d60d 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -276,7 +276,8 @@ Objective-C and Objective-C++ Dialects}.
-Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol
-Wno-div-by-zero -Wdouble-promotion @gol
-Wduplicated-branches -Wduplicated-cond @gol
--Wempty-body -Wenum-compare -Wno-endif-labels -Wexpansion-to-defined @gol
+-Wempty-body -Wenum-compare -Wenum-conversion @gol
+-Wno-endif-labels -Wexpansion-to-defined @gol
-Werror -Werror=* -Wextra-semi -Wfatal-errors @gol
-Wfloat-equal -Wformat -Wformat=2 @gol
-Wno-format-contains-nul -Wno-format-extra-args @gol
@@ -3831,6 +3832,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect
Options}.
-Wcomment @gol
-Wduplicate-decl-specifier @r{(C and Objective-C only)} @gol
-Wenum-compare @r{(in C/ObjC; this is on by default in C++)} @gol
+-Wenum-conversion @r{in C/ObjC;} @gol
-Wformat @gol
-Wint-in-bool-context @gol
-Wimplicit @r{(C and Objective-C only)} @gol
@@ -6084,6 +6086,12 @@ In C++ enumerated type mismatches in conditional
expressions are also
diagnosed and the warning is enabled by default. In C this warning is
enabled by @option{-Wall}.
+@item -Wenum-conversion @r{(C, Objective-C only)}
+@opindex Wenum-conversion
+@opindex Wno-enum-conversion
+Warn when an enum value of a type is implicitly converted to an enum of
+another type. This warning is enabled by @option{-Wall}.
+
@item -Wextra-semi @r{(C++, Objective-C++ only)}
@opindex Wextra-semi
@opindex Wno-extra-semi
diff --git a/gcc/testsuite/gcc.dg/Wenum-conversion.c
b/gcc/testsuite/gcc.dg/Wenum-conversion.c
new file mode 100644
index 00000000000..86033399b7d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wenum-conversion.c
@@ -0,0 +1,20 @@
+/* { dg-do compile } */
+/* { dg-options "-Wenum-conversion" } */
+
+enum X { x1, x2 };
+enum Y { y1, y2 };
+
+enum X obj = y1; /* { dg-warning "implicit conversion from .enum Y. to .enum
X." } */
+enum Y obj2 = y1;
+
+enum X obj3;
+void foo()
+{
+ obj3 = y2; /* { dg-warning "implicit conversion from .enum Y. to .enum X." }
*/
+}
+
+void bar(enum X);
+void f(void)
+{
+ bar (y1); /* { dg-warning "implicit conversion from .enum Y. to .enum X." }
*/
+}
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:
diff --git a/libgomp/icv.c b/libgomp/icv.c
index 233d6dbe10e..71e1f677fd7 100644
--- a/libgomp/icv.c
+++ b/libgomp/icv.c
@@ -87,14 +87,14 @@ omp_set_schedule (omp_sched_t kind, int chunk_size)
default:
return;
}
- icv->run_sched_var = kind;
+ icv->run_sched_var = (enum gomp_schedule_type) kind;
}
void
omp_get_schedule (omp_sched_t *kind, int *chunk_size)
{
struct gomp_task_icv *icv = gomp_icv (false);
- *kind = icv->run_sched_var;
+ *kind = (enum omp_sched_t) icv->run_sched_var;
*chunk_size = icv->run_sched_chunk_size;
}
