After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 656445 - const qualifier is not in the .gir
const qualifier is not in the .gir
Status: RESOLVED FIXED
Product: gobject-introspection
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gobject-introspection Maintainer(s)
gobject-introspection Maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2011-08-13 09:26 UTC by Tomeu Vizoso
Modified: 2015-02-07 17:03 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Ignore SLetter gir. Not necessarily relevant to this bug. (644 bytes, patch)
2012-07-04 21:14 UTC, Krzesimir Nowak
committed Details | Review
Fix pointer parsing in scannerparser.y (1.35 KB, patch)
2012-07-04 21:14 UTC, Krzesimir Nowak
committed Details | Review
Write complete C type in c:type attribute. (10.83 KB, patch)
2012-07-04 21:15 UTC, Krzesimir Nowak
reviewed Details | Review
Adapt the tests for detailed c:type. (19.99 KB, patch)
2012-07-04 21:16 UTC, Krzesimir Nowak
reviewed Details | Review
Write complete C type in c:type attribute and update tests. (32.85 KB, patch)
2012-07-07 13:07 UTC, Krzesimir Nowak
committed Details | Review

Description Tomeu Vizoso 2011-08-13 09:26:03 UTC
And is needed to generate C API docs from the .gir
Comment 1 Johan (not receiving bugmail) Dahlin 2011-08-13 12:17:38 UTC
We should put this in another keyword on the type tag.
Comment 2 Johan (not receiving bugmail) Dahlin 2012-02-19 14:56:46 UTC
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.
Comment 3 Krzesimir Nowak 2012-07-04 21:13:22 UTC
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.
Comment 4 Krzesimir Nowak 2012-07-04 21:14:10 UTC
Created attachment 218052 [details] [review]
Ignore SLetter gir. Not necessarily relevant to this bug.
Comment 5 Krzesimir Nowak 2012-07-04 21:14:47 UTC
Created attachment 218053 [details] [review]
Fix pointer parsing in scannerparser.y
Comment 6 Krzesimir Nowak 2012-07-04 21:15:17 UTC
Created attachment 218054 [details] [review]
Write complete C type in c:type attribute.
Comment 7 Krzesimir Nowak 2012-07-04 21:16:03 UTC
Created attachment 218055 [details] [review]
Adapt the tests for detailed c:type.
Comment 8 Colin Walters 2012-07-06 15:04:56 UTC
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.
Comment 9 Colin Walters 2012-07-06 15:07:46 UTC
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?
Comment 10 Colin Walters 2012-07-06 15:18:15 UTC
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.
Comment 11 Colin Walters 2012-07-06 15:18:31 UTC
Review of attachment 218055 [details] [review]:

To be squashed with previous patch.
Comment 12 Krzesimir Nowak 2012-07-07 13:06:24 UTC
(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.
Comment 13 Krzesimir Nowak 2012-07-07 13:07:16 UTC
Created attachment 218223 [details] [review]
Write complete C type in c:type attribute and update tests.
Comment 14 Colin Walters 2012-07-07 21:13:58 UTC
Review of attachment 218223 [details] [review]:

This looks OK to me.
Comment 15 André Klapper 2015-02-07 17:03:05 UTC
[Mass-moving gobject-introspection tickets to its own Bugzilla product - see bug 708029. Mass-filter your bugmail for this message: introspection20150207 ]