Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Insert comments to separate sections dealing with parsing, semantic
analysis, code generation, and so forth.

Move helpers to their proper section.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

To have expression semantic analysis in one place rather than two.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Screwed up in commit e53188ad.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

We maintain a stack of filenames in include_hist for convenient cycle
detection.

As error_path() demonstrates, the same information is readily
available in the expr_info, so just use that, and drop include_hist.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

We print the name as it appears in the include expression.  Tools
processing error messages want it relative to the working directory.
Make it so.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

old name      new name
----------------------------
input_file    fname
input_relname fname
input_fname   abs_fname
include_path  incl_abs_fname
parent_info   incl_info
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Mandatory option is silly, and the error handling is missing: the
programs crash when -i isn't supplied.  Make it an argument, and check
it properly.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Report to stderr, prefix with the program name.  Also reject
extra arguments.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Reviewed-by: NEric Blake <eblake@redhat.com>

Continuing the string of cleanups for supporting downstream names
containing '.', this patch focuses on ensuring c_type() can
handle a downstream name.  This patch alone does not fix the
places where generator output should be calling this function
but was open-coding things instead, but it gets us a step closer.

In particular, the changes to c_list_type() and type_name() mean
that type_name(FOO) now handles the case when FOO contains '.',
'-', or is a ticklish identifier other than a builtin (builtins
are exempted because ['int'] must remain mapped to 'intList' and
not 'q_intList').  Meanwhile, ['unix'] now maps to 'q_unixList'
rather than 'unixList', to match the fact that 'unix' is ticklish;
however, our naming conventions state that complex types should
start with a capital, so no type name following conventions will
ever have the 'q_' prepended.

Likewise, changes to c_type() mean that c_type(FOO) properly
handles an enum or complex type FOO with '.' or '-' in the
name, or is a ticklish identifier (again, a ticklish identifier
as a type name violates conventions).
Signed-off-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

c_type() is designed to be called on both string names and on
array designations, so 'name' is a bit misleading because it
operates on more than strings.  Also, no caller ever passes
an empty string.  Finally, + notation is a bit nicer to read
than '%s' % value for string concatenation.
Signed-off-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

Missed in commit b0b58195.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>

Now that the two functions are identical, we only need one of them,
and we might as well give it a more descriptive name.  Basically,
the function serves as the translation from a QAPI name into a
(portion of a) C identifier, without regards to whether it is a
variable or function name.
Signed-off-by: NEric Blake <eblake@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

c_fun() maps '.' to '_', c_var() doesn't.  Nothing prevents '.' in
QAPI names that get passed to c_var().

Which QAPI names get passed to c_fun(), to c_var(), or to both is not
obvious.  Names of command parameters and struct type members get
passed to c_var().

c_var() strips a leading '*', but this cannot happen.  c_fun()
doesn't.

Fix c_var() to work exactly like c_fun().

Perhaps they should be replaced by a single mapping function.
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>
[add 'import string']
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NAlberto Garcia <berto@igalia.com>

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>

The handling of \ inside QAPI strings was less than ideal, and
really only worked JSON's \/, \\, \", and our extension of \'
(an obvious extension, when you realize we use '' instead of ""
for strings).  For other things, like '\n', it resulted in a
literal 'n' instead of a newline.

Of course, at the moment, we really have no use for escaped
characters, as QAPI has to map to C identifiers, and we currently
support ASCII only for that.  But down the road, we may add
support for default values for string parameters to a command
or struct; if that happens, it would be nice to correctly support
all JSON escape sequences, such as \n or \uXXXX.  This gets us
closer, by supporting Unicode escapes in the ASCII range.

Since JSON does not require \OCTAL or \xXX escapes, and our QMP
implementation does not understand them either, I intentionally
reject it here, but it would be an easy addition if we desired it.
Likewise, intentionally refusing the NUL byte means we don't have
to worry about C strings being shorter than the qapi input.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

A future patch will be using a 'name':{dictionary} entry in the
QAPI schema to specify a default value for an optional argument
(see previous commit messages for more details why); but existing
use of inline nested structs conflicts with that goal. Now that
all commands have been changed to avoid inline nested structs,
nuke support for them, and turn it into a hard error. Update the
testsuite to reflect tighter parsing rules.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

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.  Finish up the
conversion to using "struct" in qapi schema by removing the hack
in the generator that allowed 'type'.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

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>

Now that we have a way to validate every type, we can also be
stricter about enforcing that callers that want to bypass
type safety in generated code.  Prior to this patch, it didn't
matter what value was associated with the key 'gen', but it
looked odd that 'gen':'yes' could result in bypassing the
generated code.  These changes also enforce the changes made
earlier in the series for documentation and consolidation of
using '**' as the wildcard type, as well as 'gen':false as the
canonical spelling for requesting type bypass.

Note that 'gen':false is a one-way switch away from the default;
we do not support 'gen':true (similar for 'success-response').
In practice, this doesn't matter.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

...or an array of dictionaries.  Although we have to cater to
existing commands, returning a non-dictionary means the command
is not extensible (no new name/value pairs can be added if more
information must be returned in parallel).  By making the
whitelist explicit, any new command that falls foul of this
practice will have to be self-documenting, which will encourage
developers to either justify the action or rework the design to
use a dictionary after all.

