Re: [PATCH v2 05/13] qapi/docs: adjust stub member insertion algorithm

[Markus Armbruster <armbru@redhat.com>]

John Snow <jsnow@redhat.com> writes:

> On Mon, May 4, 2026 at 7:20 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> I'm feeling dense again.  Please be patient with me.
>
> It's okay, this is a complex one. This is part of the reason for
> pursuing strict ordering to begin with: the insertion algorithm is
> complex and ugly.
>
>>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > A forthcoming patch removes the implicit PLAIN section that always
>> > starts a QAPIDoc section list. Further future changes begin converting
>> > "PLAIN" sections to "INTRO" sections. To accommodate this, the insertion
>> > algorithm that places stub and dummy members must be adjusted to cope.
>>
>> What are the stub and dummy members?
>
> "stub" - undocumented members.
> "dummy" - placeholder section named q_dummy that causes "The members
> of ..." references to be printed in the rendered documentation.

Got it, thanks.

I figure synthesized "Returns:" can also be effected.

>> > This algorithm can handle zero-or-more PLAIN *or* INTRO sections at the
>> > beginning of a QAPIDoc object.
>>
>> The revised algorithm, I presume.
>>
>> What's the structure of its valid input before and after this patch?
>
> Before: Plain EverythingElse?
> After: (Plain* | Intro*) EverythingElse?
>
> Where EverythingElse may never start with "Intro", and may contain
> "Plain" but not as the first token when following a Plain section from
> the prior production. (Because contiguous plain sections are merged by
> the parser into one section.)
>
> This is to allow a gradual conversion. Once everything is fully
> converted, and especially after "details" is introduced and the strict
> ordering is enforced, this is tightened down considerably to: Intro?
> EverythingElse?
>
> where EverythingElse may no longer contain Plain (It is removed), may
> never contain Intro, and may only contain one Details section in the
> appropriate position (Near the end, before Since.)
>
> More or less: this patch removes the assumption that every QAPIDoc
> starts with exactly one Plain section and allows it to cope with any
> number of Intro/Plain sections at the beginning, but makes no change
> to what the parser actually produces or accepts. In effect, we go
> from:
>
> Plain EverythingElse
>
> to, in the next patch:
>
> Plain? EverythingElse
>
> then as the conversion continues, one of these two:
>
> Intro Plain? EverythingElse? (Without converted intro)
> Intro EverythingElse? (With converted intro)
>
> ... This entire insertion algorithm gets to be removed once we enforce
> strict ordering, because we can insert directly to the correct
> position in the list thereafter. So, it is only a temporary complexity
> that exists for the sake of gradual conversion and piecemeal list
> review of each QAPI module.

Would working this into the commit message make sense?

>> > Since we have three places that need to insert stub members, take the
>> > opportunity to unify and deduplicate this code.
>>
>> Three?  I can only see two: Transmogrifier.visit_sections() and
>> QAPIDoc.connect_member().
>
> Bad wording on my part again. ensure_returns also inserts stub
> *sections*, not "members", sorry.
>
>>
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> >  docs/sphinx/qapidoc.py | 36 ++++++++---------
>> >  scripts/qapi/parser.py | 90 +++++++++++++++++++++++++++---------------
>> >  2 files changed, 75 insertions(+), 51 deletions(-)
>> >
>> > diff --git a/docs/sphinx/qapidoc.py b/docs/sphinx/qapidoc.py
>> > index 1f7c15b7075..70ab9cdc214 100644
>> > --- a/docs/sphinx/qapidoc.py
>> > +++ b/docs/sphinx/qapidoc.py
>> > @@ -349,30 +349,32 @@ def _get_target(
>> >                  )
>> >
>> >      def visit_sections(self, ent: QAPISchemaDefinition) -> None:
>> > +        # Generate a placeholder right after the member section(s) which
>> > +        # will be used to generate documentation for "The members of..."
>> > +        # pointers in the rendered document.
>> > +        # This is a temporary hack until the inliner is merged.
>> > +        if ent.doc:
>> > +            ent.doc.append_member_stub(
>> > +                QAPIDoc.ArgSection(
>> > +                    ent.doc.info, QAPIDoc.Kind.MEMBER, "q_dummy"
>> > +                )
>> > +            )
>> > +
>>
>> This hack is of the nastier sort: passing a QAPISchema to the doc
>> generator modifies it.
>>
>> Would it be possible to add this dummy always in QAPISchema?  Any
>> drawbacks?
>
> Well...
>
> I didn't like the idea of generating "q_dummy" stubs inside the
> parser, as it is an implementation detail of qapidoc. Though as you
> note, this leaks it back out anyway.
>
> Here's my argument: the entire "q_dummy" thing goes away with the
> inliner anyway, which is what I am actively working towards, and this
> ugliness goes away entirely either way: we do not need q_dummy, we do
> not need member pointer stubs, we will not need to modify the caller's
> section list.
>
> I found this easier to do, despite the ugliness. Also consider that in
> this case, we are building an isolated schema directly inside of the
> Sphinx process anyway, so we are spiritually already "modifying our
> own copy" - i.e. there's no chance that this stuff leaks out into
> other users of the Schema. I think that's actually quite appropriate
> and unlikely to cause unintended consequences.

I think this is really a QAPIDoc defect.

The doc parser knows where the (empty) arguments are.

QAPIDoc also knows, but makes it hard to retrieve.

The old code searches QAPIDoc.all_sections to rediscover the spot.
Moderately ugly, relies on the well-known possible sequence of sections
in all_sections.

Your patch factors out the search, runs it once, and hacks up
.all_sections to store the result.

If we want the result stored in .all_sections (or anywhere in QAPIDoc,
really), why not simply store it when we know it?

Sometimes I wish QAPIDoc had a structure more useful than flat list of
sections...

>> [Remainder left for later...]
>>