[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl ann
From: |
Michael Roth |
Subject: |
Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations |
Date: |
Mon, 15 Oct 2012 10:50:02 -0500 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Mon, Oct 15, 2012 at 03:37:18PM +0200, Paolo Bonzini wrote:
> Il 05/10/2012 18:47, Michael Roth ha scritto:
> > On Fri, Oct 05, 2012 at 05:53:09PM +0200, Paolo Bonzini wrote:
> >> Il 05/10/2012 17:41, Michael Roth ha scritto:
> >>> On Fri, Oct 05, 2012 at 05:07:46PM +0200, Paolo Bonzini wrote:
> >>>> Il 04/10/2012 19:33, Michael Roth ha scritto:
> >>>>> Signed-off-by: Michael Roth <address@hidden>
> >>>>> ---
> >>>>> qidl.h | 113
> >>>>> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>> 1 file changed, 113 insertions(+)
> >>>>> create mode 100644 qidl.h
> >>>>>
> >>>>> diff --git a/qidl.h b/qidl.h
> >>>>> new file mode 100644
> >>>>> index 0000000..eae0202
> >>>>> --- /dev/null
> >>>>> +++ b/qidl.h
> >>>>> @@ -0,0 +1,113 @@
> >>>>> +/*
> >>>>> + * QEMU IDL Macros/stubs
> >>>>> + *
> >>>>> + * See docs/qidl.txt for usage information.
> >>>>> + *
> >>>>> + * Copyright IBM, Corp. 2012
> >>>>> + *
> >>>>> + * Authors:
> >>>>> + * Michael Roth <address@hidden>
> >>>>> + *
> >>>>> + * This work is licensed under the terms of the GNU GPLv2 or later.
> >>>>> + * See the COPYING file in the top-level directory.
> >>>>> + *
> >>>>> + */
> >>>>> +
> >>>>> +#ifndef QIDL_H
> >>>>> +#define QIDL_H
> >>>>> +
> >>>>> +#include <glib.h>
> >>>>> +#include "qapi/qapi-visit-core.h"
> >>>>> +#include "qemu/object.h"
> >>>>> +#include "hw/qdev-properties.h"
> >>>>> +
> >>>>> +#ifdef QIDL_GEN
> >>>>> +
> >>>>> +/* we pass the code through the preprocessor with QIDL_GEN defined to
> >>>>> parse
> >>>>> + * structures as they'd appear after preprocessing, and use the
> >>>>> following
> >>>>> + * definitions mostly to re-insert the initial macros/annotations so
> >>>>> they
> >>>>> + * stick around for the parser to process
> >>>>> + */
> >>>>> +#define QIDL(...) QIDL(__VA_ARGS__)
> >>>>> +#define QIDL_START(name, ...) QIDL_START(name, ##__VA_ARGS__)
> >>>>> +
> >>>>> +#define QIDL_VISIT_TYPE(name, v, s, f, e)
> >>>>> +#define QIDL_SCHEMA_ADD_LINK(name, obj, path, errp)
> >>>>> +#define QIDL_PROPERTIES(name)
> >>>>
> >>>> Ok, a few questions...
> >>>>
> >>>> Why do you need these to expand to nothing in the QIDL_GEN case?
> >>>>
> >>>
> >>> They don't need to, I was just trying to be explicit about what
> >>> directives were relevant to the parser and which ones were relevant to
> >>> the actually compiled code. It was more a development "aid" than
> >>> anything else though, so I think we can drop the special handling and
> >>> clean these up a bit.
> >>
> >> Yes, thanks!
> >>
> >>>>> +#define QIDL_DECLARE(name, ...) \
> >>>>
> >>>> Can QIDL_DECLARE replace QIDL_ENABLED as the magic detection string for
> >>>> qidl compilation?
> >>>>
> >>>
> >>> In some cases the declarations will come via #include'd headers, so the
> >>> only way to do that reliable is to run it through the preprocessor
> >>> first, which is how things were done in v1. But running everything
> >>> through cpp adds substantial overhead, and just because a QIDL-fied
> >>> struct is included in a C file, it doesn't mean that the C file intends
> >>> to use any qidl-generated code.
> >>
> >> Ok, I guess I need to see some example. We can clean it up later if we
> >> find a more clever way to do things.
> >
> > This was the main example I hit (not yet rebased):
> >
> > https://github.com/mdroth/qemu/commit/d8ea7c7a882e2fcbd0a9b7ab9ea47a389f87d31b
> >
> > As part of that patch We add annotations to PCIDevice in pci.h, which then
> > gets
> > pulled in from quite a few devices. So we end up with *.qidl.c files for
> > devices
> > that don't expose a "state" property or even have a QIDL_DECLARE()
> > directive.
> >
> > If we were to scan for QIDL_DECLARE() in advance of running it through
> > the preprocessor, we'd address a lot of those case. But then we miss
> > cases like this:
> >
> > https://github.com/mdroth/qemu/commit/2199f721daebd5c3961069bdd51de80a5b4fa827
> >
> > where, in pci.c, we use code generated from declarations in pci_internals.h
> > even
> > though pci.c doesn't contain a QIDL_DECLARE()
>
> Hmm, this opens another can of worms. There is a substantial amount of
> duplicated code between generated files. For example,
> visit_type_PCIDevice is found in all *.qidl.c files for PCI devices.
> Worse, the same is true for the properties array.
>
> Right now, QIDL_DECLARE is a no-op at code-generation time. Could it be
> a marker to generate code for that particular struct? Then you would
> put a normal
>
> struct PCIDevice {
> };
>
> declaration in hw/pci.h, and a
>
> QIDL_DECLARE(PCIDevice);
>
> in hw/pci.c that would trigger creation of the visitor etc. The code
> generator can also prepare extern declarations for types that are used
> but not defined, for example visit_type_PCIDevice in piix_pci.qidl.c.
Hmm, this could work... what I'd probably do though is:
- for cases where we're annotating "public" structs that may get pulled
into multiple files, use QIDL_DECLARE_PUBLIC() in place of
QIDL_DECLARE(). This will tell the code gen/qidl.h to only declare extern
declarations for code we'll be linking against to access the generated
visitors/properties/etc for that struct
- within one or a few common objects that everyone links against (we
can add one if one doesn't already exists), we can have, say, #include
"hw/pci.h", and in the body we have a
QIDL_IMPLEMENT_PUBLIC(PCIDevice), which tells the code gen to inject
the code there like it would for QIDL_DECLARE().
If this seems reasonable, I could probably implement this as a follow-up, prior
to any large-scale conversions.
>
> Paolo
>
- [Qemu-devel] [PATCH v3 18/22] qidl: add lexer library (based on QC parser), (continued)
- [Qemu-devel] [PATCH v3 18/22] qidl: add lexer library (based on QC parser), Michael Roth, 2012/10/04
- [Qemu-devel] [PATCH v3 17/22] qidl: add documentation, Michael Roth, 2012/10/04
- [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations, Michael Roth, 2012/10/04
- Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations, Paolo Bonzini, 2012/10/05
- Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations, Paolo Bonzini, 2012/10/05
- Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations, Michael Roth, 2012/10/05
- Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations, Paolo Bonzini, 2012/10/05
- Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations, Michael Roth, 2012/10/05
- Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations, Paolo Bonzini, 2012/10/15
- Re: [Qemu-devel] [PATCH v3 21/22] qidl: qidl.h, definitions for qidl annotations,
Michael Roth <=
[Qemu-devel] [PATCH v3 22/22] qidl: unit tests and build infrastructure, Michael Roth, 2012/10/04
[Qemu-devel] [PATCH v3 16/22] qdev: move Property-related declarations to qdev-properties.h, Michael Roth, 2012/10/04
[Qemu-devel] [PATCH v3 19/22] qidl: add C parser (based on QC parser), Michael Roth, 2012/10/04
[Qemu-devel] [PATCH v3 14/22] qom-fuse: workaround for truncated properties > 4096, Michael Roth, 2012/10/04
[Qemu-devel] [PATCH v3 20/22] qidl: add QAPI-based code generator, Michael Roth, 2012/10/04