It's a little bit sloppy that we share a single whitelist among
three clients (it's too permissive for each).  If this is a
problem, a future patch could tighten things by having the
generator take the whitelist as an argument (as in
scripts/qapi-commands.py --legacy-returns=...), or by having
the generator output C code that requires explicit use of the
whitelist (as in:
 #ifndef FROBNICATE_LEGACY_RETURN_OK
 # error Command 'frobnicate' should return a dictionary
 #endif
then having the callers define appropriate macros).  But until
we need such fine-grained separation (if ever), this patch does
the job just fine.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Previous commits demonstrated that the generator overlooked various
bad naming situations:
- types, commands, and events need a valid name
- enum members must be valid names, when combined with prefix
- union and alternate branches cannot be marked optional

Valid upstream names match [a-zA-Z][a-zA-Z0-9_-]*; valid downstream
names match __[a-zA-Z][a-zA-Z0-9._-]*.  Enumerations match the
weaker [a-zA-Z0-9._-]+ (in part thanks to QKeyCode picking an enum
that starts with a digit, which we can't change now due to
backwards compatibility).  Rather than call out three separate
regex, this patch just uses a broader combination that allows both
upstream and downstream names, as well as a small hack that
realizes that any enum name is merely a suffix to an already valid
name prefix (that is, any enum name is valid if prepending _ fits
the normal rules).

We could reject new enumeration names beginning with a digit by
whitelisting existing exceptions.  We could also be stricter
about the distinction between upstream names (no leading
underscore, no use of dot) and downstream (mandatory leading
double underscore), but it is probably not worth the bother.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Now that we know every expression is valid with regards to
its keys, we can add further tests that those keys refer to
valid types.  With this patch, all uses of a type (the 'data':
of command, type, union, alternate, and event; the 'returns':
of command; the 'base': of type and union) must resolve to an
appropriate subset of metatypes  declared by the current qapi
parse; this includes recursing into each member of a data
dictionary.  Dealing with '**' and nested anonymous structs
will be done in later patches.

Update the testsuite to match improved output.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

In the near term, we will use it for a sensible-looking
'gen':false inside command declarations, instead of the
current ugly 'gen':'no'.

In the long term, it will allow conversion from shorthand
with defaults mentioned only in side-band documentation:
 'data':{'*flag':'bool', '*string':'str'}
into an explicit default value documentation, as in:
 'data':{'flag':{'type':'bool', 'optional':true, 'default':true},
         'string':{'type':'str', 'optional':true, 'default':null}}

We still don't parse integer values (also necessary before
we can allow explicit defaults), but that can come in a later
series.

Update the testsuite to match an improved error message.
Signed-off-by: NFam Zheng <famz@redhat.com>
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

The previous commit demonstrated that the generator overlooked
duplicate expressions:
- a complex type or command reusing a built-in type name
- redeclaration of a type name, whether by the same or different
metatype
- redeclaration of a command or event
- collision of a type with implicit 'Kind' enum for a union
- collision with an implicit MAX enum constant

Since the c_type() function in the generator treats all names
as being in the same namespace, this patch adds a global array
to track all known names and their source, to prevent collisions
before it can cause further problems.  While valid .json files
won't trigger any of these cases, we might as well be nicer to
developers that make a typo while trying to add new QAPI code.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

The previous commit demonstrated that the generator overlooked some
fairly basic broken expressions:
- missing metataype
- metatype key has a non-string value
- unknown key in relation to the metatype
- conflicting metatype (this patch treats the second metatype as an
unknown key of the first key visited, which is not necessarily the
first key the user typed)

Add check_keys to cover these situations, and update testcases to
match.  A couple other tests (enum-missing-data, indented-expr) had
to change since the validation added here occurs so early.
Conversely, changes to ident-with-escape results show that we still
have problems where our handling of escape sequences differs from
true JSON, which will matter down the road if we allow arbitrary
default string values for optional parameters (but for now is not
too bad, as we currently can avoid unicode escaping as we don't
need to represent anything beyond C identifier material).

While valid .json files won't trigger any of these cases, we might
as well be nicer to developers that make a typo while trying to add
new QAPI code.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

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: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

Special-casing 'discriminator == {}' for handling anonymous unions
is getting awkward; since this particular type is not always a
dictionary on the wire, it is easier to treat it as a completely
different class of type, "alternate", so that if a type is listed
in the union_types array, we know it is not an anonymous union.

This patch just further segregates union handling, to make sure that
anonymous unions are not stored in union_types, and splitting up
check_union() into separate functions.  A future patch will change
the qapi grammar, and having the segregation already in place will
make it easier to deal with the distinct meta-type.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

This patch widens the scope of a try block (with the attending
reindentation required by Python) in preparation for a future
patch adding more instances of QAPIExprError inside the block.
It's easier to separate indentation from semantic changes, so
this patch has no real behavior change.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

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: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

None of the existing QMP or QGA interfaces uses a union with a
base type but no discriminator; it is easier to avoid this in the
generator to save room for other future extensions more likely to
be useful.  An earlier commit added a union-base-no-discriminator
test to ensure that we eventually give a decent error message;
likewise, removing UserDefUnion outright is okay, because we moved
all the tests we wish to keep into the tests of the simple union
UserDefNativeListUnion in the previous commit.  Now is the time to
actually forbid simple union with base, and remove the last
vestiges from the testsuite.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>

The previous commit demonstrated that the generator had several
flaws with less-than-perfect enums:
- an enum that listed the same string twice (or two variant
strings that map to the same C enumerator) ended up generating
an invalid C enum
- because the generator adds a _MAX terminator to each enum,
the use of an enum member 'max' can also cause this clash
- if an enum omits 'data', the generator left a python stack
trace rather than a graceful message
- an enum that used a non-array 'data' was silently accepted by
the parser
- an enum that used non-string members in the 'data' member
was silently accepted by the parser

Add check_enum to cover these situations, and update testcases
to match.  While valid .json files won't trigger any of these
cases, we might as well be nicer to developers that make a typo
while trying to add new QAPI code.
Signed-off-by: NEric Blake <eblake@redhat.com>
Reviewed-by: NMarkus Armbruster <armbru@redhat.com>
Signed-off-by: NMarkus Armbruster <armbru@redhat.com>