| Message ID | 1445576998-2921-6-git-send-email-eblake@redhat.com |
|---|---|
| State | New |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > c_name() produces names starting with 'q_' when protecting > a QMP member name that would fail to directly compile, but > in doing so can cause clashes with any QMP name already > beginning with 'q-' or 'q_'. Likewise, we create a C name > 'has_' for any optional member, that can clash with any QMP > name beginning with 'has-' or 'has_'. > > Technically, rather than blindly reserving the namespace, > we could try to complain about user names only when an actual > collision occurs, or even teach c_name() how to munge names > to avoid collisions. But it is not trivial, especially when > collisions can occur across multiple types (such as via > inheritance or flat unions). Besides, no existing .json > files are trying to use these names. So it's easier to just > outright forbid the potential for collision. We can always > relax things in the future if a real need arises for QMP to > express member names that have been forbidden here. > > 'has_' only has to be reserved for struct/union member names, > while 'q_' is reserved everywhere (matching the fact that > we only have optional members, but use c_name() everywhere). > > Update and add tests to cover the new error messages. > > Signed-off-by: Eric Blake <eblake@redhat.com> > > --- > v10: retitle, improve comments, defer 'u' changes for later, use > c_name(name).startswith('has_') rather than open coding > v9: new patch > --- > docs/qapi-code-gen.txt | 11 +++++++---- > scripts/qapi.py | 8 +++++++- > tests/qapi-schema/args-has-clash.err | 1 + > tests/qapi-schema/args-has-clash.exit | 2 +- > tests/qapi-schema/args-has-clash.json | 8 ++++---- > tests/qapi-schema/args-has-clash.out | 6 ------ > tests/qapi-schema/reserved-command.err | 1 + > tests/qapi-schema/reserved-command.exit | 2 +- > tests/qapi-schema/reserved-command.json | 7 +++---- > tests/qapi-schema/reserved-command.out | 5 ----- > tests/qapi-schema/reserved-member.err | 1 + > tests/qapi-schema/reserved-member.exit | 2 +- > tests/qapi-schema/reserved-member.json | 7 +++---- > tests/qapi-schema/reserved-member.out | 4 ---- > 14 files changed, 30 insertions(+), 35 deletions(-) All right, this is simple enough to be done now. And as you say, we can always relax things again later. > diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt > index c4264a8..4d8c2fc 100644 > --- a/docs/qapi-code-gen.txt > +++ b/docs/qapi-code-gen.txt > @@ -112,7 +112,9 @@ and field names within a type, should be all lower case with words > separated by a hyphen. However, some existing older commands and > complex types use underscore; when extending such expressions, > consistency is preferred over blindly avoiding underscore. Event > -names should be ALL_CAPS with words separated by underscore. > +names should be ALL_CAPS with words separated by underscore. Field > +names cannot start with 'has-' or 'has_', as this is reserved for > +tracking optional fields. > > Any name (command, event, type, field, or enum value) beginning with > "x-" is marked experimental, and may be withdrawn or changed > @@ -123,9 +125,10 @@ vendor), even if the rest of the name uses dash (example: > __com.redhat_drive-mirror). Other than downstream extensions (with > leading underscore and the use of dots), all names should begin with a > letter, and contain only ASCII letters, digits, dash, and underscore. > -It is okay to reuse names that match C keywords; the generator will > -rename a field named "default" in the QAPI to "q_default" in the > -generated C code. > +Names beginning with 'q_' are reserved for the generator: QMP names > +that resemble C keywords or other problematic strings will be munged > +in C to use this prefix. For example, a field named "default" in > +qapi becomes "q_default" in the generated C code. > > In the rest of this document, usage lines are given for each > expression type, with literal strings written in lower case and > diff --git a/scripts/qapi.py b/scripts/qapi.py > index d53b5c4..04bcbf7 100644 > --- a/scripts/qapi.py > +++ b/scripts/qapi.py > @@ -376,7 +376,9 @@ def check_name(expr_info, source, name, allow_optional=False, > # code always prefixes it with the enum name > if enum_member: > membername = '_' + membername > - if not valid_name.match(membername): > + # Reserve the entire 'q_' namespace for c_name() > + if not valid_name.match(membername) or \ > + membername.replace('-', '_').startswith('q_'): Why not c_name(membername).startswith('q_')? > raise QAPIExprError(expr_info, > "%s uses invalid name '%s'" % (source, name)) > > @@ -488,6 +490,10 @@ def check_type(expr_info, source, value, allow_array=False, > for (key, arg) in value.items(): > check_name(expr_info, "Member of %s" % source, key, > allow_optional=allow_optional) > + if c_name(key).startswith('has_'): > + raise QAPIExprError(expr_info, > + "Member of %s uses reserved name '%s'" > + % (source, key)) > # Todo: allow dictionaries to represent default values of > # an optional argument. > check_type(expr_info, "Member '%s' of %s" % (key, source), arg, > diff --git a/tests/qapi-schema/args-has-clash.err b/tests/qapi-schema/args-has-clash.err > index e69de29..595fe93 100644 > --- a/tests/qapi-schema/args-has-clash.err > +++ b/tests/qapi-schema/args-has-clash.err > @@ -0,0 +1 @@ > +tests/qapi-schema/args-has-clash.json:6: Member of 'data' for command 'oops' uses reserved name 'has-a' > diff --git a/tests/qapi-schema/args-has-clash.exit b/tests/qapi-schema/args-has-clash.exit > index 573541a..d00491f 100644 > --- a/tests/qapi-schema/args-has-clash.exit > +++ b/tests/qapi-schema/args-has-clash.exit > @@ -1 +1 @@ > -0 > +1 > diff --git a/tests/qapi-schema/args-has-clash.json b/tests/qapi-schema/args-has-clash.json > index a2197de..38453e7 100644 > --- a/tests/qapi-schema/args-has-clash.json > +++ b/tests/qapi-schema/args-has-clash.json > @@ -1,6 +1,6 @@ > # C member name collision > -# FIXME - This parses, but fails to compile, because the C struct is given > -# two 'has_a' members, one from the flag for optional 'a', and the other > -# from member 'has-a'. Either reject this at parse time, or munge the C > -# names to avoid the collision. > +# This would attempt to create two 'has_a' members of the C struct, one > +# from the flag for optional 'a', and the other from member 'has-a'; so > +# instead we reject any member with a name that would collide with 'has_'. Suggest something like: # We reject names like 'has-a', because they can collide with the flag # for an optional 'a' in generated C. > +# TODO we could munge the optional flag name to avoid the collision. > { 'command': 'oops', 'data': { '*a': 'str', 'has-a': 'str' } } > diff --git a/tests/qapi-schema/args-has-clash.out b/tests/qapi-schema/args-has-clash.out > index 5a18b6b..e69de29 100644 > --- a/tests/qapi-schema/args-has-clash.out > +++ b/tests/qapi-schema/args-has-clash.out > @@ -1,6 +0,0 @@ > -object :empty > -object :obj-oops-arg > - member a: str optional=True > - member has-a: str optional=False > -command oops :obj-oops-arg -> None > - gen=True success_response=True > diff --git a/tests/qapi-schema/reserved-command.err b/tests/qapi-schema/reserved-command.err > index e69de29..a70856b 100644 > --- a/tests/qapi-schema/reserved-command.err > +++ b/tests/qapi-schema/reserved-command.err > @@ -0,0 +1 @@ > +tests/qapi-schema/reserved-command.json:6: 'command' uses invalid name 'q-unix' > diff --git a/tests/qapi-schema/reserved-command.exit b/tests/qapi-schema/reserved-command.exit > index 573541a..d00491f 100644 > --- a/tests/qapi-schema/reserved-command.exit > +++ b/tests/qapi-schema/reserved-command.exit > @@ -1 +1 @@ > -0 > +1 > diff --git a/tests/qapi-schema/reserved-command.json b/tests/qapi-schema/reserved-command.json > index be9944c..d662a96 100644 > --- a/tests/qapi-schema/reserved-command.json > +++ b/tests/qapi-schema/reserved-command.json > @@ -1,7 +1,6 @@ > # C entity name collision > -# FIXME - This parses, but fails to compile, because it attempts to declare > -# two 'qmp_q_unix' functions (one for 'q-unix', the other because c_name() > -# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should > -# reject attempts to explicitly use 'q_' names, to reserve it for qapi. > +# This would attempt to declare two 'qmp_q_unix' functions (one for 'q-unix', > +# the other because c_name() munges 'unix' to 'q_unix' to avoid reserved word > +# collisions). Therefore, we reserve 'q_' names for qapi. Suggest something like: # We reject names like 'q-unix', because they can collide with the # mangled name for 'unix' in generated C. > { 'command': 'unix' } > { 'command': 'q-unix' } > diff --git a/tests/qapi-schema/reserved-command.out b/tests/qapi-schema/reserved-command.out > index b31b38f..e69de29 100644 > --- a/tests/qapi-schema/reserved-command.out > +++ b/tests/qapi-schema/reserved-command.out > @@ -1,5 +0,0 @@ > -object :empty > -command q-unix None -> None > - gen=True success_response=True > -command unix None -> None > - gen=True success_response=True > diff --git a/tests/qapi-schema/reserved-member.err b/tests/qapi-schema/reserved-member.err > index e69de29..8752c5b 100644 > --- a/tests/qapi-schema/reserved-member.err > +++ b/tests/qapi-schema/reserved-member.err > @@ -0,0 +1 @@ > +tests/qapi-schema/reserved-member.json:5: Member of 'data' for struct 'Foo' uses invalid name 'q-unix' > diff --git a/tests/qapi-schema/reserved-member.exit b/tests/qapi-schema/reserved-member.exit > index 573541a..d00491f 100644 > --- a/tests/qapi-schema/reserved-member.exit > +++ b/tests/qapi-schema/reserved-member.exit > @@ -1 +1 @@ > -0 > +1 > diff --git a/tests/qapi-schema/reserved-member.json b/tests/qapi-schema/reserved-member.json > index 1602ed3..a8dac22 100644 > --- a/tests/qapi-schema/reserved-member.json > +++ b/tests/qapi-schema/reserved-member.json > @@ -1,6 +1,5 @@ > # C member name collision > -# FIXME - This parses, but fails to compile, because it attempts to declare > -# two 'q_unix' members (one for 'q-unix', the other because c_name() > -# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should > -# reject attempts to explicitly use 'q_' names, to reserve it for qapi. > +# This would attempt to declare two 'q_unix' members (one for 'q-unix', > +# the other because c_name() munges 'unix' to 'q_unix' to avoid reserved word > +# collisions). Therefore, we reserve 'q_' names for qapi. > { 'struct': 'Foo', 'data': { 'unix':'int', 'q-unix':'bool' } } Likewise. Remind me, why do we need both reserved-command.json and reserved-member.json? > diff --git a/tests/qapi-schema/reserved-member.out b/tests/qapi-schema/reserved-member.out > index 0d8685a..e69de29 100644 > --- a/tests/qapi-schema/reserved-member.out > +++ b/tests/qapi-schema/reserved-member.out > @@ -1,4 +0,0 @@ > -object :empty > -object Foo > - member unix: int optional=False > - member q-unix: bool optional=False
On 10/23/2015 07:05 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> c_name() produces names starting with 'q_' when protecting >> a QMP member name that would fail to directly compile, but >> in doing so can cause clashes with any QMP name already >> beginning with 'q-' or 'q_'. Likewise, we create a C name >> 'has_' for any optional member, that can clash with any QMP >> name beginning with 'has-' or 'has_'. >> >> Technically, rather than blindly reserving the namespace, >> we could try to complain about user names only when an actual >> collision occurs, or even teach c_name() how to munge names >> to avoid collisions. But it is not trivial, especially when >> collisions can occur across multiple types (such as via >> inheritance or flat unions). Besides, no existing .json >> files are trying to use these names. So it's easier to just >> outright forbid the potential for collision. We can always >> relax things in the future if a real need arises for QMP to >> express member names that have been forbidden here. >> >> 'has_' only has to be reserved for struct/union member names, >> while 'q_' is reserved everywhere (matching the fact that >> we only have optional members, but use c_name() everywhere). >> >> Update and add tests to cover the new error messages. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> >> +++ b/scripts/qapi.py >> @@ -376,7 +376,9 @@ def check_name(expr_info, source, name, allow_optional=False, >> # code always prefixes it with the enum name >> if enum_member: >> membername = '_' + membername >> - if not valid_name.match(membername): >> + # Reserve the entire 'q_' namespace for c_name() >> + if not valid_name.match(membername) or \ >> + membername.replace('-', '_').startswith('q_'): > > Why not c_name(membername).startswith('q_')? Recursion. c_name('unix') is 'q_unix', which would be rejected, even though we want to accept it. (And yes, that was my first try, before the approach you see here) >> +++ b/tests/qapi-schema/args-has-clash.json >> @@ -1,6 +1,6 @@ >> # C member name collision >> -# FIXME - This parses, but fails to compile, because the C struct is given >> -# two 'has_a' members, one from the flag for optional 'a', and the other >> -# from member 'has-a'. Either reject this at parse time, or munge the C >> -# names to avoid the collision. >> +# This would attempt to create two 'has_a' members of the C struct, one >> +# from the flag for optional 'a', and the other from member 'has-a'; so >> +# instead we reject any member with a name that would collide with 'has_'. > > Suggest something like: > > # We reject names like 'has-a', because they can collide with the flag > # for an optional 'a' in generated C. Sure, your wording changes are fine. > > Remind me, why do we need both reserved-command.json and > reserved-member.json? Because they are different namespaces. Your comment in 1/25 pointed out that we could change the command munging to allow qmp_q_unix() independently from qmp_unix(), even when we can't do the same for members.
Eric Blake <eblake@redhat.com> writes: > On 10/23/2015 07:05 AM, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >>> c_name() produces names starting with 'q_' when protecting >>> a QMP member name that would fail to directly compile, but >>> in doing so can cause clashes with any QMP name already >>> beginning with 'q-' or 'q_'. Likewise, we create a C name >>> 'has_' for any optional member, that can clash with any QMP >>> name beginning with 'has-' or 'has_'. >>> >>> Technically, rather than blindly reserving the namespace, >>> we could try to complain about user names only when an actual >>> collision occurs, or even teach c_name() how to munge names >>> to avoid collisions. But it is not trivial, especially when >>> collisions can occur across multiple types (such as via >>> inheritance or flat unions). Besides, no existing .json >>> files are trying to use these names. So it's easier to just >>> outright forbid the potential for collision. We can always >>> relax things in the future if a real need arises for QMP to >>> express member names that have been forbidden here. >>> >>> 'has_' only has to be reserved for struct/union member names, >>> while 'q_' is reserved everywhere (matching the fact that >>> we only have optional members, but use c_name() everywhere). >>> >>> Update and add tests to cover the new error messages. >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> >>> > >>> +++ b/scripts/qapi.py >>> @@ -376,7 +376,9 @@ def check_name(expr_info, source, name, allow_optional=False, >>> # code always prefixes it with the enum name >>> if enum_member: >>> membername = '_' + membername >>> - if not valid_name.match(membername): >>> + # Reserve the entire 'q_' namespace for c_name() >>> + if not valid_name.match(membername) or \ >>> + membername.replace('-', '_').startswith('q_'): >> >> Why not c_name(membername).startswith('q_')? > > Recursion. c_name('unix') is 'q_unix', which would be rejected, even > though we want to accept it. (And yes, that was my first try, before the > approach you see here) c_name(membername, False).startswith('q_')? [...]
On 10/23/2015 08:47 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> On 10/23/2015 07:05 AM, Markus Armbruster wrote: >>> Eric Blake <eblake@redhat.com> writes: >>> >>>> c_name() produces names starting with 'q_' when protecting >>>> a QMP member name that would fail to directly compile, but >>>> in doing so can cause clashes with any QMP name already >>>> beginning with 'q-' or 'q_'. Likewise, we create a C name >>>> 'has_' for any optional member, that can clash with any QMP >>>> name beginning with 'has-' or 'has_'. >>>> >>>> - if not valid_name.match(membername): >>>> + # Reserve the entire 'q_' namespace for c_name() >>>> + if not valid_name.match(membername) or \ >>>> + membername.replace('-', '_').startswith('q_'): >>> >>> Why not c_name(membername).startswith('q_')? >> >> Recursion. c_name('unix') is 'q_unix', which would be rejected, even >> though we want to accept it. (And yes, that was my first try, before the >> approach you see here) > > c_name(membername, False).startswith('q_')? Oh, I forgot about the optional bool. I'll try it.
diff --git a/docs/qapi-code-gen.txt b/docs/qapi-code-gen.txt index c4264a8..4d8c2fc 100644 --- a/docs/qapi-code-gen.txt +++ b/docs/qapi-code-gen.txt @@ -112,7 +112,9 @@ and field names within a type, should be all lower case with words separated by a hyphen. However, some existing older commands and complex types use underscore; when extending such expressions, consistency is preferred over blindly avoiding underscore. Event -names should be ALL_CAPS with words separated by underscore. +names should be ALL_CAPS with words separated by underscore. Field +names cannot start with 'has-' or 'has_', as this is reserved for +tracking optional fields. Any name (command, event, type, field, or enum value) beginning with "x-" is marked experimental, and may be withdrawn or changed @@ -123,9 +125,10 @@ vendor), even if the rest of the name uses dash (example: __com.redhat_drive-mirror). Other than downstream extensions (with leading underscore and the use of dots), all names should begin with a letter, and contain only ASCII letters, digits, dash, and underscore. -It is okay to reuse names that match C keywords; the generator will -rename a field named "default" in the QAPI to "q_default" in the -generated C code. +Names beginning with 'q_' are reserved for the generator: QMP names +that resemble C keywords or other problematic strings will be munged +in C to use this prefix. For example, a field named "default" in +qapi becomes "q_default" in the generated C code. In the rest of this document, usage lines are given for each expression type, with literal strings written in lower case and diff --git a/scripts/qapi.py b/scripts/qapi.py index d53b5c4..04bcbf7 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -376,7 +376,9 @@ def check_name(expr_info, source, name, allow_optional=False, # code always prefixes it with the enum name if enum_member: membername = '_' + membername - if not valid_name.match(membername): + # Reserve the entire 'q_' namespace for c_name() + if not valid_name.match(membername) or \ + membername.replace('-', '_').startswith('q_'): raise QAPIExprError(expr_info, "%s uses invalid name '%s'" % (source, name)) @@ -488,6 +490,10 @@ def check_type(expr_info, source, value, allow_array=False, for (key, arg) in value.items(): check_name(expr_info, "Member of %s" % source, key, allow_optional=allow_optional) + if c_name(key).startswith('has_'): + raise QAPIExprError(expr_info, + "Member of %s uses reserved name '%s'" + % (source, key)) # Todo: allow dictionaries to represent default values of # an optional argument. check_type(expr_info, "Member '%s' of %s" % (key, source), arg, diff --git a/tests/qapi-schema/args-has-clash.err b/tests/qapi-schema/args-has-clash.err index e69de29..595fe93 100644 --- a/tests/qapi-schema/args-has-clash.err +++ b/tests/qapi-schema/args-has-clash.err @@ -0,0 +1 @@ +tests/qapi-schema/args-has-clash.json:6: Member of 'data' for command 'oops' uses reserved name 'has-a' diff --git a/tests/qapi-schema/args-has-clash.exit b/tests/qapi-schema/args-has-clash.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/args-has-clash.exit +++ b/tests/qapi-schema/args-has-clash.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/args-has-clash.json b/tests/qapi-schema/args-has-clash.json index a2197de..38453e7 100644 --- a/tests/qapi-schema/args-has-clash.json +++ b/tests/qapi-schema/args-has-clash.json @@ -1,6 +1,6 @@ # C member name collision -# FIXME - This parses, but fails to compile, because the C struct is given -# two 'has_a' members, one from the flag for optional 'a', and the other -# from member 'has-a'. Either reject this at parse time, or munge the C -# names to avoid the collision. +# This would attempt to create two 'has_a' members of the C struct, one +# from the flag for optional 'a', and the other from member 'has-a'; so +# instead we reject any member with a name that would collide with 'has_'. +# TODO we could munge the optional flag name to avoid the collision. { 'command': 'oops', 'data': { '*a': 'str', 'has-a': 'str' } } diff --git a/tests/qapi-schema/args-has-clash.out b/tests/qapi-schema/args-has-clash.out index 5a18b6b..e69de29 100644 --- a/tests/qapi-schema/args-has-clash.out +++ b/tests/qapi-schema/args-has-clash.out @@ -1,6 +0,0 @@ -object :empty -object :obj-oops-arg - member a: str optional=True - member has-a: str optional=False -command oops :obj-oops-arg -> None - gen=True success_response=True diff --git a/tests/qapi-schema/reserved-command.err b/tests/qapi-schema/reserved-command.err index e69de29..a70856b 100644 --- a/tests/qapi-schema/reserved-command.err +++ b/tests/qapi-schema/reserved-command.err @@ -0,0 +1 @@ +tests/qapi-schema/reserved-command.json:6: 'command' uses invalid name 'q-unix' diff --git a/tests/qapi-schema/reserved-command.exit b/tests/qapi-schema/reserved-command.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/reserved-command.exit +++ b/tests/qapi-schema/reserved-command.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/reserved-command.json b/tests/qapi-schema/reserved-command.json index be9944c..d662a96 100644 --- a/tests/qapi-schema/reserved-command.json +++ b/tests/qapi-schema/reserved-command.json @@ -1,7 +1,6 @@ # C entity name collision -# FIXME - This parses, but fails to compile, because it attempts to declare -# two 'qmp_q_unix' functions (one for 'q-unix', the other because c_name() -# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should -# reject attempts to explicitly use 'q_' names, to reserve it for qapi. +# This would attempt to declare two 'qmp_q_unix' functions (one for 'q-unix', +# the other because c_name() munges 'unix' to 'q_unix' to avoid reserved word +# collisions). Therefore, we reserve 'q_' names for qapi. { 'command': 'unix' } { 'command': 'q-unix' } diff --git a/tests/qapi-schema/reserved-command.out b/tests/qapi-schema/reserved-command.out index b31b38f..e69de29 100644 --- a/tests/qapi-schema/reserved-command.out +++ b/tests/qapi-schema/reserved-command.out @@ -1,5 +0,0 @@ -object :empty -command q-unix None -> None - gen=True success_response=True -command unix None -> None - gen=True success_response=True diff --git a/tests/qapi-schema/reserved-member.err b/tests/qapi-schema/reserved-member.err index e69de29..8752c5b 100644 --- a/tests/qapi-schema/reserved-member.err +++ b/tests/qapi-schema/reserved-member.err @@ -0,0 +1 @@ +tests/qapi-schema/reserved-member.json:5: Member of 'data' for struct 'Foo' uses invalid name 'q-unix' diff --git a/tests/qapi-schema/reserved-member.exit b/tests/qapi-schema/reserved-member.exit index 573541a..d00491f 100644 --- a/tests/qapi-schema/reserved-member.exit +++ b/tests/qapi-schema/reserved-member.exit @@ -1 +1 @@ -0 +1 diff --git a/tests/qapi-schema/reserved-member.json b/tests/qapi-schema/reserved-member.json index 1602ed3..a8dac22 100644 --- a/tests/qapi-schema/reserved-member.json +++ b/tests/qapi-schema/reserved-member.json @@ -1,6 +1,5 @@ # C member name collision -# FIXME - This parses, but fails to compile, because it attempts to declare -# two 'q_unix' members (one for 'q-unix', the other because c_name() -# munges 'unix' to 'q_unix' to avoid reserved word collisions). We should -# reject attempts to explicitly use 'q_' names, to reserve it for qapi. +# This would attempt to declare two 'q_unix' members (one for 'q-unix', +# the other because c_name() munges 'unix' to 'q_unix' to avoid reserved word +# collisions). Therefore, we reserve 'q_' names for qapi. { 'struct': 'Foo', 'data': { 'unix':'int', 'q-unix':'bool' } } diff --git a/tests/qapi-schema/reserved-member.out b/tests/qapi-schema/reserved-member.out index 0d8685a..e69de29 100644 --- a/tests/qapi-schema/reserved-member.out +++ b/tests/qapi-schema/reserved-member.out @@ -1,4 +0,0 @@ -object :empty -object Foo - member unix: int optional=False - member q-unix: bool optional=False
c_name() produces names starting with 'q_' when protecting a QMP member name that would fail to directly compile, but in doing so can cause clashes with any QMP name already beginning with 'q-' or 'q_'. Likewise, we create a C name 'has_' for any optional member, that can clash with any QMP name beginning with 'has-' or 'has_'. Technically, rather than blindly reserving the namespace, we could try to complain about user names only when an actual collision occurs, or even teach c_name() how to munge names to avoid collisions. But it is not trivial, especially when collisions can occur across multiple types (such as via inheritance or flat unions). Besides, no existing .json files are trying to use these names. So it's easier to just outright forbid the potential for collision. We can always relax things in the future if a real need arises for QMP to express member names that have been forbidden here. 'has_' only has to be reserved for struct/union member names, while 'q_' is reserved everywhere (matching the fact that we only have optional members, but use c_name() everywhere). Update and add tests to cover the new error messages. Signed-off-by: Eric Blake <eblake@redhat.com> --- v10: retitle, improve comments, defer 'u' changes for later, use c_name(name).startswith('has_') rather than open coding v9: new patch --- docs/qapi-code-gen.txt | 11 +++++++---- scripts/qapi.py | 8 +++++++- tests/qapi-schema/args-has-clash.err | 1 + tests/qapi-schema/args-has-clash.exit | 2 +- tests/qapi-schema/args-has-clash.json | 8 ++++---- tests/qapi-schema/args-has-clash.out | 6 ------ tests/qapi-schema/reserved-command.err | 1 + tests/qapi-schema/reserved-command.exit | 2 +- tests/qapi-schema/reserved-command.json | 7 +++---- tests/qapi-schema/reserved-command.out | 5 ----- tests/qapi-schema/reserved-member.err | 1 + tests/qapi-schema/reserved-member.exit | 2 +- tests/qapi-schema/reserved-member.json | 7 +++---- tests/qapi-schema/reserved-member.out | 4 ---- 14 files changed, 30 insertions(+), 35 deletions(-)