[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 09/36] tcg: Consolidate 3 bits into enum TCGTempKind
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH v2 09/36] tcg: Consolidate 3 bits into enum TCGTempKind |
Date: |
Fri, 24 Apr 2020 10:08:19 +0100 |
User-agent: |
Mutt/1.13.3 (2020-01-12) |
On Thu, Apr 23, 2020 at 04:11:14PM -0700, Richard Henderson wrote:
> 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.
It depends how many of these exceptions you have. If there are only a
small handful of exceptions like this, then you can use Pragmas to
selectively disable the warning in those few cases, while still
benefitting from it across the rest of the code. If there are alot
of such exceptions though, then I agree it is impractical.
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 :|
- [PATCH v2 07/36] tcg: Add tcg_gen_gvec_dup_tl, (continued)
- [PATCH v2 07/36] tcg: Add tcg_gen_gvec_dup_tl, Richard Henderson, 2020/04/21
- [PATCH v2 06/36] tcg: Remove tcg_gen_gvec_dup{8,16,32,64}i, Richard Henderson, 2020/04/21
- [PATCH v2 08/36] tcg: Improve vector tail clearing, Richard Henderson, 2020/04/21
- [PATCH v2 09/36] tcg: Consolidate 3 bits into enum TCGTempKind, Richard Henderson, 2020/04/21
[PATCH v2 12/36] tcg: Use tcg_constant_i32 with icount expander, Richard Henderson, 2020/04/21
[PATCH v2 11/36] tcg: Introduce TYPE_CONST temporaries, Richard Henderson, 2020/04/21
[PATCH v2 14/36] tcg: Use tcg_constant_{i32, vec} with tcg vec expanders, Richard Henderson, 2020/04/21
[PATCH v2 16/36] tcg: Rename struct tcg_temp_info to TempOptInfo, Richard Henderson, 2020/04/21
[PATCH v2 13/36] tcg: Use tcg_constant_{i32, i64} with tcg int expanders, Richard Henderson, 2020/04/21