From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:41334)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1YpYtG-0000QP-Js
	for qemu-devel@nongnu.org; Tue, 05 May 2015 05:11:49 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1YpYtB-0006Zq-GN
	for qemu-devel@nongnu.org; Tue, 05 May 2015 05:11:46 -0400
Received: from mx1.redhat.com ([209.132.183.28]:36874)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <armbru@redhat.com>) id 1YpYtB-0006ZX-B2
	for qemu-devel@nongnu.org; Tue, 05 May 2015 05:11:41 -0400
From: Markus Armbruster <armbru@redhat.com>
References: <1430751937-17523-1-git-send-email-eblake@redhat.com>
	<1430751937-17523-21-git-send-email-eblake@redhat.com>
Date: Tue, 05 May 2015 11:11:37 +0200
In-Reply-To: <1430751937-17523-21-git-send-email-eblake@redhat.com> (Eric
	Blake's message of "Mon, 4 May 2015 09:05:17 -0600")
Message-ID: <87ioc7v16e.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH v8 20/40] qapi: Better error messages for
	duplicated expressions
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Eric Blake <eblake@redhat.com>
Cc: kwolf@redhat.com, berto@igalia.com, qemu-devel@nongnu.org

Eric Blake <eblake@redhat.com> 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.
>
> Signed-off-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Markus Armbruster <armbru@redhat.com>
[...]
> diff --git a/scripts/qapi.py b/scripts/qapi.py
> index 868f08b..eea0976 100644
> --- a/scripts/qapi.py
> +++ b/scripts/qapi.py
[...]
> @@ -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))

Let's put a space between binary operator % and its right operand.

> +    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.

> +    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):
[...]