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 <mdr...@linux.vnet.ibm.com> > >>> --- > >>> 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 <mdr...@linux.vnet.ibm.com> > >>> + * > >>> + * 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() We could in theory scan for QIDL_PROPERTIES()/QIDL_SCHEMA_ADD_LINK()/QIDL_VISIT_TYPE() to avoid the need for QIDL_ENABLE(), but to me the latter approach seemed like it would scale better if we were to leverage QIDL for other things in the future. > > Paolo >
