Re: GObject introspection patches

[Mathias Hasselmann <mathias.hasselmann@gmx.de>]

[-- 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 --]