Skip to content
Snippets Groups Projects
  1. May 20, 2021
    • John Snow's avatar
      qapi/parser: add docstrings · d4092ffa
      John Snow authored
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-16-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      [Doc string spacing tweaked slightly]
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      d4092ffa
    • John Snow's avatar
      qapi/parser: allow 'ch' variable name · 9b91e76b
      John Snow authored
      
      We can have a two-letter variable name, as a treat.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-15-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      9b91e76b
    • John Snow's avatar
      qapi/parser: Remove superfluous list comprehension · 013a3ace
      John Snow authored
      
      A generator suffices (and quiets a pylint warning).
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-14-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      013a3ace
    • John Snow's avatar
      qapi/parser: add type hint annotations · 810aff8f
      John Snow authored
      
      Annotations do not change runtime behavior.
      This commit *only* adds annotations.
      
      (Annotations for QAPIDoc are in a forthcoming commit.)
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-13-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      810aff8f
    • John Snow's avatar
      qapi/parser: Rework _check_pragma_list_of_str as a TypeGuard · 03386200
      John Snow authored
      
      TypeGuards wont exist in Python proper until 3.10. Ah well. We can hack
      up our own by declaring this function to return the type we claim it
      checks for and using this to safely downcast object -> List[str].
      
      In so doing, I bring this function under _pragma so it can use the
      'info' object in its closure. Having done this, _pragma also now no
      longer needs to take a 'self' parameter, so drop it.
      
      To help with line-length, and with the context evident from its new
      scope, rename the function to the shorter check_list_str().
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      
      Message-Id: <20210519183951.3946870-12-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      03386200
    • John Snow's avatar
      qapi/parser: Fix token membership tests when token can be None · c256263f
      John Snow authored
      
      When the token can be None (EOF), we can't use 'x in "abc"' style
      membership tests to group types of tokens together, because 'None in
      "abc"' is a TypeError.
      
      Easy enough to fix. (Use a tuple: It's neither a static typing error nor
      a runtime error to check for None in Tuple[str, ...])
      
      Add tests to prevent a regression. (Note: they cannot be added prior to
      this fix, as the unhandled stack trace will not match test output in the
      CI system.)
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-11-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      c256263f
    • John Snow's avatar
      qapi: add must_match helper · e0e8a0ac
      John Snow authored
      
      Mypy cannot generally understand that these regex functions cannot
      possibly fail. Add a "must_match" helper that makes this clear for
      mypy.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-10-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      e0e8a0ac
    • John Snow's avatar
      qapi/parser: Use @staticmethod where appropriate · 43b1be65
      John Snow authored
      
      No self, no thank you!
      
      (Quiets pylint warnings.)
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-9-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      43b1be65
    • John Snow's avatar
      qapi/parser: assert object keys are strings · 234dce2c
      John Snow authored
      
      The single quote token implies the value is a string. Assert this to be
      the case, to allow us to write an accurate return type for get_members.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-8-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      234dce2c
    • John Snow's avatar
      qapi/parser: enforce all top-level expressions must be dict in _parse() · 9cd0205d
      John Snow authored
      
      Instead of using get_expr nested=False, allow get_expr to always return
      any expression. In exchange, add a new error message to the top-level
      parser that explains the semantic error: Top-level expressions must
      always be JSON objects.
      
      This helps mypy understand the rest of this function which assumes that
      get_expr did indeed return a dict.
      
      The exception type changes from QAPIParseError to QAPISemError as a
      result, and the error message in two tests now changes.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      
      Message-Id: <20210519183951.3946870-7-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      9cd0205d
    • John Snow's avatar
      qapi/parser: Assert lexer value is a string · 7c610ce6
      John Snow authored
      
      The type checker can't narrow the type of the token value to string,
      because it's only loosely correlated with the return token.
      
      We know that a token of '#' should always have a "str" value.
      Add an assertion.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-6-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      7c610ce6
    • John Snow's avatar
      qapi/parser: factor parsing routine into method · 16ff40ac
      John Snow authored
      
      For the sake of keeping __init__ smaller (and treating it more like a
      gallery of what state variables we can expect to see), put the actual
      parsing action into a parse method. It remains invoked from the init
      method to reduce churn.
      
      To accomplish this, @previously_included becomes the private data
      member ._included, and the filename is stashed as ._fname.
      
      Add any missing declarations to the init method, and group them by
      function so they can be understood quickly at a glance.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-5-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      16ff40ac
    • John Snow's avatar
      qapi/source: Remove line number from QAPISourceInfo initializer · b2b31fdf
      John Snow authored
      
      With the QAPISourceInfo(None, None, None) construct gone, there's no
      longer any reason to have to specify that a file starts on the first
      line. Remove it from the initializer and default it to 1.
      
      Remove the last vestiges where we check for 'line' being unset, that
      can't happen, now.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-4-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      b2b31fdf
    • John Snow's avatar
      qapi/parser: Don't try to handle file errors · 3404e574
      John Snow authored
      
      Fixes: f5d4361c
      Fixes: 52a47418
      Fixes: 46f49468
      
      Remove the try/except block that handles file-opening errors in
      QAPISchemaParser.__init__() and add one each to
      QAPISchemaParser._include() and QAPISchema.__init__() respectively.
      
      This simultaneously fixes the typing of info.fname (f5d4361c), A
      static typing violation in test-qapi (46f49468), and a regression of
      an error message (52a47418).
      
      The short-ish version of what motivates this patch is:
      
      - It's hard to write a good error message in the init method,
        because we need to determine the context of our caller to do so.
        It's easier to just let the caller write the message.
      - We don't want to allow QAPISourceInfo(None, None, None) to exist. The
        typing introduced by commit f5d4361c types the 'fname' field as
        (non-optional) str, which was premature until the removal of this
        construct.
      - Errors made using such an object are currently incorrect (since
        52a47418)
      - It's not technically a semantic error if we cannot open the schema.
      - There are various typing constraints that make mixing these two cases
        undesirable for a single special case.
      - test-qapi's code handling an fname of 'None' is now dead, drop it.
        Additionally, Not all QAPIError objects have an 'info' field (since
        46f49468), so deleting this stanza corrects a typing oversight in
        test-qapi introduced by that commit.
      
      Other considerations:
      
      - open() is moved to a 'with' block to ensure file pointers are
        cleaned up deterministically.
      - Python 3.3 deprecated IOError and made it a synonym for OSError.
        Avoid the misleading perception these exception handlers are
        narrower than they really are.
      
      The long version:
      
      The error message here is incorrect (since commit 52a47418):
      
      > python3 qapi-gen.py 'fake.json'
      qapi-gen.py: qapi-gen.py: can't read schema file 'fake.json': No such file or directory
      
      In pursuing it, we find that QAPISourceInfo has a special accommodation
      for when there's no filename. Meanwhile, the intent when QAPISourceInfo
      was typed (f5d4361c) was non-optional 'str'. This usage was
      overlooked.
      
      To remove this, I'd want to avoid having a "fake" QAPISourceInfo
      object. I also don't want to explicitly begin accommodating
      QAPISourceInfo itself being None, because we actually want to eventually
      prove that this can never happen -- We don't want to confuse "The file
      isn't open yet" with "This error stems from a definition that wasn't
      defined in any file".
      
      (An earlier series tried to create a dummy info object, but it was tough
      to prove in review that it worked correctly without creating new
      regressions. This patch avoids that distraction. We would like to first
      prove that we never raise QAPISemError for any built-in object before we
      add "special" info objects. We aren't ready to do that yet.)
      
      So, which way out of the labyrinth?
      
      Here's one way: Don't try to handle errors at a level with "mixed"
      semantic contexts; i.e. don't mix inclusion errors (should report a
      source line where the include was triggered) and command line errors
      (where we specified a file we couldn't read).
      
      Remove the error handling from the initializer of the parser. Pythonic!
      Now it's the caller's job to figure out what to do about it. Handle the
      error in QAPISchemaParser._include() instead, where we can write a
      targeted error message where we are guaranteed to have an 'info' context
      to report with.
      
      The root level error can similarly move to QAPISchema.__init__(), where
      we know we'll never have an info context to report with, so we use a
      more abstract error type.
      
      Now the error looks sensible again:
      
      > python3 qapi-gen.py 'fake.json'
      qapi-gen.py: can't read schema file 'fake.json': No such file or directory
      
      With these error cases separated, QAPISourceInfo can be solidified as
      never having placeholder arguments that violate our desired types. Clean
      up test-qapi along similar lines.
      
      Signed-off-by: default avatarJohn Snow <jsnow@redhat.com>
      Message-Id: <20210519183951.3946870-2-jsnow@redhat.com>
      Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: default avatarMarkus Armbruster <armbru@redhat.com>
      3404e574
  2. May 14, 2021
  3. May 12, 2021
  4. May 04, 2021
  5. May 03, 2021
  6. May 01, 2021
  7. Apr 30, 2021
Loading