GNOME Bugzilla – Bug 656445
const qualifier is not in the .gir
Last modified: 2015-02-07 17:03:05 UTC
And is needed to generate C API docs from the .gir
We should put this in another keyword on the type tag.
There should probably be two different parts: a) parse the const and do put an useful attribute on the type tag b) retain the complete type in the c namespace.
Hi, I recently needed those complete C types in gir files for my gmmproc refactor (a script for generating sources for gtkmm, glibmm and such). So, there goes some patches. I would be grateful for review.
Created attachment 218052 [details] [review] Ignore SLetter gir. Not necessarily relevant to this bug.
Created attachment 218053 [details] [review] Fix pointer parsing in scannerparser.y
Created attachment 218054 [details] [review] Write complete C type in c:type attribute.
Created attachment 218055 [details] [review] Adapt the tests for detailed c:type.
I'd prefer the second and third patches to be squashed to keep the tests passing for all commits. It's interesting that the first patch has no effect on the tests. We should add a test that uses "volatile" if we're going to support it in the parser. Probably a structure member? In theory, supporting "volatile" would require it to be exposed in bindings, so they could cast to (volatile type*) before doing a read. But in reality there's an immense amount of code between a binding structure read and the actual access, so it's not like the compiler is going to be able to e.g. keep the structure member in a register between reads.
Review of attachment 218053 [details] [review]: ::: giscanner/scannerparser.y @@ +1089,3 @@ | '*' type_qualifier_list pointer { + GISourceType **base = &($3->base_type); I don't understand the double indirection. Couldn't we drop a * and & here?
Review of attachment 218054 [details] [review]: Hmm...kind of unfortuante that we need to have a new complete_ctype with more data, but I understand why you did it this way. ::: giscanner/transformer.py @@ +439,3 @@ + def _create_complete_source_type(self, source_type): + if source_type is None or source_type.type == CTYPE_INVALID: + return 'None' 'None' as a string? I guess you just copied that from _create_source_type() but it seems broken. We're probably just not hitting that case. Update: confirmed. If I change that line to "assert False", everything still builds and tests pass. @@ +466,3 @@ + value += ' volatile' + + else: Are we hitting this case either? This seems weird because we just checked above whether the type was a pointer, and if it's *not* we're returning 'gpointer'? Update: We *are* hitting this case.
Review of attachment 218055 [details] [review]: To be squashed with previous patch.
(In reply to comment #8) > I'd prefer the second and third patches to be squashed to keep the tests > passing for all commits. Done. > It's interesting that the first patch has no effect on the tests. > > We should add a test that uses "volatile" if we're going to support it in the > parser. Probably a structure member? In theory, supporting "volatile" would > require it to be exposed in bindings, so they could cast to (volatile type*) > before doing a read. But in reality there's an immense amount of code between > a binding structure read and the actual access, so it's not like the compiler > is going to be able to e.g. keep the structure member in a register between > reads. I added a test for volatile struct member.(In reply to comment #9) > Review of attachment 218053 [details] [review]: > > ::: giscanner/scannerparser.y > @@ +1089,3 @@ > | '*' type_qualifier_list pointer > { > + GISourceType **base = &($3->base_type); > > I don't understand the double indirection. Couldn't we drop a * and & here? I see that you already pushed the patch to master, but still: we are assigning a new stuff to *base. Also, this code is a sort of copy-pasta of a gi_source_symbol_merge_type().(In reply to comment #10) > Review of attachment 218054 [details] [review]: > > Hmm...kind of unfortuante that we need to have a new complete_ctype with more > data, but I understand why you did it this way. > > ::: giscanner/transformer.py > @@ +439,3 @@ > + def _create_complete_source_type(self, source_type): > + if source_type is None or source_type.type == CTYPE_INVALID: > + return 'None' > > 'None' as a string? I guess you just copied that from _create_source_type() > but it seems broken. We're probably just not hitting that case. > > Update: confirmed. If I change that line to "assert False", everything still > builds and tests pass. Then changed this to assertion. > @@ +466,3 @@ > + value += ' volatile' > + > + else: > > Are we hitting this case either? This seems weird because we just checked > above whether the type was a pointer, and if it's *not* we're returning > 'gpointer'? > > Update: We *are* hitting this case. Patch in next comment.
Created attachment 218223 [details] [review] Write complete C type in c:type attribute and update tests.
Review of attachment 218223 [details] [review]: This looks OK to me.
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]