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

[Daniel P . Berrangé]

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.
[/quote]

If we want to have a default: in every switch, then we could also
use -Wswitch-default too !

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|