[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v8 20/40] qapi: Better error messages for duplic
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v8 20/40] qapi: Better error messages for duplicated expressions |
Date: |
Tue, 05 May 2015 18:28:48 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> writes:
> On 05/05/2015 03:11 AM, Markus Armbruster wrote:
>> Eric Blake <address@hidden> writes:
>>
>>> 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.
>>>
>
>>> +def add_name(name, info, meta, implicit = False):
>>> + global all_names
>>> + if name in all_names:
>>> + raise QAPIExprError(info,
>>> + "%s '%s' is already defined"
>>> + %(all_names[name], name))
>>
>> Let's put a space between binary operator % and its right operand.
>
> I was copying existing examples, but I can easily do a separate followup
> patch that unifies the style of % formatting (or switches to +
> concatenation where that is more legible).
That would be nice.
>>> + if not implicit and name[-4:] == 'Kind':
>>> + raise QAPIExprError(info,
>>> + "%s '%s' should not end in 'Kind'"
>>> + %(meta, name))
>>
>> Likewise. Can fix up on commit.
>
> Of course, I'll wait until any of your fixups in the pull request are
> already in place to write such a followup, to minimize rebase churn.
I'm working towards a pull request. It has the appended cleanups
squashed into this patch and PATCH 40.
diff --git a/scripts/qapi.py b/scripts/qapi.py
index edfaf9e..166b74f 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -421,10 +421,10 @@ def check_member_clash(expr_info, base_name, data, source
= ""):
for key in data.keys():
if key.startswith('*'):
key = key[1:]
- if key in base_members or "*%s" %key in base_members:
+ if key in base_members or "*" + key in base_members:
raise QAPIExprError(expr_info,
"Member name '%s'%s clashes with base '%s'"
- %(key, source, base_name))
+ % (key, source, base_name))
if base.get('base'):
check_member_clash(expr_info, base['base'], data, source)
@@ -524,7 +524,7 @@ def check_union(expr, expr_info):
branch_struct = find_struct(value)
assert branch_struct
check_member_clash(expr_info, base, branch_struct['data'],
- " of branch '%s'" %key)
+ " of branch '%s'" % key)
# If the discriminator names an enum type, then all members
# of 'data' must also be members of the enum type.
@@ -800,11 +800,11 @@ def add_name(name, info, meta, implicit = False):
if name in all_names:
raise QAPIExprError(info,
"%s '%s' is already defined"
- %(all_names[name], name))
+ % (all_names[name], name))
if not implicit and name[-4:] == 'Kind':
raise QAPIExprError(info,
"%s '%s' should not end in 'Kind'"
- %(meta, name))
+ % (meta, name))
all_names[name] = meta
def add_struct(definition, info):
- [Qemu-devel] [PATCH v8 13/40] qapi: Segregate anonymous unions into alternates in generator, (continued)
- [Qemu-devel] [PATCH v8 13/40] qapi: Segregate anonymous unions into alternates in generator, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 15/40] qapi: Document new 'alternate' meta-type, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 12/40] qapi: Prepare for catching more semantic parse errors, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 14/40] qapi: Rename anonymous union type in test, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 19/40] qapi: Add tests of redefined expressions, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 17/40] qapi: Add some expr tests, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 16/40] qapi: Use 'alternate' to replace anonymous union, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 20/40] qapi: Better error messages for duplicated expressions, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 18/40] qapi: Better error messages for bad expressions, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 11/40] qapi: Tighten checking of unions, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 22/40] qapi: Unify type bypass and add tests, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 31/40] qapi: Forbid 'type' in schema, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 27/40] qapi: More rigorous checking for type safety bypass, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 29/40] qapi: Document 'struct' metatype, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 21/40] qapi: Allow true, false and null in schema json, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 23/40] qapi: Add some type check tests, Eric Blake, 2015/05/04
- [Qemu-devel] [PATCH v8 25/40] qapi: Require valid names, Eric Blake, 2015/05/04