Re: [PATCH v2 09/36] tcg: Consolidate 3 bits into enum TCGTempKind

[Richard Henderson]

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.

With the latter, you only get a runtime failure, which can be more difficult to
find if you've missed one.

We do not always have the option of relying on -Wswitch, if there are other
compounding warnings such as uninitialized variables.

In this instance, we can rely on -Wswitch, and I see no reason to add a default
case.


r~