John Snow <jsnow@xxxxxxxxxx> writes:
> On Mon, Oct 25, 2021 at 1:25 AM Markus Armbruster <armbru@xxxxxxxxxx> 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@xxxxxxxxxx>
>> ---
>> 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@xxxxxxxxxx>
Thanks!
