Re: [PATCH v4 01/75] qapi: add QAPITypeInfo struct definition

[Markus Armbruster <armbru@redhat.com>]

Marc-André Lureau <marcandre.lureau@redhat.com> writes:

> Introduce a minimal struct that pairs a QAPI type's internal name with
> its "masked" introspection name. Generated constants of this type will
> let QOM property registration carry a reliable reference to the QAPI
> schema and possibly other associated data.
>
> The fields are populated in following commits.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  include/qapi/qapi-type-info.h | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/include/qapi/qapi-type-info.h b/include/qapi/qapi-type-info.h
> new file mode 100644
> index 000000000000..97eb1d7bf490
> --- /dev/null
> +++ b/include/qapi/qapi-type-info.h
> @@ -0,0 +1,32 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#ifndef QAPI_TYPE_INFO_H
> +#define QAPI_TYPE_INFO_H
> +
> +#include "qapi/util.h"
> +
> +/**
> + * QAPITypeInfo - QAPI type metadata
> + *
> + * @name: QAPI type name (e.g. "str", "OnOffAuto", "int32List").
> + *        QOM uses this as the property type string.
> + * @masked_name: Name of the type in the QAPI introspection schema, assigned by
> + *        scripts/qapi code generator (e.g. "368").
> + *        NULL for implicit types.

I believe the last sentence is inaccurate.  I can't see any QAPITypeInfo
for implicit types, nor can I see any where .masked_name is null.

We discussed this in review of v2.  You asked whether I think it should
be mandatory, but I neglected to answer (my apologies), so you didn't
change anything for now.  Fair.

If we really want to permit null, we need a more accurate comment.  If
we don't, we can simply drop the sentence.

I guess the remaining motivation to permit null here comes from QOM
introspection [PATCH 08].  There, you add optional member @qapi-type to
ObjectPropertyInfo, documented to be absent if the type does not occur
in the output of query-qmp-schema.

@qapi-type is optional so we can QAPIfy QOM properties one by one.
Three cases:

1. Not QAPIfied: the property's ObjectPropertyInfo has no QAPITypeInfo
   (member .qapi_type is null).  @qapi-type is absent.

2. Fully QAPIfied: it has a QAPITypeInfo, and its .masked_name is
   non-null.  @qapi-type is .masked_name.

3. Badly QAPIfied: it has a QAPITypeInfo, but it's useless: .masked_name
   is null.  @qapi-type is absent.

The code to compute @qapi-type is written the obvious way, and the
obvious way just works for all three cases.  Regardless, I don't think
case 3 has a right to exist :)  Why would we bother to tack a
QAPITypeInfo to ObjectPropertyInfo when it's useless?

The only reason I can imagine is QAPIfication in two steps where the
first one tacks on the QAPITypeInfo, and the second one adds its
.masked_name.  We go to case 2 via case 3.  But why would we do step one
without step 2?  And if we do both, why not swap their order?

Am I missing something?

> + * @lookup: QEnumLookup for this type, or NULL for non-enum types.
> + *        visit_type_enum(), qapi_enum_parse(), and qapi_enum_lookup()
> + *        all rely on it.
> + * @list: The list-type counterpart, or NULL if none exists.
> + *        str_type_info.list points to strList_type_info.  qdev array
> + *        properties follow this to find the list type for their element.
> + */
> +typedef struct QAPITypeInfo {
> +    const char *name;
> +    const char *masked_name;
> +    const QEnumLookup *lookup;
> +    const struct QAPITypeInfo *list;
> +} QAPITypeInfo;
> +
> +#endif /* QAPI_TYPE_INFO_H */