r312750 - [Sema] -Wtautological-compare: handle comparison of unsigned with 0S.

[Hans Wennborg via cfe-commits]

On Fri, Sep 8, 2017 at 2:26 PM, Friedman, Eli <efriedma at codeaurora.org> wrote:
> On 9/8/2017 2:18 PM, Hans Wennborg via cfe-commits wrote:
>>
>> On Fri, Sep 8, 2017 at 2:09 PM, Roman Lebedev <lebedev.ri at gmail.com>
>> wrote:
>>>
>>>
>>> Interesting. My first thought was to explicitly specify enum as signed:
>>>
>>> enum MediaDeviceType : signed int {
>>> MEDIA_DEVICE_TYPE_AUDIO_INPUT = 0,
>>> MEDIA_DEVICE_TYPE_VIDEO_INPUT,
>>> MEDIA_DEVICE_TYPE_AUDIO_OUTPUT,
>>> NUM_MEDIA_DEVICE_TYPES,
>>> };
>>>
>>> inline bool IsValidMediaDeviceType(MediaDeviceType type) {
>>> return type >= 0U && type < NUM_MEDIA_DEVICE_TYPES;
>>> }
>>>
>>> But it still seem to warn, and *that* sounds like a bug.
>>> Please open a new bugreport.
>>
>> I'm reporting it here :-)
>>
>>> As for different default signedness, i'm not sure what is there to do.
>>> Does
>>> not sound like a problem for this diagnostic to intentionally avoid to
>>> be honest.
>>
>> I think it is a problem for this warning. If a user sees the warning
>> and removes the "type >= 0" check, thinking that it was unnecessary
>> because the compiler told them so, they have now introduced a bug in
>> the code.
>
>
> Even if you declare the enum type as signed, it still isn't allowed to
> contain a negative number; that's undefined behavior.  You can check for
> this using ubsan's -fsanitize=enum.

Is it? I thought if you define it as signed, it falls under "For an
enumeration whose underlying type is fixed, the values of the
enumeration are the values of the underlying type." (c++11 7.2p7)

My standards-fu is weak though, so I could be wrong.


But I think you're right about the case when the underlying type is
not specified. Even if it happens to be 'int' on Windows, negative
values aren't allowed (unless there are negative enumerators). That
doesn't mean it won't happen though, and lots of code isn't UB-clean
:-/