qapi: new sphinx qapi domain pre-requisites

[PATCH 06/20] qapi/parser: fix comment parsing immediately following a doc block

Posted by John Snow 6 months, 2 weeks ago

If a comment immediately follows a doc block, the parser doesn't ignore
that token appropriately. Fix that.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/parser.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 41b9319e5cb..161768b8b96 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc':
                 line = self.get_doc_line()
                 first = False
 
-        self.accept(False)
+        self.accept()
         doc.end()
         return doc
 
-- 
2.44.0

Re: [PATCH 06/20] qapi/parser: fix comment parsing immediately following a doc block

Posted by Markus Armbruster 6 months, 1 week ago

John Snow <jsnow@redhat.com> writes:

> If a comment immediately follows a doc block, the parser doesn't ignore
> that token appropriately. Fix that.

Reproducer?

>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  scripts/qapi/parser.py | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> index 41b9319e5cb..161768b8b96 100644
> --- a/scripts/qapi/parser.py
> +++ b/scripts/qapi/parser.py
> @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc':
>                  line = self.get_doc_line()
>                  first = False
>  
> -        self.accept(False)
> +        self.accept()
>          doc.end()
>          return doc

Can't judge the fix without understanding the problem, and the problem
will be easier to understand for me with a reproducer.

Re: [PATCH 06/20] qapi/parser: fix comment parsing immediately following a doc block

Posted by John Snow 6 months, 1 week ago

On Thu, May 16, 2024 at 2:01 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > If a comment immediately follows a doc block, the parser doesn't ignore
> > that token appropriately. Fix that.
>
> Reproducer?
>
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  scripts/qapi/parser.py | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> > index 41b9319e5cb..161768b8b96 100644
> > --- a/scripts/qapi/parser.py
> > +++ b/scripts/qapi/parser.py
> > @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc':
> >                  line = self.get_doc_line()
> >                  first = False
> >
> > -        self.accept(False)
> > +        self.accept()
> >          doc.end()
> >          return doc
>
> Can't judge the fix without understanding the problem, and the problem
> will be easier to understand for me with a reproducer.
>

audio.json:

```
##
# = Audio
##

##
# @AudiodevPerDirectionOptions:
#
# General audio backend options that are used for both playback and
# recording.
#
```

Modify this excerpt to have a comment after the "= Audio" header, say for
instance if you were to take out that intro paragraph and transform it into
a comment that preceded the AudiodevPerDirectionOptions doc block.

e.g.

```
##
# = Audio
##

# Lorem Ipsum

##
# @AudiodevPerDirectionOptions:
```

the parser breaks because the line I changed that primes the next token is
still set to "not ignore comments", but that breaks the parser and gives a
rather unhelpful message:

../qapi/audio.json:13:1: junk after '##' at start of documentation comment

Encountered when converting developer commentary from documentation
paragraphs to mere QAPI comments.

--js

Re: [PATCH 06/20] qapi/parser: fix comment parsing immediately following a doc block

Posted by Markus Armbruster 5 months, 2 weeks ago

John Snow <jsnow@redhat.com> writes:

> On Thu, May 16, 2024 at 2:01 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > If a comment immediately follows a doc block, the parser doesn't ignore
>> > that token appropriately. Fix that.
>>
>> Reproducer?
>>
>> >
>> > Signed-off-by: John Snow <jsnow@redhat.com>
>> > ---
>> >  scripts/qapi/parser.py | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
>> > index 41b9319e5cb..161768b8b96 100644
>> > --- a/scripts/qapi/parser.py
>> > +++ b/scripts/qapi/parser.py
>> > @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc':
>> >                  line = self.get_doc_line()
>> >                  first = False
>> >
>> > -        self.accept(False)
>> > +        self.accept()
>> >          doc.end()
>> >          return doc
>>
>> Can't judge the fix without understanding the problem, and the problem
>> will be easier to understand for me with a reproducer.
>>
>
> audio.json:
>
> ```
> ##
> # = Audio
> ##
>
> ##
> # @AudiodevPerDirectionOptions:
> #
> # General audio backend options that are used for both playback and
> # recording.
> #
> ```
>
> Modify this excerpt to have a comment after the "= Audio" header, say for
> instance if you were to take out that intro paragraph and transform it into
> a comment that preceded the AudiodevPerDirectionOptions doc block.
>
> e.g.
>
> ```
> ##
> # = Audio
> ##
>
> # Lorem Ipsum
>
> ##
> # @AudiodevPerDirectionOptions:
> ```
>
> the parser breaks because the line I changed that primes the next token is
> still set to "not ignore comments", but that breaks the parser and gives a
> rather unhelpful message:
>
> ../qapi/audio.json:13:1: junk after '##' at start of documentation comment

