On 4/23/20 10:24 AM, Daniel P. Berrangé wrote:
> On Thu, Apr 23, 2020 at 08:40:10AM -0700, Richard Henderson wrote:
>> On 4/23/20 2:00 AM, Philippe Mathieu-Daudé wrote:
>>>>> @@ -1885,12 +1896,17 @@ static char *tcg_get_arg_str_ptr(TCGContext *s,
>>>>> char *buf, int buf_size,
>>>>> {
>>>>> int idx = temp_idx(ts);
>>>>>
>>>>> - if (ts->temp_global) {
>>>>> + switch (ts->kind) {
>>>>> + case TEMP_FIXED:
>>>>> + case TEMP_GLOBAL:
>>>>> pstrcpy(buf, buf_size, ts->name);
>>>>> - } else if (ts->temp_local) {
>>>>> + break;
>>>>> + case TEMP_LOCAL:
>>>>> snprintf(buf, buf_size, "loc%d", idx - s->nb_globals);
>>>>> - } else {
>>>>> + break;
>>>>> + case TEMP_NORMAL:
>>>>> snprintf(buf, buf_size, "tmp%d", idx - s->nb_globals);
>>>>> + break;
>>>>> }
>>>>
>>>> Hmm, why this switch doesn't have:
>>>>
>>>> default:
>>>> g_assert_not_reached();
>>>>
>>>> like the other ones?
>>>
>>> ... then all switch should have a default case, as noticed Aleksandar.
>>
>> There's a bit of a conflict between wanting to use -Werror -Wswitch, and
>> making
>> sure every switch has a default.
>>
>> With the former, you get a compiler error of the form
>>
>> error: enumeration value ‘FOO’ not handled in switch
>>
>> which lets you easily find places that need adjustment enumerators are added.
>
> FYI, -Wswitch-enum can deal with this. This gives a warning about
> missing enum cases, even if there is a default statement:
>
> [quote]
> '-Wswitch-enum'
> Warn whenever a 'switch' statement has an index of enumerated type
> and lacks a 'case' for one or more of the named codes of that
> enumeration. 'case' labels outside the enumeration range also
> provoke warnings when this option is used. The only difference
> between '-Wswitch' and this option is that this option gives a
> warning about an omitted enumeration code even if there is a
> 'default' label.
This warning, IMO, is useless.
All you need is one enumeration with 100 elements -- e.g. TCGOp -- and you
certainly will not want to have to add all enumerators to every switch.
r~
