Commit Graph

9 Commits

Author SHA1 Message Date
Markus Armbruster
fa110c6a9e qapi: Move context-sensitive checking to the proper place
When we introduced the QAPISchema intermediate representation (commit
ac88219a6c), we took a shortcut: we left check_exprs() & friends
alone instead of moving semantic checks into the
QAPISchemaFOO.check().  The .check() assert check_exprs() did its job.

Time to finish the conversion job.  Move exactly the context-sensitive
checks to the .check().  They replace assertions there.  Context-free
checks stay put.

Fixes the misleading optional tag error demonstrated by test
flat-union-optional-discriminator.

A few other error message improve.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-17-armbru@redhat.com>
2019-09-28 17:17:19 +02:00
Markus Armbruster
2ab218aad6 qapi: Change frontend error messages to start with lower case
Starting error messages with a capital letter complicates things when
text can get interpolated both at the beginning and in the middle of
an error message.  The next patch will do that.  Switch to lower case
to keep it simpler.

For what it's worth, the GNU Coding Standards advise the message
"should not begin with a capital letter when it follows a program name
and/or file name, because that isn’t the beginning of a sentence. (The
sentence conceptually starts at the beginning of the line.)"

While there, avoid breaking lines containing multiple arguments in the
middle of an argument.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-7-armbru@redhat.com>
2019-09-28 17:17:18 +02:00
Markus Armbruster
7be6c51194 qapi: Prefix frontend errors with an "in definition" line
We take pains to include the offending expression in error messages,
e.g.

    tests/qapi-schema/alternate-any.json:2: alternate 'Alt' member 'one' cannot use type 'any'

But not always:

    tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings

Instead of improving them one by one, report the offending expression
whenever it is known, like this:

    tests/qapi-schema/enum-if-invalid.json: In enum 'TestIfEnum':
    tests/qapi-schema/enum-if-invalid.json:2: 'if' condition must be a string or a list of strings

Error messages that mention the offending expression become a bit
redundant, e.g.

    tests/qapi-schema/alternate-any.json: In alternate 'Alt':
    tests/qapi-schema/alternate-any.json:2: alternate 'Alt' member 'one' cannot use type 'any'

I'll take care of that later in this series.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20190927134639.4284-5-armbru@redhat.com>
2019-09-28 17:17:18 +02:00
Markus Armbruster
c0644771eb qapi: Reject alternates that can't work with keyval_parse()
Alternates are sum types like unions, but use the JSON type on the
wire / QType in QObject instead of an explicit tag.  That's why we
require alternate members to have distinct QTypes.

The recently introduced keyval_parse() (commit d454dbe) can only
produce string scalars.  The qobject_input_visitor_new_keyval() input
visitor mostly hides the difference, so code using a QObject input
visitor doesn't have to care whether its input was parsed from JSON or
KEY=VALUE,...  The difference leaks for alternates, as noted in commit
0ee9ae7: a non-string, non-enum scalar alternate value can't currently
be expressed.

In part, this is just our insufficiently sophisticated implementation.
Consider alternate type 'GuestFileWhence'.  It has an integer member
and a 'QGASeek' member.  The latter is an enumeration with values
'set', 'cur', 'end'.  The meaning of b=set, b=cur, b=end, b=0, b=1 and
so forth is perfectly obvious.  However, our current implementation
falls apart at run time for b=0, b=1, and so forth.  Fixable, but not
today; add a test case and a TODO comment.

Now consider an alternate type with a string and an integer member.
What's the meaning of a=42?  Is it the string "42" or the integer 42?
Whichever meaning you pick makes the other inexpressible.  This isn't
just an implementation problem, it's fundamental.  Our current
implementation will pick string.

So far, we haven't needed such alternates.  To make sure we stop and
think before we add one that cannot sanely work with keyval_parse(),
let's require alternate members to have sufficiently distinct
representation in KEY=VALUE,... syntax:

* A string member clashes with any other scalar member

* An enumeration member clashes with bool members when it has value
  'on' or 'off'.