When ._parse()'s loop sees a comment token in .tok, it expects it to be
the start of a doc comment block (because any other comments are to be
skipped).  It then calls .get_doc(), which expects the same.  The
unexpected comment token then triggers .get_doc()'s check for junk after
'##'.

> Encountered when converting developer commentary from documentation
> paragraphs to mere QAPI comments.

Your fix is correct.

It's actually a regression.  Please add

    Fixes: 3d035cd2cca6 (qapi: Rewrite doc comment parser)

to the commit message.

Let's add a reproducer to tests/qapi-schema/doc-good.json right in this
patch, e.g.

diff --git a/tests/qapi-schema/doc-good.json b/tests/qapi-schema/doc-good.json
index de38a386e8..8b39eb946a 100644
--- a/tests/qapi-schema/doc-good.json
+++ b/tests/qapi-schema/doc-good.json
@@ -55,6 +55,8 @@
 # - {braces}
 ##
 
+# Not a doc comment
+
 ##
 # @Enum:
 #

Re: [PATCH 06/20] qapi/parser: fix comment parsing immediately following a doc block

Posted by John Snow 5 months, 1 week ago

On Thu, Jun 13, 2024 at 10:32 AM Markus Armbruster <armbru@redhat.com>
wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > On Thu, May 16, 2024 at 2:01 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >
> >> John Snow <jsnow@redhat.com> writes:
> >>
> >> > If a comment immediately follows a doc block, the parser doesn't
> ignore
> >> > that token appropriately. Fix that.
> >>
> >> Reproducer?
> >>
> >> >
> >> > Signed-off-by: John Snow <jsnow@redhat.com>
> >> > ---
> >> >  scripts/qapi/parser.py | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
> >> > index 41b9319e5cb..161768b8b96 100644
> >> > --- a/scripts/qapi/parser.py
> >> > +++ b/scripts/qapi/parser.py
> >> > @@ -587,7 +587,7 @@ def get_doc(self) -> 'QAPIDoc':
> >> >                  line = self.get_doc_line()
> >> >                  first = False
> >> >
> >> > -        self.accept(False)
> >> > +        self.accept()
> >> >          doc.end()
> >> >          return doc
> >>
> >> Can't judge the fix without understanding the problem, and the problem
> >> will be easier to understand for me with a reproducer.
> >>
> >
> > audio.json:
> >
> > ```
> > ##
> > # = Audio
> > ##
> >
> > ##
> > # @AudiodevPerDirectionOptions:
> > #
> > # General audio backend options that are used for both playback and
> > # recording.
> > #
> > ```
> >
> > Modify this excerpt to have a comment after the "= Audio" header, say for
> > instance if you were to take out that intro paragraph and transform it
> into
> > a comment that preceded the AudiodevPerDirectionOptions doc block.
> >
> > e.g.
> >
> > ```
> > ##
> > # = Audio
> > ##
> >
> > # Lorem Ipsum
> >
> > ##
> > # @AudiodevPerDirectionOptions:
> > ```
> >
> > the parser breaks because the line I changed that primes the next token
> is
> > still set to "not ignore comments", but that breaks the parser and gives
> a
> > rather unhelpful message:
> >
> > ../qapi/audio.json:13:1: junk after '##' at start of documentation
> comment
>
> When ._parse()'s loop sees a comment token in .tok, it expects it to be
> the start of a doc comment block (because any other comments are to be
> skipped).  It then calls .get_doc(), which expects the same.  The
> unexpected comment token then triggers .get_doc()'s check for junk after
> '##'.
>
> > Encountered when converting developer commentary from documentation
> > paragraphs to mere QAPI comments.
>
> Your fix is correct.
>
> It's actually a regression.  Please add
>
>     Fixes: 3d035cd2cca6 (qapi: Rewrite doc comment parser)
>
> to the commit message.
>
> Let's add a reproducer to tests/qapi-schema/doc-good.json right in this
> patch, e.g.
>
> diff --git a/tests/qapi-schema/doc-good.json
> b/tests/qapi-schema/doc-good.json
> index de38a386e8..8b39eb946a 100644
> --- a/tests/qapi-schema/doc-good.json
> +++ b/tests/qapi-schema/doc-good.json
> @@ -55,6 +55,8 @@
>  # - {braces}
>  ##
>
> +# Not a doc comment
> +
>  ##
>  # @Enum:
>  #
>

Okey-dokey, all set.

--js