qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: John Snow <jsnow@redhat.com>
To: qemu-devel@nongnu.org
Cc: Eduardo Habkost <ehabkost@redhat.com>,
	Michael Roth <michael.roth@amd.com>,
	Markus Armbruster <armbru@redhat.com>,
	Eric Blake <eblake@redhat.com>, Cleber Rosa <crosa@redhat.com>,
	John Snow <jsnow@redhat.com>
Subject: [PATCH v4 07/13] qapi/parser: Introduce NullSection
Date: Thu, 30 Sep 2021 16:57:10 -0400	[thread overview]
Message-ID: <20210930205716.1148693-8-jsnow@redhat.com> (raw)
In-Reply-To: <20210930205716.1148693-1-jsnow@redhat.com>

Here's the weird bit. QAPIDoc generally expects -- virtually everywhere
-- that it will always have a current section. The sole exception to
this is in the case that end_comment() is called, which leaves us with
*no* section. However, in this case, we also don't expect to actually
ever mutate the comment contents ever again.

NullSection is just a Null-object that allows us to maintain the
invariant that we *always* have a current section, enforced by static
typing -- allowing us to type that field as QAPIDoc.Section instead of
the more ambiguous Optional[QAPIDoc.Section].

end_section is renamed to switch_section and now accepts as an argument
the new section to activate, clarifying that no callers ever just
unilaterally end a section; they only do so when starting a new section.

Signed-off-by: John Snow <jsnow@redhat.com>

---

For my money: Optional types can be a nuisance because an unfamiliar
reader may wonder in what circumstances the field may be unset. This
makes the condition quite a bit more explicit and statically provable.

Doing it in this way (and not by creating a mutable dummy section) will also
continue to reject (rather noisily) any erroneous attempts to append
additional lines after end_comment() has been called.

Also, this section isn't indexed into .sections[] and isn't really
visible in any way to external users of the class, so it seems like a
harmless and low-cost way to formalize the "life cycle" of a QAPIDoc
parser.

Clean and clear as I can make it, in as few lines as I could muster.

Signed-off-by: John Snow <jsnow@redhat.com>
---
 scripts/qapi/parser.py | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/scripts/qapi/parser.py b/scripts/qapi/parser.py
index 82f1d952b13..40c5da4b172 100644
--- a/scripts/qapi/parser.py
+++ b/scripts/qapi/parser.py
@@ -478,6 +478,13 @@ def __init__(self, parser, name, indent=0):
         def connect(self, member):
             self.member = member
 
+    class NullSection(Section):
+        """
+        Immutable dummy section for use at the end of a doc block.
+        """
+        def append(self, line):
+            assert False, "Text appended after end_comment() called."
+
     def __init__(self, parser, info):
         # self._parser is used to report errors with QAPIParseError.  The
         # resulting error position depends on the state of the parser.
@@ -525,7 +532,7 @@ def append(self, line):
         self._append_line(line)
 
     def end_comment(self):
-        self._end_section()
+        self._switch_section(QAPIDoc.NullSection(self._parser))
 
     @staticmethod
     def _is_section_tag(name):
@@ -699,9 +706,9 @@ def _start_symbol_section(self, symbols_dict, name, indent):
             raise QAPIParseError(self._parser,
                                  "'%s' parameter name duplicated" % name)
         assert not self.sections
-        self._end_section()
-        self._section = QAPIDoc.ArgSection(self._parser, name, indent)
-        symbols_dict[name] = self._section
+        new_section = QAPIDoc.ArgSection(self._parser, name, indent)
+        self._switch_section(new_section)
+        symbols_dict[name] = new_section
 
     def _start_args_section(self, name, indent):
         self._start_symbol_section(self.args, name, indent)
@@ -713,13 +720,11 @@ def _start_section(self, name=None, indent=0):
         if name in ('Returns', 'Since') and self.has_section(name):
             raise QAPIParseError(self._parser,
                                  "duplicated '%s' section" % name)
-        self._end_section()
-        self._section = QAPIDoc.Section(self._parser, name, indent)
-        self.sections.append(self._section)
-
-    def _end_section(self):
-        assert self._section is not None
+        new_section = QAPIDoc.Section(self._parser, name, indent)
+        self._switch_section(new_section)
+        self.sections.append(new_section)
 
+    def _switch_section(self, new_section):
         text = self._section.text = self._section.text.strip()
 
         # Only the 'body' section is allowed to have an empty body.
@@ -732,7 +737,7 @@ def _end_section(self):
                 self._parser,
                 "empty doc section '%s'" % self._section.name)
 
-        self._section = None
+        self._section = new_section
 
     def _append_freeform(self, line):
         match = re.match(r'(@\S+:)', line)
-- 
2.31.1



  parent reply	other threads:[~2021-09-30 21:09 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-30 20:57 [PATCH v4 00/13] qapi: static typing conversion, pt5b John Snow
2021-09-30 20:57 ` [PATCH v4 01/13] qapi/pylintrc: ignore 'consider-using-f-string' warning John Snow
2021-09-30 20:57 ` [PATCH v4 02/13] qapi/gen: use dict.items() to iterate over _modules John Snow
2021-09-30 20:57 ` [PATCH v4 03/13] qapi/parser: fix unused check_args_section arguments John Snow
2021-10-01 14:11   ` Markus Armbruster
2021-10-01 18:55     ` John Snow
2021-09-30 20:57 ` [PATCH v4 04/13] qapi: Add spaces after symbol declaration for consistency John Snow
2021-09-30 20:57 ` [PATCH v4 05/13] qapi/parser: remove FIXME comment from _append_body_line John Snow
2021-09-30 20:57 ` [PATCH v4 06/13] qapi/parser: clarify _end_section() logic John Snow
2021-09-30 20:57 ` John Snow [this message]
2021-09-30 20:57 ` [PATCH v4 08/13] qapi/parser: add import cycle workaround John Snow
2021-09-30 20:57 ` [PATCH v4 09/13] qapi/parser: add type hint annotations (QAPIDoc) John Snow
2021-09-30 20:57 ` [PATCH v4 10/13] qapi/parser: Add FIXME for consolidating JSON-related types John Snow
2021-09-30 20:57 ` [PATCH v4 11/13] qapi/parser: enable mypy checks John Snow
2021-09-30 20:57 ` [PATCH v4 12/13] qapi/parser: Silence too-few-public-methods warning John Snow
2021-09-30 20:57 ` [PATCH v4 13/13] qapi/parser: enable pylint checks John Snow
2021-10-01 14:11 ` [PATCH v4 00/13] qapi: static typing conversion, pt5b Markus Armbruster
2021-10-02  6:20   ` Markus Armbruster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210930205716.1148693-8-jsnow@redhat.com \
    --to=jsnow@redhat.com \
    --cc=armbru@redhat.com \
    --cc=crosa@redhat.com \
    --cc=eblake@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=michael.roth@amd.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).