qapi: Better error messages for duplicated expressions
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: Eric Blake <eblake@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Markus Armbruster <armbru@redhat.com>
This commit is contained in:
Eric Blake
Markus Armbruster
parent
cfdd5bcad5
commit
4dc2e6906e
@ -32,6 +32,12 @@ builtin_types = {
|
||||
'size': 'QTYPE_QINT',
|
||||
}
|
||||
|
||||
enum_types = []
|
||||
struct_types = []
|
||||
union_types = []
|
||||
events = []
|
||||
all_names = {}
|
||||
|
||||
def error_path(parent):
|
||||
res = ""
|
||||
while parent:
|
||||
@ -256,7 +262,14 @@ def discriminator_find_enum_define(expr):
|
||||
return find_enum(discriminator_type)
|
||||
|
||||
def check_event(expr, expr_info):
|
||||
global events
|
||||
name = expr['event']
|
||||
params = expr.get('data')
|
||||
|
||||
if name.upper() == 'MAX':
|
||||
raise QAPIExprError(expr_info, "Event name 'MAX' cannot be created")
|
||||
events.append(name)
|
||||
|
||||
if params:
|
||||
for argname, argentry, optional, structured in parse_args(params):
|
||||
if structured:
|
||||
@ -430,6 +443,9 @@ def check_keys(expr_elem, meta, required, optional=[]):
|
||||
|
||||
|
||||
def parse_schema(input_file):
|
||||
global all_names
|
||||
exprs = []
|
||||
|
||||
# First pass: read entire file into memory
|
||||
try:
|
||||
schema = QAPISchema(open(input_file, "r"))
|
||||
@ -437,30 +453,34 @@ def parse_schema(input_file):
|
||||
print >>sys.stderr, e
|
||||
exit(1)
|
||||
|
||||
exprs = []
|
||||
|
||||
try:
|
||||
# Next pass: learn the types and check for valid expression keys. At
|
||||
# this point, top-level 'include' has already been flattened.
|
||||
for builtin in builtin_types.keys():
|
||||
all_names[builtin] = 'built-in'
|
||||
for expr_elem in schema.exprs:
|
||||
expr = expr_elem['expr']
|
||||
info = expr_elem['info']
|
||||
if expr.has_key('enum'):
|
||||
check_keys(expr_elem, 'enum', ['data'])
|
||||
add_enum(expr['enum'], expr['data'])
|
||||
add_enum(expr['enum'], info, expr['data'])
|
||||
elif expr.has_key('union'):
|
||||
check_keys(expr_elem, 'union', ['data'],
|
||||
['base', 'discriminator'])
|
||||
add_union(expr)
|
||||
add_union(expr, info)
|
||||
elif expr.has_key('alternate'):
|
||||
check_keys(expr_elem, 'alternate', ['data'])
|
||||
add_name(expr['alternate'], info, 'alternate')
|
||||
elif expr.has_key('type'):
|
||||
check_keys(expr_elem, 'type', ['data'], ['base'])
|
||||
add_struct(expr)
|
||||
add_struct(expr, info)
|
||||
elif expr.has_key('command'):
|
||||
check_keys(expr_elem, 'command', [],
|
||||
['data', 'returns', 'gen', 'success-response'])
|
||||
add_name(expr['command'], info, 'command')
|
||||
elif expr.has_key('event'):
|
||||
check_keys(expr_elem, 'event', [], ['data'])
|
||||
add_name(expr['event'], info, 'event')
|
||||
else:
|
||||
raise QAPIExprError(expr_elem['info'],
|
||||
"Expression is missing metatype")
|
||||
@ -471,9 +491,11 @@ def parse_schema(input_file):
|
||||
expr = expr_elem['expr']
|
||||
if expr.has_key('union'):
|
||||
if not discriminator_find_enum_define(expr):
|
||||
add_enum('%sKind' % expr['union'])
|
||||
add_enum('%sKind' % expr['union'], expr_elem['info'],
|
||||
implicit=True)
|
||||
elif expr.has_key('alternate'):
|
||||
add_enum('%sKind' % expr['alternate'])
|
||||
add_enum('%sKind' % expr['alternate'], expr_elem['info'],
|
||||
implicit=True)
|
||||
|
||||
# Final pass - validate that exprs make sense
|
||||
check_exprs(schema)
|
||||
@ -567,12 +589,22 @@ def type_name(name):
|
||||
return c_list_type(name[0])
|
||||
return name
|
||||
|
||||
enum_types = []
|
||||
struct_types = []
|
||||
union_types = []
|
||||
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))
|
||||
if not implicit and name[-4:] == 'Kind':
|
||||
raise QAPIExprError(info,
|
||||
"%s '%s' should not end in 'Kind'"
|
||||
% (meta, name))
|
||||
all_names[name] = meta
|
||||
|
||||
def add_struct(definition):
|
||||
def add_struct(definition, info):
|
||||
global struct_types
|
||||
name = definition['type']
|
||||
add_name(name, info, 'struct')
|
||||
struct_types.append(definition)
|
||||
|
||||
def find_struct(name):
|
||||
@ -582,8 +614,10 @@ def find_struct(name):
|
||||
return struct
|
||||
return None
|
||||
|
||||
def add_union(definition):
|
||||
def add_union(definition, info):
|
||||
global union_types
|
||||
name = definition['union']
|
||||
add_name(name, info, 'union')
|
||||
union_types.append(definition)
|
||||
|
||||
def find_union(name):
|
||||
@ -593,8 +627,9 @@ def find_union(name):
|
||||
return union
|
||||
return None
|
||||
|
||||
def add_enum(name, enum_values = None):
|
||||
def add_enum(name, info, enum_values = None, implicit = False):
|
||||
global enum_types
|
||||
add_name(name, info, 'enum', implicit)
|
||||
enum_types.append({"enum_name": name, "enum_values": enum_values})
|
||||
|
||||
def find_enum(name):
|
||||
@ -636,7 +671,7 @@ def c_type(name, is_param=False):
|
||||
return name
|
||||
elif name == None or len(name) == 0:
|
||||
return 'void'
|
||||
elif name == name.upper():
|
||||
elif name in events:
|
||||
return '%sEvent *%s' % (camel_case(name), eatspace)
|
||||
else:
|
||||
return '%s *%s' % (name, eatspace)
|
||||
|
||||
@ -0,0 +1 @@
|
||||
tests/qapi-schema/command-int.json:2: built-in 'int' is already defined
|
||||
@ -1 +1 @@
|
||||
0
|
||||
1
|
||||
|
||||
@ -1,3 +1,3 @@
|
||||
# FIXME: we should reject collisions between commands and types
|
||||
# we reject collisions between commands and types
|
||||
{ 'command': 'int', 'data': { 'character': 'str' },
|
||||
'returns': { 'value': 'int' } }
|
||||
|
||||
@ -1,3 +0,0 @@
|
||||
[OrderedDict([('command', 'int'), ('data', OrderedDict([('character', 'str')])), ('returns', OrderedDict([('value', 'int')]))])]
|
||||
[]
|
||||
[]
|
||||
@ -0,0 +1 @@
|
||||
tests/qapi-schema/enum-union-clash.json:2: enum 'UnionKind' should not end in 'Kind'
|
||||
@ -1 +1 @@
|
||||
0
|
||||
1
|
||||
|
||||
@ -1,4 +1,4 @@
|
||||
# FIXME: we should reject types that would conflict with implicit union enum
|
||||
# we reject types that would conflict with implicit union enum
|
||||
{ 'enum': 'UnionKind', 'data': [ 'oops' ] }
|
||||
{ 'union': 'Union',
|
||||
'data': { 'a': 'int' } }
|
||||
|
||||
@ -1,5 +0,0 @@
|
||||
[OrderedDict([('enum', 'UnionKind'), ('data', ['oops'])]),
|
||||
OrderedDict([('union', 'Union'), ('data', OrderedDict([('a', 'int')]))])]
|
||||
[{'enum_name': 'UnionKind', 'enum_values': ['oops']},
|
||||
{'enum_name': 'UnionKind', 'enum_values': None}]
|
||||
[]
|
||||
@ -0,0 +1 @@
|
||||
tests/qapi-schema/event-max.json:2: Event name 'MAX' cannot be created
|
||||
@ -1 +1 @@
|
||||
0
|
||||
1
|
||||
|
||||
@ -1,2 +1,2 @@
|
||||
# FIXME: an event named 'MAX' would conflict with implicit C enum
|
||||
# an event named 'MAX' would conflict with implicit C enum
|
||||
{ 'event': 'MAX' }
|
||||
|
||||
@ -1,3 +0,0 @@
|
||||
[OrderedDict([('event', 'MAX')])]
|
||||
[]
|
||||
[]
|
||||
@ -0,0 +1 @@
|
||||
tests/qapi-schema/redefined-builtin.json:2: built-in 'size' is already defined
|
||||
@ -1 +1 @@
|
||||
0
|
||||
1
|
||||
|
||||
@ -1,2 +1,2 @@
|
||||
# FIXME: we should reject types that duplicate builtin names
|
||||
# we reject types that duplicate builtin names
|
||||
{ 'type': 'size', 'data': { 'myint': 'size' } }
|
||||
|
||||
@ -1,3 +0,0 @@
|
||||
[OrderedDict([('type', 'size'), ('data', OrderedDict([('myint', 'size')]))])]
|
||||
[]
|
||||
[OrderedDict([('type', 'size'), ('data', OrderedDict([('myint', 'size')]))])]
|
||||
@ -0,0 +1 @@
|
||||
tests/qapi-schema/redefined-command.json:3: command 'foo' is already defined
|
||||
@ -1 +1 @@
|
||||
0
|
||||
1
|
||||
|
||||
@ -1,3 +1,3 @@
|
||||
# FIXME: we should reject commands defined more than once
|
||||
# we reject commands defined more than once
|
||||
{ 'command': 'foo', 'data': { 'one': 'str' } }
|
||||
{ 'command': 'foo', 'data': { '*two': 'str' } }
|
||||
|
||||
@ -1,4 +0,0 @@
|
||||
[OrderedDict([('command', 'foo'), ('data', OrderedDict([('one', 'str')]))]),
|
||||
OrderedDict([('command', 'foo'), ('data', OrderedDict([('*two', 'str')]))])]
|
||||
[]
|
||||
[]
|
||||
@ -0,0 +1 @@
|
||||
tests/qapi-schema/redefined-event.json:3: event 'EVENT_A' is already defined
|
||||
@ -1 +1 @@
|
||||
0
|
||||
1
|
||||
|
||||
@ -1,3 +1,3 @@
|
||||
# FIXME: we should reject duplicate events
|
||||
# we reject duplicate events
|
||||
{ 'event': 'EVENT_A', 'data': { 'myint': 'int' } }
|
||||
{ 'event': 'EVENT_A', 'data': { 'myint': 'int' } }
|
||||
|
||||
@ -1,4 +0,0 @@
|
||||
[OrderedDict([('event', 'EVENT_A'), ('data', OrderedDict([('myint', 'int')]))]),
|
||||
OrderedDict([('event', 'EVENT_A'), ('data', OrderedDict([('myint', 'int')]))])]
|
||||
[]
|
||||
[]
|
||||
@ -0,0 +1 @@
|
||||
tests/qapi-schema/redefined-type.json:3: struct 'foo' is already defined
|
||||
@ -1 +1 @@
|
||||
0
|
||||
1
|
||||
|
||||
@ -1,3 +1,3 @@
|
||||
# FIXME: we should reject types defined more than once
|
||||
# we reject types defined more than once
|
||||
{ 'type': 'foo', 'data': { 'one': 'str' } }
|
||||
{ 'enum': 'foo', 'data': [ 'two' ] }
|
||||
|
||||
@ -1,4 +0,0 @@
|
||||
[OrderedDict([('type', 'foo'), ('data', OrderedDict([('one', 'str')]))]),
|
||||
OrderedDict([('enum', 'foo'), ('data', ['two'])])]
|
||||
[{'enum_name': 'foo', 'enum_values': ['two']}]
|
||||
[OrderedDict([('type', 'foo'), ('data', OrderedDict([('one', 'str')]))])]
|
||||
Loading…
Reference in New Issue
Block a user