Re: [1/2] PR 78736: New warning -Wenum-conversion

[Joseph Myers <joseph at codesourcery dot 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