GObject introspection patches

GObject introspection patches

[Mathias Hasselmann]

[-- Attachment #1.1: Type: text/plain, Size: 569 bytes --]

Hello,

For GObject introspection c2xml has to be extended to output information
about custom attributes like this:

          GList* create_list() __attribute__((element_type(char*)))

Attached patch set adds support for custom attributes to c2xml. It also
changes c2xml to provide command line options and to output initializer
expressions. The patchset also is available via git:

http://taschenorakel.de/gitweb/?p=sparse;a=shortlog;h=gobject-introspection

Ciao,
Mathias
-- 
Mathias Hasselmann <mathias.hasselmann@gmx.de>
http://taschenorakel.de/

[-- Attachment #1.2: 0001-Introduce-attribute-structure-for-storing-custom-att.patch --]
[-- Type: application/mbox, Size: 4158 bytes --]

[-- Attachment #1.3: 0002-Extract-init_keyword_table-from-init_parser-for-usag.patch --]
[-- Type: application/mbox, Size: 2594 bytes --]

[-- Attachment #1.4: 0003-c2xml-Add-support-for-custom-attributes-and-omit-en.patch --]
[-- Type: application/mbox, Size: 8395 bytes --]

[-- Attachment #1.5: 0004-c2xml-Implement-command-line-options.patch --]
[-- Type: application/mbox, Size: 4144 bytes --]

[-- Attachment #1.6: 0005-c2xml-Output-initializer-expressions-needed-for-in.patch --]
[-- Type: application/mbox, Size: 1217 bytes --]

[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: GObject introspection patches

[Rob Taylor]

Mathias Hasselmann wrote:
> Hello,
> 
> For GObject introspection c2xml has to be extended to output information
> about custom attributes like this:
> 
>           GList* create_list() __attribute__((element_type(char*)))
> 
> Attached patch set adds support for custom attributes to c2xml. It also
> changes c2xml to provide command line options and to output initializer
> expressions. The patchset also is available via git:
> 
> http://taschenorakel.de/gitweb/?p=sparse;a=shortlog;h=gobject-introspection
> 
> Ciao,
> Mathias

> From 98fae4f8d2d1f375a18aeaf1e0a0c71107fbeba8 Mon Sep 17 00:00:00 2001
> From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> Date: Tue, 4 Sep 2007 22:26:47 +0200
> Subject: [PATCH] Introduce attribute structure for storing custom attributes in symbol nodes.
> 
> Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>

This patch looks good to me, from my (albeit limited) knowledge of
sparse internals. It would be good to have someone with deeper
understanding comment.

From 124215ffb5ba8ae28028588e1f12d2a586d21e20 Mon Sep 17 00:00:00 2001
From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
Date: Tue, 4 Sep 2007 22:30:14 +0200
Subject: [PATCH] Extract init_keyword_table from init_parser for usage
in c2xml.

Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>

Again, this looks fine to me.

> From 42a4d680b17d6ed4d0cc1108cef5d1b299495f01 Mon Sep 17 00:00:00 2001
> From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> Date: Tue, 4 Sep 2007 22:34:15 +0200
> Subject: [PATCH] c2xml: Add support for custom attributes and omit end position information when
> equal to start position. Custom attributes are needed for GObject introspection
> to express details like this:
> 
>       GList* create_list() __attribute__((element_type(char*)))
> 
> Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>

This looks mostly fine, though I'm not sure about defining
attribute_table inside c2xml. Either these are generally useful and so
should be in (say) sparse.c, or they're specific to
Gobject-introspection, and so there should be a way of providing them as
input to c2xml, perhaps defined in an input xml document, and
namespacing them in the output appropriately?

> From be96d3c60e9e52e6448bf90444eb1e8bb3a48d5c Mon Sep 17 00:00:00 2001
> From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> Date: Tue, 4 Sep 2007 22:44:58 +0200
> Subject: [PATCH] c2xml: Implement command line options:
> 
>     -d           include document type declaration
>     -p           output position information for symbols
>     -o FILENAME  write the document to FILENAME
>     -h           display usage information - this page
> 
> Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>

This looks like it breaks commandline parsing, Mathias has already told
me on irc it needs fixing :) The options it gives could be useful
though, if refactored to work correctly with the sparse commandline parsing.

> From 1605b862637aecd54cfd5b159c96415bdddf46c3 Mon Sep 17 00:00:00 2001
> From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> Date: Tue, 4 Sep 2007 22:47:50 +0200
> Subject: [PATCH] c2xml: Output initializer expressions, needed for instance for enum values.
> 
> Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>

The second hunk of this patch looks unnecessary, and I think the first
hunk should be included in 42a4d680.

Thanks,
Rob Taylor



-- 
Rob Taylor, Codethink Ltd. -  http://codethink.co.uk

Re: GObject introspection patches

[Josh Triplett]

Thanks for reviewing these, Rob.  Further comments below.

On Wed, 2007-09-05 at 20:21 +0100, Rob Taylor wrote:
> Mathias Hasselmann wrote:
> > Hello,
> > 
> > For GObject introspection c2xml has to be extended to output information
> > about custom attributes like this:
> > 
> >           GList* create_list() __attribute__((element_type(char*)))
> > 
> > Attached patch set adds support for custom attributes to c2xml. It also
> > changes c2xml to provide command line options and to output initializer
> > expressions. The patchset also is available via git:
> > 
> > http://taschenorakel.de/gitweb/?p=sparse;a=shortlog;h=gobject-introspection
> > 
> > Ciao,
> > Mathias
> 
> > From 98fae4f8d2d1f375a18aeaf1e0a0c71107fbeba8 Mon Sep 17 00:00:00 2001
> > From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> > Date: Tue, 4 Sep 2007 22:26:47 +0200
> > Subject: [PATCH] Introduce attribute structure for storing custom attributes in symbol nodes.
> > 
> > Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> 
> This patch looks good to me, from my (albeit limited) knowledge of
> sparse internals. It would be good to have someone with deeper
> understanding comment.

Looks reasonable to me too; I wonder if this generalization could cover
some other things like contexts, particularly with appropriate
factored-out functions to combine attributes from symbols.

I'd like to hear any comments from others as well.

> From 124215ffb5ba8ae28028588e1f12d2a586d21e20 Mon Sep 17 00:00:00 2001
> From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> Date: Tue, 4 Sep 2007 22:30:14 +0200
> Subject: [PATCH] Extract init_keyword_table from init_parser for usage
> in c2xml.
> 
> Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> 
> Again, this looks fine to me.

Goes with the next change; seems OK, though I'd also like to hear
further comments from other people.

> > From 42a4d680b17d6ed4d0cc1108cef5d1b299495f01 Mon Sep 17 00:00:00 2001
> > From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> > Date: Tue, 4 Sep 2007 22:34:15 +0200
> > Subject: [PATCH] c2xml: Add support for custom attributes and omit end position information when
> > equal to start position. Custom attributes are needed for GObject introspection
> > to express details like this:
> > 
> >       GList* create_list() __attribute__((element_type(char*)))
> > 
> > Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> 
> This looks mostly fine, though I'm not sure about defining
> attribute_table inside c2xml. Either these are generally useful and so
> should be in (say) sparse.c, or they're specific to
> Gobject-introspection, and so there should be a way of providing them as
> input to c2xml, perhaps defined in an input xml document, and
> namespacing them in the output appropriately?

At a minimum, Sparse should know about these attributes in order to
ignore them.  In theory, Sparse should ignore any attributes it doesn't
know about, like GCC's -Wno-attributes, but that seems error-prone.
Alternatively, we could introduce some new builtin for testing for
available attributes; feature testing seems better than version testing,
and Sparse does develop new features over time, so that might help for
new Sparse features as well.  That would allow something like
conditional compilation based on individual attributes:
#if defined(__CHECKER__) && __attribute_exists__(element_type)
#define element_type(x) __attribute__((element_type(x)))
#endif

> > From be96d3c60e9e52e6448bf90444eb1e8bb3a48d5c Mon Sep 17 00:00:00 2001
> > From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> > Date: Tue, 4 Sep 2007 22:44:58 +0200
> > Subject: [PATCH] c2xml: Implement command line options:
> > 
> >     -d           include document type declaration
> >     -p           output position information for symbols
> >     -o FILENAME  write the document to FILENAME
> >     -h           display usage information - this page
> > 
> > Signed-off-by: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> 
> This looks like it breaks commandline parsing, Mathias has already told
> me on irc it needs fixing :) The options it gives could be useful
> though, if refactored to work correctly with the sparse commandline parsing.

Just to clarify something: feel free to refactor Sparse's command-line
handling itself, which I think needs some work to make it usable by
anything other than the sparse backend.

- Josh Triplett

Re: GObject introspection patches

[Mathias Hasselmann]

[-- Attachment #1.1: Type: text/plain, Size: 3137 bytes --]

Rob and Josh, thank you for review. Attaching more patches.

Am Mittwoch, den 05.09.2007, 13:36 -0700 schrieb Josh Triplett:
> Thanks for reviewing these, Rob.  Further comments below.
> 
> > > From 98fae4f8d2d1f375a18aeaf1e0a0c71107fbeba8 Mon Sep 17 00:00:00 2001
> > > From: Mathias Hasselmann <mathias.hasselmann@gmx.de>
> > > Date: Tue, 4 Sep 2007 22:26:47 +0200
> > > Subject: [PATCH] Introduce attribute structure for storing custom attributes in symbol nodes.
>
> Looks reasonable to me too; I wonder if this generalization could cover
> some other things like contexts, particularly with appropriate
> factored-out functions to combine attributes from symbols.

Seems that I do not know Sparse well enough yet to understand
which chance for refactoring you see.

> > > Subject: [PATCH] c2xml: Add support for custom attributes and omit end position information when
> > > equal to start position. Custom attributes are needed for GObject introspection
> > > to express details like this:
> > > 
> > This looks mostly fine, though I'm not sure about defining
> > attribute_table inside c2xml. Either these are generally useful and so
> > should be in (say) sparse.c, or they're specific to
> > Gobject-introspection, and so there should be a way of providing them as
> > input to c2xml, perhaps defined in an input xml document, and
> > namespacing them in the output appropriately?

Having definitions for GObject specific attributes within an external
file seems reasonable for me: No need to patch sparse everytime we GNOME
guys invent a new attribute for our code. But instead of using XML for
that external definition I feel like using Sparse's builtin tokenizer. 

> At a minimum, Sparse should know about these attributes in order to
> ignore them.  In theory, Sparse should ignore any attributes it doesn't
> know about, like GCC's -Wno-attributes, but that seems error-prone.
> Alternatively, we could introduce some new builtin for testing for
> available attributes; feature testing seems better than version testing,
> and Sparse does develop new features over time, so that might help for
> new Sparse features as well.  That would allow something like
> conditional compilation based on individual attributes:
> #if defined(__CHECKER__) && __attribute_exists__(element_type)
> #define element_type(x) __attribute__((element_type(x)))
> #endif

Agreed. This __attribute_exists__ builtin looks like a pretty idea.
Guess we also should convince the gcc hackers to introduce it.

> > > Subject: [PATCH] c2xml: Implement command line options:
>
> Just to clarify something: feel free to refactor Sparse's command-line
> handling itself, which I think needs some work to make it usable by
> anything other than the sparse backend.

Attached there is a new variant of the patch, playing nice with Sparse's
command-line parse. Concept is borrowed from glib's GOptionContext. Upon
approval I'll modify Sparse's command line parser to use this extensible
infrastructure.

Ciao,
Mathias
-- 
Mathias Hasselmann <mathias.hasselmann@gmx.de>
http://taschenorakel.de/

[-- Attachment #1.2: 0001-parse.dtd-Fix-errors-in-definition-of-the-symbol-el.patch --]
[-- Type: application/mbox, Size: 2145 bytes --]

[-- Attachment #1.3: 0002-c2xml-Output-argument-and-expansion-nodes-for-macro.patch --]
[-- Type: application/mbox, Size: 4459 bytes --]

[-- Attachment #1.4: 0003-c2xml-Implement-support-for-command-line-options-b.patch --]
[-- Type: application/mbox, Size: 6255 bytes --]

[-- Attachment #2: Dies ist ein digital signierter Nachrichtenteil --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: GObject introspection patches

[Mathias Hasselmann]

> > > > Subject: [PATCH] Introduce attribute structure for storing custom
> attributes in symbol nodes.
> >
> > Looks reasonable to me too; I wonder if this generalization could cover
> > some other things like contexts, particularly with appropriate
> > factored-out functions to combine attributes from symbols.
> 
> Seems that I do not know Sparse well enough yet to understand
> which chance for refactoring you see.

After some grepping (and googling) I see that sparse supports some context attribute for spin-lock tracking, and that the contexts list of struct ctype is for storing this attributes. So I agree, definitly a field for refactoring and unification. Started to work on that.
-- 
Mathias Hasselmann
http://taschenorakel.de/mathias/

Psssst! Schon vom neuen GMX MultiMessenger gehört?
Der kanns mit allen: http://www.gmx.net/de/go/multimessenger