John Snow <jsnow(a)redhat.com> writes:
On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster
<armbru(a)redhat.com> wrote:
> New enum QapiSpecialFeature enumerates the special feature flags.
>
> New helper gen_special_features() returns code to represent a
> collection of special feature flags as a bitset.
>
> The next few commits will put them to use.
>
> Signed-off-by: Markus Armbruster <armbru(a)redhat.com>
> ---
> include/qapi/util.h | 4 ++++
> scripts/qapi/gen.py | 13 +++++++++++++
> scripts/qapi/schema.py | 3 +++
> 3 files changed, 20 insertions(+)
>
> diff --git a/include/qapi/util.h b/include/qapi/util.h
> index 257c600f99..7a8d5c7d72 100644
> --- a/include/qapi/util.h
> +++ b/include/qapi/util.h
> @@ -11,6 +11,10 @@
> #ifndef QAPI_UTIL_H
> #define QAPI_UTIL_H
>
> +typedef enum {
> + QAPI_DEPRECATED,
> +} QapiSpecialFeature;
> +
> /* QEnumLookup flags */
> #define QAPI_ENUM_DEPRECATED 1
>
> diff --git a/scripts/qapi/gen.py b/scripts/qapi/gen.py
> index 2ec1e7b3b6..9d07b88cf6 100644
> --- a/scripts/qapi/gen.py
> +++ b/scripts/qapi/gen.py
> @@ -29,6 +29,7 @@
> mcgen,
> )
> from .schema import (
> + QAPISchemaFeature,
> QAPISchemaIfCond,
> QAPISchemaModule,
> QAPISchemaObjectType,
> @@ -37,6 +38,18 @@
> from .source import QAPISourceInfo
>
>
> +def gen_special_features(features: QAPISchemaFeature):
> + ret = ''
> + sep = ''
> +
> + for feat in features:
> + if feat.is_special():
> + ret += ('%s1u << QAPI_%s' % (sep, feat.name.upper()))
>
Building the constant name here "feels" fragile, but I'll trust that the
test suite and/or the compiler will catch us if we accidentally goof up
this mapping. In the interest of simplicity, then, "sure, why not."
It relies on .is_special() remaining consistent with enum
QapiSpecialFeature. The C compiler should catch any screwups.
> + sep = ' | '
> +
>
+ return ret or '0'
> +
>
Subjectively more pythonic:
special_features = [f"1u << QAPI_{feat.name.upper()}" for feat in
features
if feat.is_special()]
ret = ' | '.join(special_features)
return ret or '0'
A bit more dense, but more functional. Up to you, but I find join() easier
to read and reason about for the presence of separators.
Sold!
> +
> class QAPIGen:
> def __init__(self, fname: str):
> self.fname = fname
> diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
> index 6d5f46509a..55f82d7389 100644
> --- a/scripts/qapi/schema.py
> +++ b/scripts/qapi/schema.py
> @@ -725,6 +725,9 @@ def connect_doc(self, doc):
> class QAPISchemaFeature(QAPISchemaMember):
> role = 'feature'
>
> + def is_special(self):
> + return self.name in ('deprecated')
> +
>
alrighty.
(Briefly wondered: is it worth naming special features as a property of the
class, but with only two names, it's probably fine enough to leave it
embedded in the method logic. Only a style thing and doesn't have any
actual impact that I can imagine, so ... nevermind.)
Let's keep it simple.
> class QAPISchemaObjectTypeMember(QAPISchemaMember):
> def __init__(self, name, info, typ, optional, ifcond=None,
> features=None):
> --
> 2.31.1
>
>
Well, either way:
Reviewed-by: John Snow <jsnow(a)redhat.com>
Thanks!