| Message ID | 20230315112811.22355-4-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: Simplify enum generation | expand |
* Philippe Mathieu-Daudé (philmd@linaro.org) wrote: > QAPI's gen_enum() generates QAPI enum values and the > number of this values (as foo__MAX). > The number of entries in an enum type is not part of > the enumerated values, but we generate it as such. > See for example: > > typedef enum OnOffAuto { > ON_OFF_AUTO_AUTO, > ON_OFF_AUTO_ON, > ON_OFF_AUTO_OFF, > ON_OFF_AUTO__MAX, <--------- > } OnOffAuto; > > Instead of declaring the enum count as the last enumerated > value, #define it, so it is not part of the enum. > The previous example becomes: > > typedef enum OnOffAuto { > ON_OFF_AUTO_AUTO, > ON_OFF_AUTO_ON, > ON_OFF_AUTO_OFF, > #define ON_OFF_AUTO__MAX 3 <--------- > } OnOffAuto; > > When iterating over a QAPISchemaEnumType, all possible > values are covered. The 'default' switch case generated in > gen_visit_object_members() is now unreachable, remove it. > > Since Clang enables the -Wswitch warning by default [*], > remove all pointless foo__MAX cases in switch statement, > in order to avoid: > > audio/audio.c:2231:10: error: case value not in enumerated type 'AudioFormat' (aka 'enum AudioFormat') [-Wswitch] > case AUDIO_FORMAT__MAX: > ^ > ui/input.c:233:14: error: case value not in enumerated type 'KeyValueKind' (aka 'enum KeyValueKind') [-Wswitch] > case KEY_VALUE_KIND__MAX: > ^ > ... > > [*] https://clang.llvm.org/docs/DiagnosticsReference.html#wswitch > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > Reviewed-by: Juan Quintela <quintela@redhat.com> > Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> > --- > docs/devel/qapi-code-gen.rst | 4 ++-- > scripts/qapi/types.py | 11 +++++++---- > scripts/qapi/visit.py | 2 -- > audio/audio_template.h | 3 --- > audio/audio.c | 6 ------ > migration/migration.c | 2 -- > replay/replay-input.c | 12 ------------ > softmmu/tpm-hmp-cmds.c | 2 -- > ui/input-linux.c | 4 ---- > ui/input.c | 6 ------ > 10 files changed, 9 insertions(+), 43 deletions(-) > > diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst > index d684c7c24d..45b0da448d 100644 > --- a/docs/devel/qapi-code-gen.rst > +++ b/docs/devel/qapi-code-gen.rst > @@ -227,7 +227,7 @@ optional 'prefix' member overrides PREFIX. > > The generated C enumeration constants have values 0, 1, ..., N-1 (in > QAPI schema order), where N is the number of values. There is an > -additional enumeration constant PREFIX__MAX with value N. > +additional definition constant PREFIX__MAX with value N. > > Do not use string or an integer type when an enumeration type can do > the job satisfactorily. > @@ -1825,7 +1825,7 @@ Example:: > > typedef enum example_QAPIEvent { > EXAMPLE_QAPI_EVENT_MY_EVENT, > - EXAMPLE_QAPI_EVENT__MAX, > + #define EXAMPLE_QAPI_EVENT__MAX 1 > } example_QAPIEvent; > > #define example_QAPIEvent_str(val) \ > diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py > index 7a7be7315f..6459a6f925 100644 > --- a/scripts/qapi/types.py > +++ b/scripts/qapi/types.py > @@ -88,16 +88,13 @@ def gen_enum(name: str, > members: List[QAPISchemaEnumMember], > prefix: Optional[str] = None) -> str: > assert members > - # append automatically generated _MAX value > - enum_members = members + [QAPISchemaEnumMember('_MAX', None)] > - > ret = mcgen(''' > > typedef enum %(c_name)s { > ''', > c_name=c_name(name)) > > - for memb in enum_members: > + for memb in members: > ret += memb.ifcond.gen_if() > ret += mcgen(''' > %(c_enum)s, > @@ -105,6 +102,12 @@ def gen_enum(name: str, > c_enum=c_enum_const(name, memb.name, prefix)) > ret += memb.ifcond.gen_endif() > > + ret += mcgen(''' > +#define %(c_name)s %(c_length)s > +''', > + c_name=c_enum_const(name, '_MAX', prefix), > + c_length=len(members)) > + > ret += mcgen(''' > } %(c_name)s; > ''', > diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py > index 26a584ee4c..f66a31a963 100644 > --- a/scripts/qapi/visit.py > +++ b/scripts/qapi/visit.py > @@ -159,8 +159,6 @@ def gen_visit_object_members(name: str, > > ret += var.ifcond.gen_endif() > ret += mcgen(''' > - default: > - abort(); > } > ''') > > diff --git a/audio/audio_template.h b/audio/audio_template.h > index e42326c20d..d545c03afb 100644 > --- a/audio/audio_template.h > +++ b/audio/audio_template.h > @@ -376,9 +376,6 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev) > #endif > case AUDIODEV_DRIVER_WAV: > return dev->u.wav.TYPE; > - > - case AUDIODEV_DRIVER__MAX: > - break; > } > abort(); > } > diff --git a/audio/audio.c b/audio/audio.c > index 70b096713c..ea372288eb 100644 > --- a/audio/audio.c > +++ b/audio/audio.c > @@ -2071,9 +2071,6 @@ void audio_create_pdos(Audiodev *dev) > CASE(SPICE, spice, ); > #endif > CASE(WAV, wav, ); > - > - case AUDIODEV_DRIVER__MAX: > - abort(); > }; > } > > @@ -2219,9 +2216,6 @@ int audioformat_bytes_per_sample(AudioFormat fmt) > case AUDIO_FORMAT_S32: > case AUDIO_FORMAT_F32: > return 4; > - > - case AUDIO_FORMAT__MAX: > - ; > } > abort(); > } > diff --git a/migration/migration.c b/migration/migration.c > index ae2025d9d8..bdadab3b5e 100644 > --- a/migration/migration.c > +++ b/migration/migration.c > @@ -2220,8 +2220,6 @@ bool migration_is_idle(void) > case MIGRATION_STATUS_DEVICE: > case MIGRATION_STATUS_WAIT_UNPLUG: > return false; > - case MIGRATION_STATUS__MAX: > - g_assert_not_reached(); Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com> > } > > return false; > diff --git a/replay/replay-input.c b/replay/replay-input.c > index 1147e3d34e..c6de8e33ac 100644 > --- a/replay/replay-input.c > +++ b/replay/replay-input.c > @@ -38,9 +38,6 @@ void replay_save_input_event(InputEvent *evt) > replay_put_dword(key->key->u.qcode.data); > replay_put_byte(key->down); > break; > - case KEY_VALUE_KIND__MAX: > - /* keep gcc happy */ > - break; > } > break; > case INPUT_EVENT_KIND_BTN: > @@ -58,9 +55,6 @@ void replay_save_input_event(InputEvent *evt) > replay_put_dword(move->axis); > replay_put_qword(move->value); > break; > - case INPUT_EVENT_KIND__MAX: > - /* keep gcc happy */ > - break; > } > } > > @@ -89,9 +83,6 @@ InputEvent *replay_read_input_event(void) > evt.u.key.data->key->u.qcode.data = (QKeyCode)replay_get_dword(); > evt.u.key.data->down = replay_get_byte(); > break; > - case KEY_VALUE_KIND__MAX: > - /* keep gcc happy */ > - break; > } > break; > case INPUT_EVENT_KIND_BTN: > @@ -109,9 +100,6 @@ InputEvent *replay_read_input_event(void) > evt.u.abs.data->axis = (InputAxis)replay_get_dword(); > evt.u.abs.data->value = replay_get_qword(); > break; > - case INPUT_EVENT_KIND__MAX: > - /* keep gcc happy */ > - break; > } > > return QAPI_CLONE(InputEvent, &evt); > diff --git a/softmmu/tpm-hmp-cmds.c b/softmmu/tpm-hmp-cmds.c > index 9ed6ad6c4d..5a354cf6ac 100644 > --- a/softmmu/tpm-hmp-cmds.c > +++ b/softmmu/tpm-hmp-cmds.c > @@ -52,8 +52,6 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict) > teo = ti->options->u.emulator.data; > monitor_printf(mon, ",chardev=%s", teo->chardev); > break; > - case TPM_TYPE__MAX: > - break; > } > monitor_printf(mon, "\n"); > c++; > diff --git a/ui/input-linux.c b/ui/input-linux.c > index e572a2e905..a6e7574422 100644 > --- a/ui/input-linux.c > +++ b/ui/input-linux.c > @@ -120,10 +120,6 @@ static bool input_linux_check_toggle(InputLinux *il) > return (il->keydown[KEY_LEFTCTRL] || > il->keydown[KEY_RIGHTCTRL]) && > il->keydown[KEY_SCROLLLOCK]; > - > - case GRAB_TOGGLE_KEYS__MAX: > - /* avoid gcc error */ > - break; > } > return false; > } > diff --git a/ui/input.c b/ui/input.c > index f2d1e7a3a7..ca8f49a403 100644 > --- a/ui/input.c > +++ b/ui/input.c > @@ -230,9 +230,6 @@ static void qemu_input_event_trace(QemuConsole *src, InputEvent *evt) > name = QKeyCode_str(key->key->u.qcode.data); > trace_input_event_key_qcode(idx, name, key->down); > break; > - case KEY_VALUE_KIND__MAX: > - /* keep gcc happy */ > - break; > } > break; > case INPUT_EVENT_KIND_BTN: > @@ -250,9 +247,6 @@ static void qemu_input_event_trace(QemuConsole *src, InputEvent *evt) > name = InputAxis_str(move->axis); > trace_input_event_abs(idx, name, move->value); > break; > - case INPUT_EVENT_KIND__MAX: > - /* keep gcc happy */ > - break; > } > } > > -- > 2.38.1 >
diff --git a/docs/devel/qapi-code-gen.rst b/docs/devel/qapi-code-gen.rst index d684c7c24d..45b0da448d 100644 --- a/docs/devel/qapi-code-gen.rst +++ b/docs/devel/qapi-code-gen.rst @@ -227,7 +227,7 @@ optional 'prefix' member overrides PREFIX. The generated C enumeration constants have values 0, 1, ..., N-1 (in QAPI schema order), where N is the number of values. There is an -additional enumeration constant PREFIX__MAX with value N. +additional definition constant PREFIX__MAX with value N. Do not use string or an integer type when an enumeration type can do the job satisfactorily. @@ -1825,7 +1825,7 @@ Example:: typedef enum example_QAPIEvent { EXAMPLE_QAPI_EVENT_MY_EVENT, - EXAMPLE_QAPI_EVENT__MAX, + #define EXAMPLE_QAPI_EVENT__MAX 1 } example_QAPIEvent; #define example_QAPIEvent_str(val) \ diff --git a/scripts/qapi/types.py b/scripts/qapi/types.py index 7a7be7315f..6459a6f925 100644 --- a/scripts/qapi/types.py +++ b/scripts/qapi/types.py @@ -88,16 +88,13 @@ def gen_enum(name: str, members: List[QAPISchemaEnumMember], prefix: Optional[str] = None) -> str: assert members - # append automatically generated _MAX value - enum_members = members + [QAPISchemaEnumMember('_MAX', None)] - ret = mcgen(''' typedef enum %(c_name)s { ''', c_name=c_name(name)) - for memb in enum_members: + for memb in members: ret += memb.ifcond.gen_if() ret += mcgen(''' %(c_enum)s, @@ -105,6 +102,12 @@ def gen_enum(name: str, c_enum=c_enum_const(name, memb.name, prefix)) ret += memb.ifcond.gen_endif() + ret += mcgen(''' +#define %(c_name)s %(c_length)s +''', + c_name=c_enum_const(name, '_MAX', prefix), + c_length=len(members)) + ret += mcgen(''' } %(c_name)s; ''', diff --git a/scripts/qapi/visit.py b/scripts/qapi/visit.py index 26a584ee4c..f66a31a963 100644 --- a/scripts/qapi/visit.py +++ b/scripts/qapi/visit.py @@ -159,8 +159,6 @@ def gen_visit_object_members(name: str, ret += var.ifcond.gen_endif() ret += mcgen(''' - default: - abort(); } ''') diff --git a/audio/audio_template.h b/audio/audio_template.h index e42326c20d..d545c03afb 100644 --- a/audio/audio_template.h +++ b/audio/audio_template.h @@ -376,9 +376,6 @@ AudiodevPerDirectionOptions *glue(audio_get_pdo_, TYPE)(Audiodev *dev) #endif case AUDIODEV_DRIVER_WAV: return dev->u.wav.TYPE; - - case AUDIODEV_DRIVER__MAX: - break; } abort(); } diff --git a/audio/audio.c b/audio/audio.c index 70b096713c..ea372288eb 100644 --- a/audio/audio.c +++ b/audio/audio.c @@ -2071,9 +2071,6 @@ void audio_create_pdos(Audiodev *dev) CASE(SPICE, spice, ); #endif CASE(WAV, wav, ); - - case AUDIODEV_DRIVER__MAX: - abort(); }; } @@ -2219,9 +2216,6 @@ int audioformat_bytes_per_sample(AudioFormat fmt) case AUDIO_FORMAT_S32: case AUDIO_FORMAT_F32: return 4; - - case AUDIO_FORMAT__MAX: - ; } abort(); } diff --git a/migration/migration.c b/migration/migration.c index ae2025d9d8..bdadab3b5e 100644 --- a/migration/migration.c +++ b/migration/migration.c @@ -2220,8 +2220,6 @@ bool migration_is_idle(void) case MIGRATION_STATUS_DEVICE: case MIGRATION_STATUS_WAIT_UNPLUG: return false; - case MIGRATION_STATUS__MAX: - g_assert_not_reached(); } return false; diff --git a/replay/replay-input.c b/replay/replay-input.c index 1147e3d34e..c6de8e33ac 100644 --- a/replay/replay-input.c +++ b/replay/replay-input.c @@ -38,9 +38,6 @@ void replay_save_input_event(InputEvent *evt) replay_put_dword(key->key->u.qcode.data); replay_put_byte(key->down); break; - case KEY_VALUE_KIND__MAX: - /* keep gcc happy */ - break; } break; case INPUT_EVENT_KIND_BTN: @@ -58,9 +55,6 @@ void replay_save_input_event(InputEvent *evt) replay_put_dword(move->axis); replay_put_qword(move->value); break; - case INPUT_EVENT_KIND__MAX: - /* keep gcc happy */ - break; } } @@ -89,9 +83,6 @@ InputEvent *replay_read_input_event(void) evt.u.key.data->key->u.qcode.data = (QKeyCode)replay_get_dword(); evt.u.key.data->down = replay_get_byte(); break; - case KEY_VALUE_KIND__MAX: - /* keep gcc happy */ - break; } break; case INPUT_EVENT_KIND_BTN: @@ -109,9 +100,6 @@ InputEvent *replay_read_input_event(void) evt.u.abs.data->axis = (InputAxis)replay_get_dword(); evt.u.abs.data->value = replay_get_qword(); break; - case INPUT_EVENT_KIND__MAX: - /* keep gcc happy */ - break; } return QAPI_CLONE(InputEvent, &evt); diff --git a/softmmu/tpm-hmp-cmds.c b/softmmu/tpm-hmp-cmds.c index 9ed6ad6c4d..5a354cf6ac 100644 --- a/softmmu/tpm-hmp-cmds.c +++ b/softmmu/tpm-hmp-cmds.c @@ -52,8 +52,6 @@ void hmp_info_tpm(Monitor *mon, const QDict *qdict) teo = ti->options->u.emulator.data; monitor_printf(mon, ",chardev=%s", teo->chardev); break; - case TPM_TYPE__MAX: - break; } monitor_printf(mon, "\n"); c++; diff --git a/ui/input-linux.c b/ui/input-linux.c index e572a2e905..a6e7574422 100644 --- a/ui/input-linux.c +++ b/ui/input-linux.c @@ -120,10 +120,6 @@ static bool input_linux_check_toggle(InputLinux *il) return (il->keydown[KEY_LEFTCTRL] || il->keydown[KEY_RIGHTCTRL]) && il->keydown[KEY_SCROLLLOCK]; - - case GRAB_TOGGLE_KEYS__MAX: - /* avoid gcc error */ - break; } return false; } diff --git a/ui/input.c b/ui/input.c index f2d1e7a3a7..ca8f49a403 100644 --- a/ui/input.c +++ b/ui/input.c @@ -230,9 +230,6 @@ static void qemu_input_event_trace(QemuConsole *src, InputEvent *evt) name = QKeyCode_str(key->key->u.qcode.data); trace_input_event_key_qcode(idx, name, key->down); break; - case KEY_VALUE_KIND__MAX: - /* keep gcc happy */ - break; } break; case INPUT_EVENT_KIND_BTN: @@ -250,9 +247,6 @@ static void qemu_input_event_trace(QemuConsole *src, InputEvent *evt) name = InputAxis_str(move->axis); trace_input_event_abs(idx, name, move->value); break; - case INPUT_EVENT_KIND__MAX: - /* keep gcc happy */ - break; } }
QAPI's gen_enum() generates QAPI enum values and the number of this values (as foo__MAX). The number of entries in an enum type is not part of the enumerated values, but we generate it as such. See for example: typedef enum OnOffAuto { ON_OFF_AUTO_AUTO, ON_OFF_AUTO_ON, ON_OFF_AUTO_OFF, ON_OFF_AUTO__MAX, <--------- } OnOffAuto; Instead of declaring the enum count as the last enumerated value, #define it, so it is not part of the enum. The previous example becomes: typedef enum OnOffAuto { ON_OFF_AUTO_AUTO, ON_OFF_AUTO_ON, ON_OFF_AUTO_OFF, #define ON_OFF_AUTO__MAX 3 <--------- } OnOffAuto; When iterating over a QAPISchemaEnumType, all possible values are covered. The 'default' switch case generated in gen_visit_object_members() is now unreachable, remove it. Since Clang enables the -Wswitch warning by default [*], remove all pointless foo__MAX cases in switch statement, in order to avoid: audio/audio.c:2231:10: error: case value not in enumerated type 'AudioFormat' (aka 'enum AudioFormat') [-Wswitch] case AUDIO_FORMAT__MAX: ^ ui/input.c:233:14: error: case value not in enumerated type 'KeyValueKind' (aka 'enum KeyValueKind') [-Wswitch] case KEY_VALUE_KIND__MAX: ^ ... [*] https://clang.llvm.org/docs/DiagnosticsReference.html#wswitch Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Juan Quintela <quintela@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- docs/devel/qapi-code-gen.rst | 4 ++-- scripts/qapi/types.py | 11 +++++++---- scripts/qapi/visit.py | 2 -- audio/audio_template.h | 3 --- audio/audio.c | 6 ------ migration/migration.c | 2 -- replay/replay-input.c | 12 ------------ softmmu/tpm-hmp-cmds.c | 2 -- ui/input-linux.c | 4 ---- ui/input.c | 6 ------ 10 files changed, 9 insertions(+), 43 deletions(-)