Re: [Qemu-devel] [RFC PATCH 14/32] qapi: Rework generated code for built-in types

[Markus Armbruster]

Marc-André Lureau  writes:

> Hi
>
> On Mon, Oct 2, 2017 at 5:25 PM, Markus Armbruster  wrote:
>> qapi-types.py and qapi-visit.py generate some C code for built-in
>> types.  To make this work with multiple schemas, we generate code for
>> built-ins into .c files only when the user asks for it with -b.  The
>> user is responsible for linking exactly one set of files generated
>> with -b per program.  We generate code for built-ins into .h
>> regardless of -b, but guard it with a preprocessor symbol.
>>
>> This is cumbersome and inflexible.  Move the code generated for
>> built-in types into separate files builtin-qapi-{types,visit}.{c,h}.
>> Run qapi-types.py and qapi-visit.py without a schema argument to
>> generate them.  Drop their option -b.
>>
>> Signed-off-by: Markus Armbruster 
>
>
> Good idea!
> I think I would still prefer to see a seperate argument to generate
> builtin files (rather than absence of schema), but this is minor
> detail.

An option to generate built-ins would have to conflict with -p and the
positional argument.  I tried the stupidest solution that could possibly
work first.

> Reviewed-by: Marc-André Lureau 

Thanks!

Re: [Qemu-devel] [RFC PATCH 14/32] qapi: Rework generated code for built-in types

[Marc-André Lureau]

Hi

On Mon, Oct 2, 2017 at 5:25 PM, Markus Armbruster  wrote:
> qapi-types.py and qapi-visit.py generate some C code for built-in
> types.  To make this work with multiple schemas, we generate code for
> built-ins into .c files only when the user asks for it with -b.  The
> user is responsible for linking exactly one set of files generated
> with -b per program.  We generate code for built-ins into .h
> regardless of -b, but guard it with a preprocessor symbol.
>
> This is cumbersome and inflexible.  Move the code generated for
> built-in types into separate files builtin-qapi-{types,visit}.{c,h}.
> Run qapi-types.py and qapi-visit.py without a schema argument to
> generate them.  Drop their option -b.
>
> Signed-off-by: Markus Armbruster 


Good idea!
I think I would still prefer to see a seperate argument to generate
builtin files (rather than absence of schema), but this is minor
detail.

Reviewed-by: Marc-André Lureau 


> ---
>  .gitignore   |  2 ++
>  Makefile | 18 --
>  Makefile.objs|  1 +
>  docs/devel/qapi-code-gen.txt | 24 +++--
>  qga/Makefile.objs|  1 +
>  scripts/qapi-introspect.py   |  4 +--
>  scripts/qapi-types.py| 54 ++--
>  scripts/qapi-visit.py| 62 
> 
>  scripts/qapi.py  | 57 -
>  scripts/qapi2texi.py |  2 +-
>  tests/Makefile.include   |  4 ++-
>  tests/qapi-schema/builtins.err   |  0
>  tests/qapi-schema/builtins.exit  |  1 +
>  tests/qapi-schema/builtins.json  |  1 +
>  tests/qapi-schema/builtins.out   |  3 ++
>  tests/qapi-schema/comments.out   |  3 --
>  tests/qapi-schema/doc-bad-section.out|  3 --
>  tests/qapi-schema/doc-good.out   |  3 --
>  tests/qapi-schema/empty.out  |  3 --
>  tests/qapi-schema/event-case.out |  3 --
>  tests/qapi-schema/ident-with-escape.out  |  3 --
>  tests/qapi-schema/include-relpath.out|  3 --
>  tests/qapi-schema/include-repetition.out |  3 --
>  tests/qapi-schema/include-simple.out |  3 --
>  tests/qapi-schema/indented-expr.out  |  3 --
>  tests/qapi-schema/qapi-schema-test.out   |  3 --
>  tests/qapi-schema/test-qapi.py   |  4 +--
>  27 files changed, 127 insertions(+), 144 deletions(-)
>  create mode 100644 tests/qapi-schema/builtins.err
>  create mode 100644 tests/qapi-schema/builtins.exit
>  create mode 100644 tests/qapi-schema/builtins.json
>  create mode 100644 tests/qapi-schema/builtins.out
>
> diff --git a/.gitignore b/.gitignore
> index 40acfcb9e2..84a57060ad 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -1,3 +1,5 @@
> +/builtin-qapi-types.[ch]
> +/builtin-qapi-visit.[ch]
>  /config-devices.*
>  /config-all-devices.*
>  /config-all-disas.*
> diff --git a/Makefile b/Makefile
> index 784b601247..421e65d833 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -52,6 +52,8 @@ endif
>  include $(SRC_PATH)/rules.mak
>
>  GENERATED_FILES = qemu-version.h config-host.h qemu-options.def
> +GENERATED_FILES += builtin-qapi-types.h builtin-qapi-types.c
> +GENERATED_FILES += builtin-qapi-visit.h builtin-qapi-visit.c
>  GENERATED_FILES += qmp-commands.h qapi-types.h qapi-visit.h qapi-event.h
>  GENERATED_FILES += qmp-marshal.c qapi-types.c qapi-visit.c qapi-event.c
>  GENERATED_FILES += qmp-introspect.h
> @@ -428,18 +430,30 @@ qapi-modules = $(SRC_PATH)/qapi-schema.json 
> $(SRC_PATH)/qapi/common.json \
> $(SRC_PATH)/qapi/transaction.json \
> $(SRC_PATH)/qapi/ui.json
>
> +.INTERMEDIATE: builtin-qapi-types-gen
> +builtin-qapi-types.c builtin-qapi-types.h: builtin-qapi-types-gen ;
> +builtin-qapi-types-gen: $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> +   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py, \
> +   "GEN","$@")
> +
> +.INTERMEDIATE: builtin-qapi-visit-gen
> +builtin-qapi-visit.c builtin-qapi-visit.h: builtin-qapi-visit-gen ;
> +builtin-qapi-visit-gen: $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> +   $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py, \
> +   "GEN","$@")
> +
>  .INTERMEDIATE: qapi-types-gen
>  qapi-types.c qapi-types.h: qapi-types-gen ;
>  qapi-types-gen: $(qapi-modules) $(SRC_PATH)/scripts/qapi-types.py $(qapi-py)
> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py \
> -   -b $<, \
> +   $<, \
> "GEN","$@")
>
>  .INTERMEDIATE: qapi-visit-gen
>  qapi-visit.c qapi-visit.h: qapi-visit-gen ;
>  qapi-visit-gen: $(qapi-modules) $(SRC_PATH)/scripts/qapi-visit.py $(qapi-py)
> $(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py \
> -   -b $<, \
> +