| Message ID | CAAgBjM=S2EjhqVize+nBjgb_aCd3m+TqdOEp7XBC6hfcm-6tDw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] PR 78736: New warning -Wenum-conversion | expand |
On 05/02/2017 11:11 AM, Prathamesh Kulkarni wrote: > Hi, > The attached patch attempts to add option -Wenum-conversion for C and > objective-C similar to clang, which warns when an enum value of a type > is implicitly converted to enum value of another type and is enabled > by Wall. It seems quite useful. My only high-level concern is with the growing number of specialized warnings and options for each and their interaction. I've been working on -Wenum-assign patch that complains about assigning to an enum variables an integer constants that doesn't match any of the enumerators of the type. Testing revealed that the -Wenum-assign duplicated a subset of warnings already issued by -Wconversion enabled with -Wpedantic. I'm debating whether to suppress that part of -Wenum-assign altogether or only when -Wconversion and -Wpedantic are enabled. My point is that these dependencies tend to be hard to discover and understand, and the interactions tricky to get right (e.g., avoid duplicate warnings for similar but distinct problems). This is not meant to be a negative comment on your patch, but rather a comment about a general problem that might be worth starting to think about. One comment on the patch itself: + warning_at_rich_loc (&loc, 0, "implicit conversion from" + " enum type of %qT to %qT", checktype, type); Unlike C++, the C front end formats an enumerated type E using %qT as 'enum E' so the warning prints 'enum type of 'enum E'), duplicating the "enum" part. I would suggest to simplify that to: warning_at_rich_loc (&loc, 0, "implicit conversion from " "%qT to %qT", checktype, ... Martin PS As an example to illustrate my concern above, consider this: enum __attribute__ ((packed)) E { e1 = 1 }; enum F { f256 = 256 }; enum E e = f256; It triggers -Woverflow: warning: large integer implicitly truncated to unsigned type [-Woverflow] enum E e = f256; ^~~~ also my -Wenum-assign: warning: integer constant ‘256’ converted to ‘0’ due to limited range [0, 255] of type ‘‘enum E’’ [-Wassign-enum] enum E e = f256; ^~~~ and (IIUC) will trigger your new -Wenum-conversion. Martin
On 3 May 2017 at 03:28, Martin Sebor <msebor@gmail.com> wrote: > On 05/02/2017 11:11 AM, Prathamesh Kulkarni wrote: >> >> Hi, >> The attached patch attempts to add option -Wenum-conversion for C and >> objective-C similar to clang, which warns when an enum value of a type >> is implicitly converted to enum value of another type and is enabled >> by Wall. > > > It seems quite useful. My only high-level concern is with > the growing number of specialized warnings and options for each > and their interaction. > > I've been working on -Wenum-assign patch that complains about > assigning to an enum variables an integer constants that doesn't > match any of the enumerators of the type. Testing revealed that > the -Wenum-assign duplicated a subset of warnings already issued > by -Wconversion enabled with -Wpedantic. I'm debating whether > to suppress that part of -Wenum-assign altogether or only when > -Wconversion and -Wpedantic are enabled. > > My point is that these dependencies tend to be hard to discover > and understand, and the interactions tricky to get right (e.g., > avoid duplicate warnings for similar but distinct problems). > > This is not meant to be a negative comment on your patch, but > rather a comment about a general problem that might be worth > starting to think about. > > One comment on the patch itself: > > + warning_at_rich_loc (&loc, 0, "implicit conversion from" > + " enum type of %qT to %qT", checktype, type); > > Unlike C++, the C front end formats an enumerated type E using > %qT as 'enum E' so the warning prints 'enum type of 'enum E'), > duplicating the "enum" part. > > I would suggest to simplify that to: > > warning_at_rich_loc (&loc, 0, "implicit conversion from " > "%qT to %qT", checktype, ... > Thanks for the suggestions. I have updated the patch accordingly. Hmm the issue you pointed out of warnings interaction is indeed of concern. I was wondering then if we should merge this warning with -Wconversion instead of having a separate option -Wenum-conversion ? Although that will not really help with your example below. > Martin > > PS As an example to illustrate my concern above, consider this: > > enum __attribute__ ((packed)) E { e1 = 1 }; > enum F { f256 = 256 }; > > enum E e = f256; > > It triggers -Woverflow: > > warning: large integer implicitly truncated to unsigned type [-Woverflow] > enum E e = f256; > ^~~~ > > also my -Wenum-assign: > > warning: integer constant ‘256’ converted to ‘0’ due to limited range [0, > 255] of type ‘‘enum E’’ [-Wassign-enum] > enum E e = f256; > ^~~~ > > and (IIUC) will trigger your new -Wenum-conversion. Yep, on my branch it triggered -Woverflow and -Wenum-conversion. Running the example on clang shows a single warning, which they call as -Wconstant-conversion, which I suppose is similar to your -Wassign-enum. test-eg.c:3:12: warning: implicit conversion from 'int' to 'enum E' changes value from 256 to 0 [-Wconstant-conversion] enum E e = f256; ~ ^~~~ Thanks, Prathamesh > > Martin 2017-05-02 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> * doc/invoke.text: Document Wenum-conversion. * c-family/c.opt (Wenum-conversion): New option. * c/c-typeck.c (convert_for_assignment): Handle Wenum-conversion. testsuite/ * gcc.dg/Wenum-conversion.c: New test-case.diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 9ad2f6e1fcc..e04312ec253 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -492,6 +492,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 6f9909c6396..483b2008f7b 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -6309,6 +6309,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, 0, "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 0eeea7b3b87..79b1e175374 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -273,7 +273,7 @@ Objective-C and Objective-C++ Dialects}. -Wdisabled-optimization @gol -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol -Wno-div-by-zero -Wdouble-promotion -Wduplicated-cond @gol --Wempty-body -Wenum-compare -Wno-endif-labels -Wexpansion-to-defined @gol +-Wempty-body -Wenum-compare -Wenum-conversion -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 @@ -3754,6 +3754,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 @@ -5961,6 +5962,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." } */ +}
ping https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00161.html Thanks, Prathamesh On 3 May 2017 at 11:30, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > On 3 May 2017 at 03:28, Martin Sebor <msebor@gmail.com> wrote: >> On 05/02/2017 11:11 AM, Prathamesh Kulkarni wrote: >>> >>> Hi, >>> The attached patch attempts to add option -Wenum-conversion for C and >>> objective-C similar to clang, which warns when an enum value of a type >>> is implicitly converted to enum value of another type and is enabled >>> by Wall. >> >> >> It seems quite useful. My only high-level concern is with >> the growing number of specialized warnings and options for each >> and their interaction. >> >> I've been working on -Wenum-assign patch that complains about >> assigning to an enum variables an integer constants that doesn't >> match any of the enumerators of the type. Testing revealed that >> the -Wenum-assign duplicated a subset of warnings already issued >> by -Wconversion enabled with -Wpedantic. I'm debating whether >> to suppress that part of -Wenum-assign altogether or only when >> -Wconversion and -Wpedantic are enabled. >> >> My point is that these dependencies tend to be hard to discover >> and understand, and the interactions tricky to get right (e.g., >> avoid duplicate warnings for similar but distinct problems). >> >> This is not meant to be a negative comment on your patch, but >> rather a comment about a general problem that might be worth >> starting to think about. >> >> One comment on the patch itself: >> >> + warning_at_rich_loc (&loc, 0, "implicit conversion from" >> + " enum type of %qT to %qT", checktype, type); >> >> Unlike C++, the C front end formats an enumerated type E using >> %qT as 'enum E' so the warning prints 'enum type of 'enum E'), >> duplicating the "enum" part. >> >> I would suggest to simplify that to: >> >> warning_at_rich_loc (&loc, 0, "implicit conversion from " >> "%qT to %qT", checktype, ... >> > Thanks for the suggestions. I have updated the patch accordingly. > Hmm the issue you pointed out of warnings interaction is indeed of concern. > I was wondering then if we should merge this warning with -Wconversion > instead of having a separate option -Wenum-conversion ? Although that will not > really help with your example below. >> Martin >> >> PS As an example to illustrate my concern above, consider this: >> >> enum __attribute__ ((packed)) E { e1 = 1 }; >> enum F { f256 = 256 }; >> >> enum E e = f256; >> >> It triggers -Woverflow: >> >> warning: large integer implicitly truncated to unsigned type [-Woverflow] >> enum E e = f256; >> ^~~~ >> >> also my -Wenum-assign: >> >> warning: integer constant ‘256’ converted to ‘0’ due to limited range [0, >> 255] of type ‘‘enum E’’ [-Wassign-enum] >> enum E e = f256; >> ^~~~ >> >> and (IIUC) will trigger your new -Wenum-conversion. > Yep, on my branch it triggered -Woverflow and -Wenum-conversion. > Running the example on clang shows a single warning, which they call > as -Wconstant-conversion, which > I suppose is similar to your -Wassign-enum. > > test-eg.c:3:12: warning: implicit conversion from 'int' to 'enum E' > changes value from 256 to 0 [-Wconstant-conversion] > enum E e = f256; > ~ ^~~~ > > Thanks, > Prathamesh >> >> Martin
On 05/09/2017 07:24 AM, Prathamesh Kulkarni wrote: > ping https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00161.html > > Thanks, > Prathamesh > > On 3 May 2017 at 11:30, Prathamesh Kulkarni > <prathamesh.kulkarni@linaro.org> wrote: >> On 3 May 2017 at 03:28, Martin Sebor <msebor@gmail.com> wrote: >>> On 05/02/2017 11:11 AM, Prathamesh Kulkarni wrote: >>>> >>>> Hi, >>>> The attached patch attempts to add option -Wenum-conversion for C and >>>> objective-C similar to clang, which warns when an enum value of a type >>>> is implicitly converted to enum value of another type and is enabled >>>> by Wall. >>> >>> >>> It seems quite useful. My only high-level concern is with >>> the growing number of specialized warnings and options for each >>> and their interaction. >>> >>> I've been working on -Wenum-assign patch that complains about >>> assigning to an enum variables an integer constants that doesn't >>> match any of the enumerators of the type. Testing revealed that >>> the -Wenum-assign duplicated a subset of warnings already issued >>> by -Wconversion enabled with -Wpedantic. I'm debating whether >>> to suppress that part of -Wenum-assign altogether or only when >>> -Wconversion and -Wpedantic are enabled. >>> >>> My point is that these dependencies tend to be hard to discover >>> and understand, and the interactions tricky to get right (e.g., >>> avoid duplicate warnings for similar but distinct problems). >>> >>> This is not meant to be a negative comment on your patch, but >>> rather a comment about a general problem that might be worth >>> starting to think about. >>> >>> One comment on the patch itself: >>> >>> + warning_at_rich_loc (&loc, 0, "implicit conversion from" >>> + " enum type of %qT to %qT", checktype, type); >>> >>> Unlike C++, the C front end formats an enumerated type E using >>> %qT as 'enum E' so the warning prints 'enum type of 'enum E'), >>> duplicating the "enum" part. >>> >>> I would suggest to simplify that to: >>> >>> warning_at_rich_loc (&loc, 0, "implicit conversion from " >>> "%qT to %qT", checktype, ... >>> >> Thanks for the suggestions. I have updated the patch accordingly. >> Hmm the issue you pointed out of warnings interaction is indeed of concern. >> I was wondering then if we should merge this warning with -Wconversion >> instead of having a separate option -Wenum-conversion ? Although that will not >> really help with your example below. >>> Martin >>> >>> PS As an example to illustrate my concern above, consider this: >>> >>> enum __attribute__ ((packed)) E { e1 = 1 }; >>> enum F { f256 = 256 }; >>> >>> enum E e = f256; >>> >>> It triggers -Woverflow: >>> >>> warning: large integer implicitly truncated to unsigned type [-Woverflow] >>> enum E e = f256; >>> ^~~~ >>> >>> also my -Wenum-assign: >>> >>> warning: integer constant ‘256’ converted to ‘0’ due to limited range [0, >>> 255] of type ‘‘enum E’’ [-Wassign-enum] >>> enum E e = f256; >>> ^~~~ >>> >>> and (IIUC) will trigger your new -Wenum-conversion. >> Yep, on my branch it triggered -Woverflow and -Wenum-conversion. >> Running the example on clang shows a single warning, which they call >> as -Wconstant-conversion, which >> I suppose is similar to your -Wassign-enum. -Wassign-enum is a Clang warning too, it just isn't included in either -Wall or -Wextra. It warns when a constant is assigned to a variable of an enumerated type and is not representable in it. I enhanced it for GCC to also warn when the constant doesn't correspond to an enumerator in the type, but I'm starting to think that rather than adding yet another option to GCC it might be better to extend your -Wenum-conversion once it's committed to cover those cases (and also to avoid issuing multiple warnings for essentially the same problem). Let me ponder that some more. I can't approve patches but it looks good to me for the most part. There is one minor issue that needs to be corrected: + gcc_rich_location loc (location); + warning_at_rich_loc (&loc, 0, "implicit conversion from" + " %qT to %qT", checktype, type); Here the zero should be replaced with OPT_Wenum_conversion, otherwise the warning option won't be included in the message. Martin >> >> test-eg.c:3:12: warning: implicit conversion from 'int' to 'enum E' >> changes value from 256 to 0 [-Wconstant-conversion] >> enum E e = f256; >> ~ ^~~~ >> >> Thanks, >> Prathamesh >>> >>> Martin
On 05/09/2017 07:04 PM, Martin Sebor wrote: >>> > > -Wassign-enum is a Clang warning too, it just isn't included in > either -Wall or -Wextra. It warns when a constant is assigned > to a variable of an enumerated type and is not representable in > it. I enhanced it for GCC to also warn when the constant doesn't > correspond to an enumerator in the type, Note that that means that the warning will trigger in the common use case of using enums as bit flags, like: enum flags { F1 = 1 << 1, F2 = 1 << 2, F3 = 1 << 3 }; void foo () { enum flags f = 0; f = F1 | F2; } I was going to suggest adding an attribute to mark such enum types, and it turns out that Clang has one already: https://clang.llvm.org/docs/AttributeReference.html#flag-enum So, IMHO if we add the warning, IWBN to add the attribute as well. > but I'm starting to think > that rather than adding yet another option to GCC it might be better > to extend your -Wenum-conversion once it's committed to cover those > cases (and also to avoid issuing multiple warnings for essentially > the same problem). Let me ponder that some more. Thanks, Pedro Alves
On 9 May 2017 at 23:34, Martin Sebor <msebor@gmail.com> wrote: > On 05/09/2017 07:24 AM, Prathamesh Kulkarni wrote: >> >> ping https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00161.html >> >> Thanks, >> Prathamesh >> >> On 3 May 2017 at 11:30, Prathamesh Kulkarni >> <prathamesh.kulkarni@linaro.org> wrote: >>> >>> On 3 May 2017 at 03:28, Martin Sebor <msebor@gmail.com> wrote: >>>> >>>> On 05/02/2017 11:11 AM, Prathamesh Kulkarni wrote: >>>>> >>>>> >>>>> Hi, >>>>> The attached patch attempts to add option -Wenum-conversion for C and >>>>> objective-C similar to clang, which warns when an enum value of a type >>>>> is implicitly converted to enum value of another type and is enabled >>>>> by Wall. >>>> >>>> >>>> >>>> It seems quite useful. My only high-level concern is with >>>> the growing number of specialized warnings and options for each >>>> and their interaction. >>>> >>>> I've been working on -Wenum-assign patch that complains about >>>> assigning to an enum variables an integer constants that doesn't >>>> match any of the enumerators of the type. Testing revealed that >>>> the -Wenum-assign duplicated a subset of warnings already issued >>>> by -Wconversion enabled with -Wpedantic. I'm debating whether >>>> to suppress that part of -Wenum-assign altogether or only when >>>> -Wconversion and -Wpedantic are enabled. >>>> >>>> My point is that these dependencies tend to be hard to discover >>>> and understand, and the interactions tricky to get right (e.g., >>>> avoid duplicate warnings for similar but distinct problems). >>>> >>>> This is not meant to be a negative comment on your patch, but >>>> rather a comment about a general problem that might be worth >>>> starting to think about. >>>> >>>> One comment on the patch itself: >>>> >>>> + warning_at_rich_loc (&loc, 0, "implicit conversion from" >>>> + " enum type of %qT to %qT", checktype, >>>> type); >>>> >>>> Unlike C++, the C front end formats an enumerated type E using >>>> %qT as 'enum E' so the warning prints 'enum type of 'enum E'), >>>> duplicating the "enum" part. >>>> >>>> I would suggest to simplify that to: >>>> >>>> warning_at_rich_loc (&loc, 0, "implicit conversion from " >>>> "%qT to %qT", checktype, ... >>>> >>> Thanks for the suggestions. I have updated the patch accordingly. >>> Hmm the issue you pointed out of warnings interaction is indeed of >>> concern. >>> I was wondering then if we should merge this warning with -Wconversion >>> instead of having a separate option -Wenum-conversion ? Although that >>> will not >>> really help with your example below. >>>> >>>> Martin >>>> >>>> PS As an example to illustrate my concern above, consider this: >>>> >>>> enum __attribute__ ((packed)) E { e1 = 1 }; >>>> enum F { f256 = 256 }; >>>> >>>> enum E e = f256; >>>> >>>> It triggers -Woverflow: >>>> >>>> warning: large integer implicitly truncated to unsigned type >>>> [-Woverflow] >>>> enum E e = f256; >>>> ^~~~ >>>> >>>> also my -Wenum-assign: >>>> >>>> warning: integer constant ‘256’ converted to ‘0’ due to limited range >>>> [0, >>>> 255] of type ‘‘enum E’’ [-Wassign-enum] >>>> enum E e = f256; >>>> ^~~~ >>>> >>>> and (IIUC) will trigger your new -Wenum-conversion. >>> >>> Yep, on my branch it triggered -Woverflow and -Wenum-conversion. >>> Running the example on clang shows a single warning, which they call >>> as -Wconstant-conversion, which >>> I suppose is similar to your -Wassign-enum. > > > -Wassign-enum is a Clang warning too, it just isn't included in > either -Wall or -Wextra. It warns when a constant is assigned > to a variable of an enumerated type and is not representable in > it. I enhanced it for GCC to also warn when the constant doesn't > correspond to an enumerator in the type, but I'm starting to think > that rather than adding yet another option to GCC it might be better > to extend your -Wenum-conversion once it's committed to cover those > cases (and also to avoid issuing multiple warnings for essentially > the same problem). Let me ponder that some more. > > I can't approve patches but it looks good to me for the most part. > There is one minor issue that needs to be corrected: > > + gcc_rich_location loc (location); > + warning_at_rich_loc (&loc, 0, "implicit conversion from" > + " %qT to %qT", checktype, type); > > Here the zero should be replaced with OPT_Wenum_conversion, > otherwise the warning option won't be included in the message. Oops, sorry about that, updated in the attached patch. In the patch, I have left the warning in Wall, however I was wondering whether it should be in Wextra instead ? The warning triggered for icv.c in libgomp for following assignment: icv->run_sched_var = kind; because icv->run_sched_var was of type enum gomp_schedule_type and 'kind' was of type enum omp_sched_t. However although these enums have different names, they are structurally identical (same values), so the warning in this case, although not a false positive, seems a bit artificial ? Thanks, Prathamesh > > Martin > > >>> >>> test-eg.c:3:12: warning: implicit conversion from 'int' to 'enum E' >>> changes value from 256 to 0 [-Wconstant-conversion] >>> enum E e = f256; >>> ~ ^~~~ >>> >>> Thanks, >>> Prathamesh >>>> >>>> >>>> Martin > > 2017-05-10 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> * doc/invoke.text: Document Wenum-conversion. * c-family/c.opt (Wenum-conversion): New option. * c/c-typeck.c (convert_for_assignment): Handle Wenum-conversion. testsuite/ * gcc.dg/Wenum-conversion.c: New test-case.diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 9ad2f6e1fcc..e04312ec253 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -492,6 +492,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 6f9909c6396..d7570835c61 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -6309,6 +6309,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 0eeea7b3b87..79b1e175374 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -273,7 +273,7 @@ Objective-C and Objective-C++ Dialects}. -Wdisabled-optimization @gol -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol -Wno-div-by-zero -Wdouble-promotion -Wduplicated-cond @gol --Wempty-body -Wenum-compare -Wno-endif-labels -Wexpansion-to-defined @gol +-Wempty-body -Wenum-compare -Wenum-conversion -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 @@ -3754,6 +3754,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 @@ -5961,6 +5962,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." } */ +}
On 05/10/2017 06:19 AM, Prathamesh Kulkarni wrote: > On 9 May 2017 at 23:34, Martin Sebor <msebor@gmail.com> wrote: >> On 05/09/2017 07:24 AM, Prathamesh Kulkarni wrote: >>> >>> ping https://gcc.gnu.org/ml/gcc-patches/2017-05/msg00161.html >>> >>> Thanks, >>> Prathamesh >>> >>> On 3 May 2017 at 11:30, Prathamesh Kulkarni >>> <prathamesh.kulkarni@linaro.org> wrote: >>>> >>>> On 3 May 2017 at 03:28, Martin Sebor <msebor@gmail.com> wrote: >>>>> >>>>> On 05/02/2017 11:11 AM, Prathamesh Kulkarni wrote: >>>>>> >>>>>> >>>>>> Hi, >>>>>> The attached patch attempts to add option -Wenum-conversion for C and >>>>>> objective-C similar to clang, which warns when an enum value of a type >>>>>> is implicitly converted to enum value of another type and is enabled >>>>>> by Wall. >>>>> >>>>> >>>>> >>>>> It seems quite useful. My only high-level concern is with >>>>> the growing number of specialized warnings and options for each >>>>> and their interaction. >>>>> >>>>> I've been working on -Wenum-assign patch that complains about >>>>> assigning to an enum variables an integer constants that doesn't >>>>> match any of the enumerators of the type. Testing revealed that >>>>> the -Wenum-assign duplicated a subset of warnings already issued >>>>> by -Wconversion enabled with -Wpedantic. I'm debating whether >>>>> to suppress that part of -Wenum-assign altogether or only when >>>>> -Wconversion and -Wpedantic are enabled. >>>>> >>>>> My point is that these dependencies tend to be hard to discover >>>>> and understand, and the interactions tricky to get right (e.g., >>>>> avoid duplicate warnings for similar but distinct problems). >>>>> >>>>> This is not meant to be a negative comment on your patch, but >>>>> rather a comment about a general problem that might be worth >>>>> starting to think about. >>>>> >>>>> One comment on the patch itself: >>>>> >>>>> + warning_at_rich_loc (&loc, 0, "implicit conversion from" >>>>> + " enum type of %qT to %qT", checktype, >>>>> type); >>>>> >>>>> Unlike C++, the C front end formats an enumerated type E using >>>>> %qT as 'enum E' so the warning prints 'enum type of 'enum E'), >>>>> duplicating the "enum" part. >>>>> >>>>> I would suggest to simplify that to: >>>>> >>>>> warning_at_rich_loc (&loc, 0, "implicit conversion from " >>>>> "%qT to %qT", checktype, ... >>>>> >>>> Thanks for the suggestions. I have updated the patch accordingly. >>>> Hmm the issue you pointed out of warnings interaction is indeed of >>>> concern. >>>> I was wondering then if we should merge this warning with -Wconversion >>>> instead of having a separate option -Wenum-conversion ? Although that >>>> will not >>>> really help with your example below. >>>>> >>>>> Martin >>>>> >>>>> PS As an example to illustrate my concern above, consider this: >>>>> >>>>> enum __attribute__ ((packed)) E { e1 = 1 }; >>>>> enum F { f256 = 256 }; >>>>> >>>>> enum E e = f256; >>>>> >>>>> It triggers -Woverflow: >>>>> >>>>> warning: large integer implicitly truncated to unsigned type >>>>> [-Woverflow] >>>>> enum E e = f256; >>>>> ^~~~ >>>>> >>>>> also my -Wenum-assign: >>>>> >>>>> warning: integer constant ‘256’ converted to ‘0’ due to limited range >>>>> [0, >>>>> 255] of type ‘‘enum E’’ [-Wassign-enum] >>>>> enum E e = f256; >>>>> ^~~~ >>>>> >>>>> and (IIUC) will trigger your new -Wenum-conversion. >>>> >>>> Yep, on my branch it triggered -Woverflow and -Wenum-conversion. >>>> Running the example on clang shows a single warning, which they call >>>> as -Wconstant-conversion, which >>>> I suppose is similar to your -Wassign-enum. >> >> >> -Wassign-enum is a Clang warning too, it just isn't included in >> either -Wall or -Wextra. It warns when a constant is assigned >> to a variable of an enumerated type and is not representable in >> it. I enhanced it for GCC to also warn when the constant doesn't >> correspond to an enumerator in the type, but I'm starting to think >> that rather than adding yet another option to GCC it might be better >> to extend your -Wenum-conversion once it's committed to cover those >> cases (and also to avoid issuing multiple warnings for essentially >> the same problem). Let me ponder that some more. >> >> I can't approve patches but it looks good to me for the most part. >> There is one minor issue that needs to be corrected: >> >> + gcc_rich_location loc (location); >> + warning_at_rich_loc (&loc, 0, "implicit conversion from" >> + " %qT to %qT", checktype, type); >> >> Here the zero should be replaced with OPT_Wenum_conversion, >> otherwise the warning option won't be included in the message. > Oops, sorry about that, updated in the attached patch. > In the patch, I have left the warning in Wall, however I was wondering > whether it should be > in Wextra instead ? > The warning triggered for icv.c in libgomp for following assignment: > icv->run_sched_var = kind; > > because icv->run_sched_var was of type enum gomp_schedule_type and > 'kind' was of type enum omp_sched_t. > However although these enums have different names, they are > structurally identical (same values), > so the warning in this case, although not a false positive, seems a > bit artificial ? I'd say the warning is justified in this case, even if the two enums are clearly designed to be interchangeable. It will be a reminder to review code like it to make sure it is, in fact intended and correct. If it is, it's easy to suppress by an explicit cast. So based on this example alone I wouldn't feel compelled to remove it from -Wall just yet. FWIW, when I'm concerned about the impact of a new warning I build a few packages with it (usually Binutils, Glibc, the Linux kernel, GDB, Bash, and Busybox). That gives me some sense of what to expect. In the end it's a judgment call as different people have varying degrees of tolerance for these kinds of warnings. Martin > > Thanks, > Prathamesh >> >> Martin >> >> >>>> >>>> test-eg.c:3:12: warning: implicit conversion from 'int' to 'enum E' >>>> changes value from 256 to 0 [-Wconstant-conversion] >>>> enum E e = f256; >>>> ~ ^~~~ >>>> >>>> Thanks, >>>> Prathamesh >>>>> >>>>> >>>>> Martin >> >>
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.)
--
Joseph S. Myers
joseph@codesourcery.com
On 13 June 2017 at 01:47, Joseph Myers <joseph@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 ? 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 ? Also I tested the warning by compiling the kernel for x86_64 with allmodconifg (attached), and there have been quite few instances of the warning (attached). I have been through few cases which I don't think are false positives but I wonder then whether we should relegate the warning to Wextra instead ? Thanks, Prathamesh > > -- > Joseph S. Myers > joseph@codesourcery.com mm/page-writeback.c:2436:3: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/page-writeback.c:2458:3: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/page-writeback.c:2715:4: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/page-writeback.c:2762:3: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/page-writeback.c:2817:3: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/vmscan.c:2058:14: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/vmscan.c:2745:15: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/workingset.c:292:2: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/workingset.c:296:3: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/workingset.c:478:2: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/rmap.c:1161:2: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/rmap.c:1201:2: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/memcontrol.c:3653:12: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] mm/memcontrol.c:3656:16: warning: implicit conversion from ‘enum node_stat_item’ to ‘enum memcg_stat_item’ [-Wenum-conversion] drivers/acpi/dock.c:249:3: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum dock_callback_type’ [-Wenum-conversion] mm/zsmalloc.c:756:2: warning: implicit conversion from ‘enum fullness_group’ to ‘enum zs_stat_type’ [-Wenum-conversion] mm/zsmalloc.c:784:2: warning: implicit conversion from ‘enum fullness_group’ to ‘enum zs_stat_type’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] arch/x86/kvm/kvm_cache_regs.h:43:3: warning: implicit conversion from ‘enum kvm_reg_ex’ to ‘enum kvm_reg’ [-Wenum-conversion] sound/pci/ctxfi/ctmixer.c:944:30: warning: implicit conversion from ‘enum CT_SUM_CTL’ to ‘enum CT_AMIXER_CTL’ [-Wenum-conversion] sound/pci/ctxfi/ctmixer.c:974:27: warning: implicit conversion from ‘enum CT_SUM_CTL’ to ‘enum CT_AMIXER_CTL’ [-Wenum-conversion] drivers/block/drbd/drbd_nl.c:776:11: warning: implicit conversion from ‘enum drbd_state_rv’ to ‘enum drbd_ret_code’ [-Wenum-conversion] drivers/block/drbd/drbd_nl.c:778:11: warning: implicit conversion from ‘enum drbd_state_rv’ to ‘enum drbd_ret_code’ [-Wenum-conversion] drivers/block/drbd/drbd_nl.c:1913:10: warning: implicit conversion from ‘enum drbd_state_rv’ to ‘enum drbd_ret_code’ [-Wenum-conversion] drivers/block/drbd/drbd_nl.c:2168:11: warning: implicit conversion from ‘enum drbd_ret_code’ to ‘enum drbd_state_rv’ [-Wenum-conversion] drivers/block/drbd/drbd_nl.c:2664:10: warning: implicit conversion from ‘enum drbd_state_rv’ to ‘enum drbd_ret_code’ [-Wenum-conversion] drivers/block/drbd/drbd_nl.c:2770:11: warning: implicit conversion from ‘enum drbd_state_rv’ to ‘enum drbd_ret_code’ [-Wenum-conversion] ./include/linux/platform_data/dma-ep93xx.h:87:10: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] fs/nfsd/nfs4callback.c:451:11: warning: implicit conversion from ‘enum nfs_cb_opnum4’ to ‘enum nfs_opnum4’ [-Wenum-conversion] fs/nfsd/nfs4callback.c:530:9: warning: implicit conversion from ‘enum nfs_cb_opnum4’ to ‘enum nfs_opnum4’ [-Wenum-conversion] fs/nfsd/nfs4callback.c:619:9: warning: implicit conversion from ‘enum nfs_cb_opnum4’ to ‘enum nfs_opnum4’ [-Wenum-conversion] fs/nfsd/nfs4callback.c:676:9: warning: implicit conversion from ‘enum nfs_cb_opnum4’ to ‘enum nfs_opnum4’ [-Wenum-conversion] ./include/linux/platform_data/dma-ep93xx.h:87:10: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/dma/timb_dma.c:547:2: warning: implicit conversion from ‘enum dma_transfer_direction’ to ‘enum dma_data_direction’ [-Wenum-conversion] drivers/gpu/drm/amd/amdkfd/kfd_kernel_queue.c:186:3: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum kfd_preempt_type’ [-Wenum-conversion] drivers/gpu/drm/amd/amdgpu/../include/cgs_common.h:458:4: warning: implicit conversion from ‘enum amd_powergating_state’ to ‘enum amd_clockgating_state’ [-Wenum-conversion] drivers/gpu/drm/amd/amdgpu/../include/cgs_common.h:458:4: warning: implicit conversion from ‘enum amd_clockgating_state’ to ‘enum amd_powergating_state’ [-Wenum-conversion] drivers/gpu/drm/amd/amdgpu/../include/cgs_common.h:458:4: warning: implicit conversion from ‘enum amd_powergating_state’ to ‘enum amd_clockgating_state’ [-Wenum-conversion] drivers/gpu/drm/amd/amdgpu/../include/cgs_common.h:458:4: warning: implicit conversion from ‘enum amd_clockgating_state’ to ‘enum amd_powergating_state’ [-Wenum-conversion] drivers/gpu/drm/amd/amdgpu/../include/cgs_common.h:458:4: warning: implicit conversion from ‘enum amd_clockgating_state’ to ‘enum amd_powergating_state’ [-Wenum-conversion] drivers/gpu/drm/i915/intel_display.c:1098:37: warning: implicit conversion from ‘enum transcoder’ to ‘enum pipe’ [-Wenum-conversion] drivers/gpu/drm/i915/intel_display.c:4576:2: warning: implicit conversion from ‘enum transcoder’ to ‘enum pipe’ [-Wenum-conversion] drivers/gpu/drm/i915/intel_dp.c:3546:3: warning: implicit conversion from ‘enum pipe’ to ‘enum transcoder’ [-Wenum-conversion] drivers/gpu/drm/i915/intel_dp.c:3560:3: warning: implicit conversion from ‘enum pipe’ to ‘enum transcoder’ [-Wenum-conversion] ./include/linux/iio/iio.h:128:12: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum iio_shared_by’ [-Wenum-conversion] drivers/gpu/drm/i915/intel_hdmi.c:1156:3: warning: implicit conversion from ‘enum pipe’ to ‘enum transcoder’ [-Wenum-conversion] drivers/gpu/drm/i915/intel_hdmi.c:1175:3: warning: implicit conversion from ‘enum pipe’ to ‘enum transcoder’ [-Wenum-conversion] drivers/gpu/drm/i915/intel_sdvo.c:1465:3: warning: implicit conversion from ‘enum pipe’ to ‘enum transcoder’ [-Wenum-conversion] drivers/gpu/drm/i915/intel_sdvo.c:1476:3: warning: implicit conversion from ‘enum pipe’ to ‘enum transcoder’ [-Wenum-conversion] drivers/iio/adc/max9611.c:292:14: warning: implicit conversion from ‘enum max9611_conf_ids’ to ‘enum max9611_csa_gain’ [-Wenum-conversion] drivers/iio/dac/ad5592r-base.c:496:13: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum iio_shared_by’ [-Wenum-conversion] ./include/linux/iio/iio.h:128:12: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum iio_shared_by’ [-Wenum-conversion] ./include/linux/iio/iio.h:128:12: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum iio_shared_by’ [-Wenum-conversion] drivers/infiniband/hw/cxgb4/cq.c:158:16: warning: implicit conversion from ‘enum t4_bar2_qtype’ to ‘enum cxgb4_bar2_qtype’ [-Wenum-conversion] drivers/infiniband/hw/cxgb4/qp.c:267:19: warning: implicit conversion from ‘enum t4_bar2_qtype’ to ‘enum cxgb4_bar2_qtype’ [-Wenum-conversion] drivers/infiniband/hw/cxgb4/qp.c:270:19: warning: implicit conversion from ‘enum t4_bar2_qtype’ to ‘enum cxgb4_bar2_qtype’ [-Wenum-conversion] drivers/infiniband/hw/hfi1/qp.c:196:7: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum ib_mtu’ [-Wenum-conversion] drivers/infiniband/hw/mlx4/mad.c:1823:39: warning: implicit conversion from ‘enum mlx4_ib_qp_flags’ to ‘enum ib_qp_create_flags’ [-Wenum-conversion] drivers/infiniband/hw/mlx4/mad.c:1831:39: warning: implicit conversion from ‘enum mlx4_ib_qp_flags’ to ‘enum ib_qp_create_flags’ [-Wenum-conversion] drivers/infiniband/hw/mthca/mthca_cq.c:612:21: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum ib_wc_opcode’ [-Wenum-conversion] drivers/infiniband/sw/rxe/rxe.c:112:25: warning: implicit conversion from ‘enum rxe_device_param’ to ‘enum ib_atomic_cap’ [-Wenum-conversion] drivers/infiniband/sw/rxe/rxe.c:139:20: warning: implicit conversion from ‘enum rxe_port_param’ to ‘enum ib_port_state’ [-Wenum-conversion] drivers/infiniband/sw/rxe/rxe.c:140:22: warning: implicit conversion from ‘enum rxe_port_param’ to ‘enum ib_mtu’ [-Wenum-conversion] drivers/infiniband/sw/rxe/rxe.c:141:25: warning: implicit conversion from ‘enum rxe_port_param’ to ‘enum ib_mtu’ [-Wenum-conversion] drivers/infiniband/sw/rxe/rxe.c:159:5: warning: implicit conversion from ‘enum rxe_port_param’ to ‘enum ib_mtu’ [-Wenum-conversion] ./include/linux/kernel.h:756:12: warning: implicit conversion from ‘enum rxe_port_param’ to ‘enum ib_mtu’ [-Wenum-conversion] drivers/mtd/nand/sh_flctl.c:483:3: warning: implicit conversion from ‘enum dma_transfer_direction’ to ‘enum dma_data_direction’ [-Wenum-conversion] drivers/mtd/nand/sh_flctl.c:542:3: warning: implicit conversion from ‘enum dma_transfer_direction’ to ‘enum dma_data_direction’ [-Wenum-conversion] drivers/pci/host/pci-hyperv.c:1819:32: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum pci_message_type’ [-Wenum-conversion] drivers/pinctrl/pinctrl-lpc18xx.c:643:29: warning: implicit conversion from ‘enum lpc18xx_pin_config_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/pinctrl/pinctrl-lpc18xx.c:648:12: warning: implicit conversion from ‘enum lpc18xx_pin_config_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/pinctrl/pinctrl-max77620.c:56:12: warning: implicit conversion from ‘enum max77620_pinconf_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/pinctrl/pinctrl-max77620.c:59:12: warning: implicit conversion from ‘enum max77620_pinconf_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/pinctrl/pinctrl-max77620.c:62:12: warning: implicit conversion from ‘enum max77620_pinconf_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/pinctrl/pinctrl-max77620.c:65:12: warning: implicit conversion from ‘enum max77620_pinconf_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/pinctrl/pinctrl-max77620.c:68:12: warning: implicit conversion from ‘enum max77620_pinconf_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/pinctrl/pinctrl-max77620.c:71:12: warning: implicit conversion from ‘enum max77620_pinconf_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/scsi/bnx2fc/bnx2fc_fcoe.c:2322:14: warning: implicit conversion from ‘enum fip_mode’ to ‘enum fip_state’ [-Wenum-conversion] drivers/rtc/rtc-omap.c:567:21: warning: implicit conversion from ‘enum rtc_pin_config_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/rtc/rtc-omap.c:572:12: warning: implicit conversion from ‘enum rtc_pin_config_param’ to ‘enum pin_config_param’ [-Wenum-conversion] drivers/scsi/fcoe/fcoe.c:2244:9: warning: implicit conversion from ‘enum fip_mode’ to ‘enum fip_state’ [-Wenum-conversion] drivers/scsi/fcoe/fcoe_ctlr.c:153:12: warning: implicit conversion from ‘enum fip_state’ to ‘enum fip_mode’ [-Wenum-conversion] drivers/scsi/fcoe/fcoe_ctlr.c:457:3: warning: implicit conversion from ‘enum fip_mode’ to ‘enum fip_state’ [-Wenum-conversion] drivers/scsi/fcoe/fcoe_transport.c:908:20: warning: implicit conversion from ‘enum fip_state’ to ‘enum fip_mode’ [-Wenum-conversion] drivers/scsi/fnic/fnic_main.c:769:3: warning: implicit conversion from ‘enum fip_mode’ to ‘enum fip_state’ [-Wenum-conversion] drivers/scsi/fnic/fnic_main.c:781:3: warning: implicit conversion from ‘enum fip_mode’ to ‘enum fip_state’ [-Wenum-conversion] drivers/scsi/hisi_sas/hisi_sas_main.c:1351:4: warning: implicit conversion from ‘enum port_event’ to ‘enum phy_event’ [-Wenum-conversion] drivers/spi/spi-atmel.c:746:11: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/spi/spi-atmel.c:753:11: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/scsi/isci/request.c:1629:11: warning: implicit conversion from ‘enum sci_io_status’ to ‘enum sci_status’ [-Wenum-conversion] drivers/scsi/isci/request.c:1631:10: warning: implicit conversion from ‘enum sci_io_status’ to ‘enum sci_status’ [-Wenum-conversion] drivers/scsi/isci/request.c:3476:11: warning: implicit conversion from ‘enum sci_task_status’ to ‘enum sci_status’ [-Wenum-conversion] drivers/net/wimax/i2400m/control.c:961:8: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum i2400m_tlv’ [-Wenum-conversion] ./include/linux/platform_data/dma-ep93xx.h:87:10: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] ./include/linux/dma-mapping.h:410:32: warning: implicit conversion from ‘enum dma_transfer_direction’ to ‘enum dma_data_direction’ [-Wenum-conversion] ./include/linux/dma-mapping.h:411:34: warning: implicit conversion from ‘enum dma_transfer_direction’ to ‘enum dma_data_direction’ [-Wenum-conversion] ./include/linux/dma-mapping.h:411:34: warning: implicit conversion from ‘enum dma_transfer_direction’ to ‘enum dma_data_direction’ [-Wenum-conversion] drivers/scsi/libfc/fc_exch.c:285:7: warning: implicit conversion from ‘enum fc_class’ to ‘enum fc_sof’ [-Wenum-conversion] drivers/scsi/isci/host.c:2744:10: warning: implicit conversion from ‘enum sci_status’ to ‘enum sci_task_status’ [-Wenum-conversion] drivers/scsi/isci/host.c:2753:9: warning: implicit conversion from ‘enum sci_status’ to ‘enum sci_task_status’ [-Wenum-conversion] drivers/spi/spi-pic32.c:320:12: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/spi/spi-pic32.c:330:12: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/spi/spi-rspi.c:537:13: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/spi/spi-rspi.c:557:13: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/spi/spi-sh-msiof.c:687:13: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/spi/spi-sh-msiof.c:704:13: warning: implicit conversion from ‘enum dma_data_direction’ to ‘enum dma_transfer_direction’ [-Wenum-conversion] drivers/net/wireless/ath/ath6kl/wmi.c:1825:9: warning: implicit conversion from ‘enum <anonymous>’ to ‘enum wmi_data_hdr_data_type’ [-Wenum-conversion] drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.c:267:4: warning: implicit conversion from ‘enum cxgb4_dcb_state’ to ‘enum cxgb4_dcb_state_input’ [-Wenum-conversion] drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h:69:4: warning: implicit conversion from ‘enum cxgb4_dcb_state’ to ‘enum cxgb4_dcb_state_input’ [-Wenum-conversion] drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h:69:4: warning: implicit conversion from ‘enum cxgb4_dcb_state’ to ‘enum cxgb4_dcb_state_input’ [-Wenum-conversion] drivers/net/ethernet/chelsio/cxgb4/cxgb4_dcb.h:69:4: warning: implicit conversion from ‘enum cxgb4_dcb_state’ to ‘enum cxgb4_dcb_state_input’ [-Wenum-conversion] drivers/scsi/iscsi_tcp.c:801:10: warning: implicit conversion from ‘enum iscsi_host_param’ to ‘enum iscsi_param’ [-Wenum-conversion] drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c:2602:3: warning: implicit conversion from ‘enum mlxsw_sp_l3proto’ to ‘enum mlxsw_reg_ralxx_protocol’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_sp_commands.c:163:23: warning: implicit conversion from ‘enum tunnel_clss’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_sp_commands.c:165:24: warning: implicit conversion from ‘enum tunnel_clss’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_sp_commands.c:167:24: warning: implicit conversion from ‘enum tunnel_clss’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_sp_commands.c:169:27: warning: implicit conversion from ‘enum tunnel_clss’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_sp_commands.c:171:27: warning: implicit conversion from ‘enum tunnel_clss’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_vf.c:530:2: warning: implicit conversion from ‘enum qed_tunn_mode’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_vf.c:534:2: warning: implicit conversion from ‘enum qed_tunn_mode’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_vf.c:539:2: warning: implicit conversion from ‘enum qed_tunn_mode’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_vf.c:542:2: warning: implicit conversion from ‘enum qed_tunn_mode’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_vf.c:544:2: warning: implicit conversion from ‘enum qed_tunn_mode’ to ‘enum qed_tunn_clss’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_ll2.c:764:8: warning: implicit conversion from ‘enum core_tx_dest’ to ‘enum qed_ll2_tx_dest’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_roce.c:1196:10: warning: implicit conversion from ‘enum roce_mode’ to ‘enum roce_flavor’ [-Wenum-conversion] drivers/net/ethernet/qlogic/qed/qed_roce.c:1199:10: warning: implicit conversion from ‘enum roce_mode’ to ‘enum roce_flavor’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c:4182:3: warning: implicit conversion from ‘enum ia_css_csi2_port’ to ‘mipi_port_ID_t {aka const enum <anonymous>}’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c:6067:3: warning: implicit conversion from ‘enum ia_css_csi2_port’ to ‘mipi_port_ID_t {aka const enum <anonymous>}’ [-Wenum-conversion] drivers/staging/rtl8188eu/hal/rf_cfg.c:188:2: warning: implicit conversion from ‘enum rf90_radio_path’ to ‘enum rf_radio_path’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/input_system.c:843:2: warning: implicit conversion from ‘enum <anonymous>’ to ‘gp_device_ID_t {aka const enum <anonymous>}’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/input_system.c:845:2: warning: implicit conversion from ‘enum <anonymous>’ to ‘gp_device_ID_t {aka const enum <anonymous>}’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/input_system.c:861:25: warning: implicit conversion from ‘enum <anonymous>’ to ‘input_system_connection_t {aka enum <anonymous>}’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/input_system.c:1289:2: warning: implicit conversion from ‘enum <anonymous>’ to ‘gp_device_ID_t {aka const enum <anonymous>}’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/css2400/hive_isp_css_common/host/input_system.c:1292:2: warning: implicit conversion from ‘enum <anonymous>’ to ‘gp_device_ID_t {aka const enum <anonymous>}’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c:1026:2: warning: implicit conversion from ‘enum ia_css_csi2_port’ to ‘mipi_port_ID_t {aka enum <anonymous>}’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c:1036:2: warning: implicit conversion from ‘enum ia_css_csi2_port’ to ‘mipi_port_ID_t {aka enum <anonymous>}’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.c:2167:46: warning: implicit conversion from ‘mipi_port_ID_t {aka enum <anonymous>}’ to ‘enum ia_css_csi2_port’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:38:36: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:39:36: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:40:36: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:41:36: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:42:40: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:43:40: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:44:40: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:45:40: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:46:40: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:47:40: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:48:40: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:49:40: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:50:36: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:51:36: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.h:122:21: warning: implicit conversion from ‘enum ia_css_frame_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.h:122:21: warning: implicit conversion from ‘enum ia_css_frame_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.h:122:21: warning: implicit conversion from ‘enum ia_css_frame_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.h:122:21: warning: implicit conversion from ‘enum ia_css_frame_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.h:122:21: warning: implicit conversion from ‘enum ia_css_frame_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:55:41: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_compat_css20.h:122:21: warning: implicit conversion from ‘enum ia_css_frame_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_subdev.c:58:13: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5146:8: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5166:7: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] drivers/staging/media/atomisp/pci/atomisp2/atomisp_cmd.c:5342:8: warning: implicit conversion from ‘enum atomisp_input_format’ to ‘enum ia_css_stream_format’ [-Wenum-conversion] 2017-07-11 Prathamesh Kulkarni <prathamesh.kulkarni@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. * gcc.dg/Wenum-conversion-2.c: Likewise. * gcc.dg/Wenum-conversion-3.c: Likewise.diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 05766c47856..6b14203773e 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 4d067e96dd3..479380f5748 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -6314,6 +6314,32 @@ 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)) + { + tree v1 = TYPE_VALUES (type); + tree v2 = TYPE_VALUES (checktype); + + /* Do not warn if the enums have same range of values. */ + for (; v1 && v2; v1 = TREE_CHAIN (v1), v2 = TREE_CHAIN (v2)) + if (simple_cst_equal (TREE_VALUE (v1), TREE_VALUE (v2)) != 1) + break; + + if (v1 || v2) + { + 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 f2020f847ae..5fe35515852 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 @@ -3828,6 +3829,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 @@ -6074,6 +6076,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 valeu 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-2.c b/gcc/testsuite/gcc.dg/Wenum-conversion-2.c new file mode 100644 index 00000000000..a084ce4fd3c --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wenum-conversion-2.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-bogus "implicit conversion from .enum Y. to .enum X." } */ +enum Y obj2 = y1; + +enum X obj3; +void foo() +{ + obj3 = y2; /* { dg-bogus "implicit conversion from .enum Y. to .enum X." } */ +} + +void bar(enum X); +void f(void) +{ + bar (y1); /* { dg-bogus "implicit conversion from .enum Y. to .enum X." } */ +} diff --git a/gcc/testsuite/gcc.dg/Wenum-conversion-3.c b/gcc/testsuite/gcc.dg/Wenum-conversion-3.c new file mode 100644 index 00000000000..9a01bdf2b60 --- /dev/null +++ b/gcc/testsuite/gcc.dg/Wenum-conversion-3.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-Wenum-conversion" } */ + +enum X { x1 = 5, x2 }; +enum Y { y1 = 10, 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/gcc/testsuite/gcc.dg/Wenum-conversion.c b/gcc/testsuite/gcc.dg/Wenum-conversion.c new file mode 100644 index 00000000000..177770b4115 --- /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, y3 }; + +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 298b29e8d3e..ed678895059 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; }
On 07/11/2017 06:29 AM, Prathamesh Kulkarni wrote: > @@ -6074,6 +6076,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 valeu 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 Aside from the "valeu" typo, I think this would be more clearly phrased as Warn when a value of enumerated type is implicitly converted to a different enumerated type. This warning is enabled by @option{-Wall}. The rest of the documentation changes are OK. -Sandra
On 11 July 2017 at 17:59, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > On 13 June 2017 at 01:47, Joseph Myers <joseph@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 ? > > 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 ? > > Also I tested the warning by compiling the kernel for x86_64 with > allmodconifg (attached), and there > have been quite few instances of the warning (attached). I have been > through few cases which I don't think are false positives > but I wonder then whether we should relegate the warning to Wextra instead ? ping: https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00514.html Thanks, Prathamesh > > Thanks, > Prathamesh >> >> -- >> Joseph S. Myers >> joseph@codesourcery.com
On 1 August 2017 at 00:10, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > On 11 July 2017 at 17:59, Prathamesh Kulkarni > <prathamesh.kulkarni@linaro.org> wrote: >> On 13 June 2017 at 01:47, Joseph Myers <joseph@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 ? >> >> 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 ? >> >> Also I tested the warning by compiling the kernel for x86_64 with >> allmodconifg (attached), and there >> have been quite few instances of the warning (attached). I have been >> through few cases which I don't think are false positives >> but I wonder then whether we should relegate the warning to Wextra instead ? > ping: https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00514.html ping * 2 https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00514.html Thanks, Prathamesh > > Thanks, > Prathamesh >> >> Thanks, >> Prathamesh >>> >>> -- >>> Joseph S. Myers >>> joseph@codesourcery.com
On 8 August 2017 at 09:51, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > On 1 August 2017 at 00:10, Prathamesh Kulkarni > <prathamesh.kulkarni@linaro.org> wrote: >> On 11 July 2017 at 17:59, Prathamesh Kulkarni >> <prathamesh.kulkarni@linaro.org> wrote: >>> On 13 June 2017 at 01:47, Joseph Myers <joseph@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 ? >>> >>> 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 ? >>> >>> Also I tested the warning by compiling the kernel for x86_64 with >>> allmodconifg (attached), and there >>> have been quite few instances of the warning (attached). I have been >>> through few cases which I don't think are false positives >>> but I wonder then whether we should relegate the warning to Wextra instead ? >> ping: https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00514.html > ping * 2 https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00514.html ping * 3 https://gcc.gnu.org/ml/gcc-patches/2017-07/msg00514.html Thanks, Prathamesh > > Thanks, > Prathamesh >> >> Thanks, >> Prathamesh >>> >>> Thanks, >>> Prathamesh >>>> >>>> -- >>>> Joseph S. Myers >>>> joseph@codesourcery.com
On Tue, 11 Jul 2017, Prathamesh Kulkarni wrote: > On 13 June 2017 at 01:47, Joseph Myers <joseph@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.) > 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? Also note s/valeu/value/ in the documentation. -- Joseph S. Myers joseph@codesourcery.com
On 26 August 2017 at 04:15, Joseph Myers <joseph@codesourcery.com> wrote: > On Tue, 11 Jul 2017, Prathamesh Kulkarni wrote: > >> On 13 June 2017 at 01:47, Joseph Myers <joseph@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 > joseph@codesourcery.com 2017-10-01 Prathamesh Kulkarni <prathamesh.kulkarni@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; }
On Fri, 1 Sep 2017, Prathamesh Kulkarni wrote: > > 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 ? Whether an explicit conversion is OK depends on *the semantics of the individual values and the intended semantics of the code doing the conversion*. That is, for the semantics of whatever code converts unit_sign_s to unit_sign, is it indeed correct that an input of SIGN_S should result in an output of SIGN_PROCDEFINED, an input of SIGN_SS should result in an output of SIGN_SUPPRESS and an input of SIGN_SP should result in an output of SIGN_PLUS? That is not a question one should expect C front-end maintainers to have any expertise in. Thus, I strongly advise sending each patch that fixes the warning fallout for such conversions separately, CC:ing the maintainers of the relevant part of GCC, and including an explanation with each such patch of what the semantics are in the relevant code and why an explicit conversion is correct. It would also seem a good idea to me to make sure that each enum in question has a comment on its definition, saying that the values have to be kept consistent (in whatever way) with the values of another enum, because of the conversions in whatever function named in the constant, so that people editing either enum know they can't just insert values in the middle of it. -- Joseph S. Myers joseph@codesourcery.com
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 9ad2f6e1fcc..e04312ec253 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -492,6 +492,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 6f9909c6396..c9cde8d7fef 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -6309,6 +6309,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, 0, "implicit conversion from" + " enum type of %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 0eeea7b3b87..79b1e175374 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -273,7 +273,7 @@ Objective-C and Objective-C++ Dialects}. -Wdisabled-optimization @gol -Wno-discarded-qualifiers -Wno-discarded-array-qualifiers @gol -Wno-div-by-zero -Wdouble-promotion -Wduplicated-cond @gol --Wempty-body -Wenum-compare -Wno-endif-labels -Wexpansion-to-defined @gol +-Wempty-body -Wenum-compare -Wenum-conversion -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 @@ -3754,6 +3754,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 @@ -5961,6 +5962,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..4459109c7cb --- /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 type of .enum Y. to .enum X." } */ +enum Y obj2 = y1; + +enum X obj3; +void foo() +{ + obj3 = y2; /* { dg-warning "implicit conversion from enum type of .enum Y. to .enum X." } */ +} + +void bar(enum X); +void f(void) +{ + bar (y1); /* { dg-warning "implicit conversion from enum type of .enum Y. to .enum X." } */ +}