* [PATCH 0/2] qapi: Simplify enum generation
@ 2023-02-24 15:54 Philippe Mathieu-Daudé
2023-02-24 15:54 ` [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members() Philippe Mathieu-Daudé
2023-02-24 15:54 ` [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup() Philippe Mathieu-Daudé
0 siblings, 2 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-24 15:54 UTC (permalink / raw)
To: qemu-devel
Cc: Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Markus Armbruster, Juan Quintela, Philippe Mathieu-Daudé
QAPI generating enum count as part of the enum forces handling
impossible switch cases. Modify qapi/types.py to generate the
enum count as a definition.
Do not try to cover the unreachable 'default' case.
Clean files covering unreachable foo__MAX case.
Philippe Mathieu-Daudé (2):
qapi: Do not generate default switch case in
gen_visit_object_members()
qapi: Generate enum count as definition in gen_enum_lookup()
audio/audio.c | 6 ------
audio/audio_template.h | 3 ---
migration/migration.c | 2 --
replay/replay-input.c | 12 ------------
scripts/qapi/types.py | 11 +++++++----
scripts/qapi/visit.py | 2 --
softmmu/tpm-hmp-cmds.c | 2 --
ui/input-linux.c | 4 ----
ui/input.c | 6 ------
9 files changed, 7 insertions(+), 41 deletions(-)
--
2.38.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members()
2023-02-24 15:54 [PATCH 0/2] qapi: Simplify enum generation Philippe Mathieu-Daudé
@ 2023-02-24 15:54 ` Philippe Mathieu-Daudé
2023-02-27 12:40 ` Juan Quintela
2023-02-27 13:14 ` Markus Armbruster
2023-02-24 15:54 ` [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup() Philippe Mathieu-Daudé
1 sibling, 2 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-24 15:54 UTC (permalink / raw)
To: qemu-devel
Cc: Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Markus Armbruster, Juan Quintela, Philippe Mathieu-Daudé
When iterating over a QAPISchemaEnumType, all possible
values are covered. The 'default' switch case is unreachable,
remove it.
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
scripts/qapi/visit.py | 2 --
1 file changed, 2 deletions(-)
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();
}
''')
--
2.38.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup()
2023-02-24 15:54 [PATCH 0/2] qapi: Simplify enum generation Philippe Mathieu-Daudé
2023-02-24 15:54 ` [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members() Philippe Mathieu-Daudé
@ 2023-02-24 15:54 ` Philippe Mathieu-Daudé
2023-02-24 21:16 ` Richard Henderson
` (2 more replies)
1 sibling, 3 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-02-24 15:54 UTC (permalink / raw)
To: qemu-devel
Cc: Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Markus Armbruster, Juan Quintela, Philippe Mathieu-Daudé
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;
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
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
audio/audio.c | 6 ------
audio/audio_template.h | 3 ---
migration/migration.c | 2 --
replay/replay-input.c | 12 ------------
scripts/qapi/types.py | 11 +++++++----
softmmu/tpm-hmp-cmds.c | 2 --
ui/input-linux.c | 4 ----
ui/input.c | 6 ------
8 files changed, 7 insertions(+), 39 deletions(-)
diff --git a/audio/audio.c b/audio/audio.c
index 4290309d18..57130c44f6 100644
--- a/audio/audio.c
+++ b/audio/audio.c
@@ -2079,9 +2079,6 @@ void audio_create_pdos(Audiodev *dev)
CASE(SPICE, spice, );
#endif
CASE(WAV, wav, );
-
- case AUDIODEV_DRIVER__MAX:
- abort();
};
}
@@ -2227,9 +2224,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/audio/audio_template.h b/audio/audio_template.h
index 42b4712acb..58e1255d7a 100644
--- a/audio/audio_template.h
+++ b/audio/audio_template.h
@@ -369,9 +369,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/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/scripts/qapi/types.py b/scripts/qapi/types.py
index c39d054d2c..b24bcb40ad 100644
--- a/scripts/qapi/types.py
+++ b/scripts/qapi/types.py
@@ -86,16 +86,13 @@ def gen_enum_lookup(name: str,
def gen_enum(name: str,
members: List[QAPISchemaEnumMember],
prefix: Optional[str] = None) -> str:
- # 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,
@@ -103,6 +100,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/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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup()
2023-02-24 15:54 ` [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup() Philippe Mathieu-Daudé
@ 2023-02-24 21:16 ` Richard Henderson
2023-02-27 12:41 ` Juan Quintela
2023-02-27 13:10 ` Markus Armbruster
2 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2023-02-24 21:16 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, qemu-devel
Cc: Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Markus Armbruster, Juan Quintela
On 2/24/23 05:54, Philippe Mathieu-Daudé 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;
>
> 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
> Signed-off-by: Philippe Mathieu-Daudé<philmd@linaro.org>
> ---
Yay!
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members()
2023-02-24 15:54 ` [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members() Philippe Mathieu-Daudé
@ 2023-02-27 12:40 ` Juan Quintela
2023-02-27 13:14 ` Markus Armbruster
1 sibling, 0 replies; 9+ messages in thread
From: Juan Quintela @ 2023-02-27 12:40 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Markus Armbruster
Philippe Mathieu-Daudé <philmd@linaro.org> wrote:
> When iterating over a QAPISchemaEnumType, all possible
> values are covered. The 'default' switch case is unreachable,
> remove it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Althought my qapi is rusty.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup()
2023-02-24 15:54 ` [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup() Philippe Mathieu-Daudé
2023-02-24 21:16 ` Richard Henderson
@ 2023-02-27 12:41 ` Juan Quintela
2023-02-27 13:10 ` Markus Armbruster
2 siblings, 0 replies; 9+ messages in thread
From: Juan Quintela @ 2023-02-27 12:41 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Markus Armbruster
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;
>
> 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
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Juan Quintela <quintela@redhat.com>
This other is very nice.
Thanks, Juan.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup()
2023-02-24 15:54 ` [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup() Philippe Mathieu-Daudé
2023-02-24 21:16 ` Richard Henderson
2023-02-27 12:41 ` Juan Quintela
@ 2023-02-27 13:10 ` Markus Armbruster
2023-03-15 10:32 ` Philippe Mathieu-Daudé
2 siblings, 1 reply; 9+ messages in thread
From: Markus Armbruster @ 2023-02-27 13:10 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Juan Quintela
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 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;
I'm in favour. In fact, I've wanted to do this for a while, just never
got around to 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
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Falls apart when the enum is empty:
gcc -m64 -mcx16 -Iqga/qemu-ga.p -Iqga -I../qga -I. -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -std=gnu11 -O0 -g -isystem /work/armbru/qemu/linux-headers -isystem linux-headers -iquote . -iquote /work/armbru/qemu -iquote /work/armbru/qemu/include -iquote /work/armbru/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -MD -MQ qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o -MF qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o.d -o qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o -c qga/qga-qapi-emit-events.c
In file included from qga/qga-qapi-emit-events.c:14:
qga/qga-qapi-emit-events.h:20:1: error: empty enum is invalid
20 | } qga_QAPIEvent;
| ^
qga/qapi-schema.json does not define any events, so its (implicitly
defined) event enumeration comes out empty.
We could detect this, and either emit a dummy event, or suppress code
generation for events entirely.
Explicitly defined enumerations may also be empty. qapi-code-gen.rst
says:
Nothing prevents an empty enumeration, although it is probably not
useful.
We could forbid empty enumerations.
Speaking of qapi-code-gen.rst: it also needs an update. Search for
__MAX.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members()
2023-02-24 15:54 ` [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members() Philippe Mathieu-Daudé
2023-02-27 12:40 ` Juan Quintela
@ 2023-02-27 13:14 ` Markus Armbruster
1 sibling, 0 replies; 9+ messages in thread
From: Markus Armbruster @ 2023-02-27 13:14 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: qemu-devel, Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Markus Armbruster, Juan Quintela
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> When iterating over a QAPISchemaEnumType, all possible
> values are covered. The 'default' switch case is unreachable,
> remove it.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> scripts/qapi/visit.py | 2 --
> 1 file changed, 2 deletions(-)
>
> 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();
> }
> ''')
This results in a bunch of warnings like
warning: enumeration value ‘FOO__MAX’ not handled in switch [-Wswitch]
Obvious fix: squash into the next patch.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup()
2023-02-27 13:10 ` Markus Armbruster
@ 2023-03-15 10:32 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-03-15 10:32 UTC (permalink / raw)
To: Markus Armbruster
Cc: qemu-devel, Pavel Dovgalyuk, Paolo Bonzini, Michael Roth,
Dr. David Alan Gilbert, Stefan Berger, Gerd Hoffmann,
Juan Quintela, Eric Blake, Richard Henderson
On 27/2/23 14:10, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>
>> 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;
>
> I'm in favour. In fact, I've wanted to do this for a while, just never
> got around to 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
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>
> Falls apart when the enum is empty:
>
> gcc -m64 -mcx16 -Iqga/qemu-ga.p -Iqga -I../qga -I. -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -std=gnu11 -O0 -g -isystem /work/armbru/qemu/linux-headers -isystem linux-headers -iquote . -iquote /work/armbru/qemu -iquote /work/armbru/qemu/include -iquote /work/armbru/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -fstack-protector-strong -fPIE -MD -MQ qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o -MF qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o.d -o qga/qemu-ga.p/meson-generated_.._qga-qapi-emit-events.c.o -c qga/qga-qapi-emit-events.c
> In file included from qga/qga-qapi-emit-events.c:14:
> qga/qga-qapi-emit-events.h:20:1: error: empty enum is invalid
> 20 | } qga_QAPIEvent;
> | ^
While I could find that in the C++ standard, I couldn't in the C one.
I wonder if in that case QAPI should generate foo__MAX = 0. No code /
structure should use an enum type which doesn't contain any member...
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2023-03-15 10:33 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-24 15:54 [PATCH 0/2] qapi: Simplify enum generation Philippe Mathieu-Daudé
2023-02-24 15:54 ` [PATCH 1/2] qapi: Do not generate default switch case in gen_visit_object_members() Philippe Mathieu-Daudé
2023-02-27 12:40 ` Juan Quintela
2023-02-27 13:14 ` Markus Armbruster
2023-02-24 15:54 ` [PATCH 2/2] qapi: Generate enum count as definition in gen_enum_lookup() Philippe Mathieu-Daudé
2023-02-24 21:16 ` Richard Henderson
2023-02-27 12:41 ` Juan Quintela
2023-02-27 13:10 ` Markus Armbruster
2023-03-15 10:32 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.