* An enumeration member clashes with numeric members when it has a
  value that starts with '-', '+', or a decimal digit.  This is a
  rather lazy approximation of the actual number syntax accepted by
  the visitor.

  Note that enumeration values starting with '-' and '+' are rejected
  elsewhere already, but better safe than sorry.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1495471335-23707-5-git-send-email-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
2017-05-31 16:04:09 +02:00
Markus Armbruster
87c16dceca qapi: Back out doc comments added just to please qapi.py
This reverts commit 3313b61's changes to tests/qapi-schema/, except
for tests/qapi-schema/doc-*.

We could keep some of these doc comments to serve as positive test
cases.  However, they don't actually add to what we get from doc
comment use in actual schemas, as we we don't test output matches
expectations, and don't systematically cover doc comment features.
Proper positive test coverage would be nice.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <1489582656-31133-4-git-send-email-armbru@redhat.com>
2017-03-16 07:13:01 +01:00
Marc-André Lureau
3313b6124b qapi: add qapi2texi script
As the name suggests, the qapi2texi script converts JSON QAPI
description into a texi file suitable for different target
formats (info/man/txt/pdf/html...).

It parses the following kind of blocks:

Free-form:

  ##
  # = Section
  # == Subsection
  #
  # Some text foo with *emphasis*
  # 1. with a list
  # 2. like that
  #
  # And some code:
  # | $ echo foo
  # | -> do this
  # | <- get that
  #
  ##

Symbol description:

  ##
  # @symbol:
  #
  # Symbol body ditto ergo sum. Foo bar
  # baz ding.
  #
  # @param1: the frob to frobnicate
  # @param2: #optional how hard to frobnicate
  #
  # Returns: the frobnicated frob.
  #          If frob isn't frobnicatable, GenericError.
  #
  # Since: version
  # Notes: notes, comments can have
  #        - itemized list
  #        - like this
  #
  # Example:
  #
  # -> { "execute": "quit" }
  # <- { "return": {} }
  #
  ##

That's roughly following the following EBNF grammar:

api_comment = "##\n" comment "##\n"
comment = freeform_comment | symbol_comment
freeform_comment = { "# " text "\n" | "#\n" }
symbol_comment = "# @" name ":\n" { member | tag_section | freeform_comment }
member = "# @" name ':' [ text ] "\n" freeform_comment
tag_section = "# " ( "Returns:", "Since:", "Note:", "Notes:", "Example:", "Examples:" ) [ text ]  "\n" freeform_comment
text = free text with markup

Note that the grammar is ambiguous: a line "# @foo:\n" can be parsed
both as freeform_comment and as symbol_comment.  The actual parser
recognizes symbol_comment.

See docs/qapi-code-gen.txt for more details.

Deficiencies and limitations:
- the generated QMP documentation includes internal types
- union type support is lacking
- type information is lacking in generated documentation
- doc comment error message positions are imprecise, they point
  to the beginning of the comment.
- a few minor issues, all marked TODO/FIXME in the code

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20170113144135.5150-16-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[test-qapi.py tweaked to avoid trailing empty lines in .out]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2017-01-16 10:10:35 +01:00
Eric Blake
ab916faddd qapi: Use 'alternate' to replace anonymous union
Previous patches have led up to the point where I create the
new meta-type "'alternate':'Foo'".  See the previous patches
for documentation; I intentionally split as much work into
earlier patches to minimize the size of this patch, but a lot
of it is churn due to testsuite fallout after updating to the
new type.

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-05-05 18:39:00 +02:00
Eric Blake
44bd1276a7 qapi: Tighten checking of unions
Previous commits demonstrated that the generator had several
flaws with less-than-perfect unions:
- a simple union that listed the same branch twice (or two variant
names that map to the same C enumerator, including the implicit
MAX sentinel) ended up generating invalid C code
- an anonymous union that listed two branches with the same qtype
ended up generating invalid C code
- the generator crashed on anonymous union attempts to use an
array type
- the generator was silently ignoring a base type for anonymous
unions
- the generator allowed unknown types or nested anonymous unions
as a branch in an anonymous union

Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-05-05 18:39:00 +02:00
Eric Blake
3d0c482926 qapi: Add some union tests
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: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
2015-05-05 18:39:00 +02:00