1. 13 10月, 2015 1 次提交
    • E
      qapi: Test for various name collisions · d220fbcd
      Eric Blake 提交于
      Expose some weaknesses in the generator: we don't always forbid
      the generation of structs that contain multiple members that map
      to the same C or QMP name.  This has already been marked FIXME in
      qapi.py in commit d90675fa, but having more tests will make sure
      future patches produce desired behavior; and updating existing
      patches to better document things doesn't hurt, either.  Some of
      these collisions are already caught in the old-style parser
      checks, but ultimately we want all collisions to be caught in the
      new-style QAPISchema*.check() methods.
      
      This patch focuses on C struct members, and does not consider
      collisions between commands and events (affecting C function
      names), or even collisions between generated C type names with
      user type names (for things like automatic FOOList struct
      representing array types or FOOKind for an implicit enum).
      
      There are two types of struct collisions we want to catch:
       1) Collision between two keys in a JSON object. qapi.py prevents
          that within a single struct (see test duplicate-key), but it is
          possible to have collisions between a type's members and its
          base type's members (existing tests struct-base-clash,
          struct-base-clash-deep), and its flat union variant members
          (renamed test flat-union-clash-member).
       2) Collision between two members of the C struct that is generated
          for a given QAPI type:
          a) Multiple QAPI names map to the same C name (new test
             args-name-clash)
          b) A QAPI name maps to a C name that is used for another purpose
             (new tests flat-union-clash-branch, struct-base-clash-base,
             union-clash-data). We already fixed some such cases in commit
             0f61af3e and 1e6c1616, but more remain.
          c) Two C names generated for other purposes clash
             (updated test alternate-clash, new test union-clash-branches,
             union-clash-type, flat-union-clash-type)
      
      Ultimately, if we need to have a flat union where a tag value
      clashes with a base member name, we could change the generator to
      name the union (using 'foo.u.value' rather than 'foo.value') or
      otherwise munge the C name corresponding to tag values.  But
      unless such a need arises, it will probably be easier to just
      forbid these collisions.
      
      Some of these negative tests will be deleted later, and positive
      tests added to qapi-schema-test.json in their place, when the
      generator code is reworked to avoid particular code generation
      collisions in class 2).
      
      [Note that viewing this patch with git rename detection enabled
      may see some confusion due to renaming some tests while adding
      others, but where the content is similar enough that git picks
      the wrong pre- and post-patch files to associate]
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Message-Id: <1443565276-4535-6-git-send-email-eblake@redhat.com>
      [Improve commit message and comments a bit, drop an unrelated test]
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      d220fbcd
  2. 06 5月, 2015 3 次提交
    • E
      qapi: Check for member name conflicts with a base class · ff55d72e
      Eric Blake 提交于
      Our type inheritance for both 'struct' and for flat 'union' merges
      key/value pairs from the base class with those from the type in
      question.  Although the C code currently boxes things so that there
      is a distinction between which member is referred to, the QMP wire
      format does not allow passing a key more than once in a single
      object.  Besides, if we ever change the generated C code to not be
      quite so boxy, we'd want to avoid duplicate member names there,
      too.
      
      Fix a testsuite entry added in an earlier patch, as well as adding
      a couple more tests to ensure we have appropriate coverage.  Ensure
      that collisions are detected, regardless of whether there is a
      difference in opinion on whether the member name is optional.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      ff55d72e
    • E
      qapi: Prefer 'struct' over 'type' in generator · fd41dd4e
      Eric Blake 提交于
      Referring to "type" as both a meta-type (built-in, enum, union,
      alternate, or struct) and a specific type (the name that the
      schema uses for declaring structs) is confusing.  The confusion
      is only made worse by the fact that the generator mostly already
      refers to struct even when dealing with expr['type'].  This
      commit changes the generator to consistently refer to it as
      struct everywhere, plus a single back-compat tweak that allows
      accepting the existing .json files as-is, so that the meat of
      this change is separate from the mindless churn of that change.
      
      Fix the testsuite fallout for error messages that change, and
      in some cases, become more legible.  Improve comments to better
      match our intentions where a struct (rather than any complex
      type) is required.  Note that in some cases, an error message
      now refers to 'struct' while the schema still refers to 'type';
      that will be cleaned up in the later commit to the schema.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      fd41dd4e
    • E
      qapi: Add some union tests · 3d0c4829
      Eric Blake 提交于
      Demonstrate that the qapi generator doesn't deal well with unions
      that aren't up to par. Later patches will update the expected
      reseults as the generator is made stricter.  A few tests work
      as planned, but most show poor or missing error messages.
      
      Of particular note, qapi-code-gen.txt documents 'base' only for
      flat unions, but the tests here demonstrate that we currently allow
      a 'base' to a simple union, although it is exercised only in the
      testsuite.  Later patches will remove this undocumented feature, to
      give us more flexibility in adding other future extensions to union
      types.  For example, one possible extension is the idea of a
      type-safe simple enum, where added fields tie the discriminator to
      a user-defined enum type rather than creating an implicit enum from
      the names in 'data'.  But adding such safety on top of a simple
      enum with a base type could look ambiguous with a flat enum;
      besides, the documentation also mentions how any simple union can
      be represented by an equivalent flat union.  So it will be simpler
      to just outlaw support for something we aren't using.
      Signed-off-by: NEric Blake <eblake@redhat.com>
      Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
      Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
      3d0c4829