GNOME.org
 

[gobject-introspection/ebassi/split-disguised: 1/2] Split disguised attribute into two


commit dce9b3d8810225fe8b4771fd5e305c73b5be74e7
Author: Emmanuele Bassi <ebassi gnome org>
Date:   Wed Jul 13 12:45:57 2022 +0100

    Split disguised attribute into two
    
    The disguised attribute started off as a way to indicate a typedef to a
    structure pointer, e.g.
    
        typedef struct Foo* FooPtr;
    
    Over the years, though, it started to include opaque structure types,
    e.g.
    
        typedef struct _FooObject FooObject;
        typedef struct _FooObjectClass FooObjectClass;
    
    This has led to issues in language bindings, code generators, and
    documentation generators, which now have issues when dealing with both
    pointer aliases and opaque types.
    
    An initial attempt at fixing this mess in commit f606183a ended up
    breaking Vala, and had to be reverted.
    
    To avoid breaking existing users we can follow a similar approach to the
    allow-none/nullable/optional solution:
    
    1. introduce a new pair of attributes: "pointer" and "opaque"
    2. deprecate the "disguised" attribute
    
    The "pointer" attribute covers the case of pointer types.
    
    The "opaque" attribute covers the case of opaque structured types.
    
    See also: https://gitlab.gnome.org/GNOME/vala/-/issues/735
    
    Fixes: #101

 docs/gir-1.2.rnc                            | 11 +++++++++--
 giscanner/ast.py                            | 12 ++++++++++++
 giscanner/gdumpparser.py                    |  8 ++++++--
 giscanner/girparser.py                      |  2 ++
 giscanner/girwriter.py                      |  4 ++++
 giscanner/transformer.py                    | 27 +++++++++++++++++----------
 tests/scanner/Identfilter-1.0-expected.gir  |  7 +++++--
 tests/scanner/Regress-1.0-expected.gir      | 13 ++++++++++---
 tests/scanner/Symbolfilter-1.0-expected.gir |  2 +-
 tests/scanner/Typedefs-1.0-expected.gir     |  1 +
 tests/scanner/test_transformer.py           | 26 ++++++++++++++++++++++++++
 11 files changed, 93 insertions(+), 20 deletions(-)
---
diff --git a/docs/gir-1.2.rnc b/docs/gir-1.2.rnc
index 10c6927c..4b28a8e1 100644
--- a/docs/gir-1.2.rnc
+++ b/docs/gir-1.2.rnc
@@ -215,9 +215,16 @@ grammar {
       attribute name { xsd:string },
       ## Corresponding C type of the record
       attribute c:type { xsd:string }?,
-      ## Binary attribute to tell if the record is disguised, i.e. whether the c:type is a typedef that 
doesn't look like a pointer, but is one internally
-         ## Its second meaning is "private" and is set when any typedef struct is parsed which doesn't also 
include a full struct with fields (https://gitlab.gnome.org/GNOME/gobject-introspection/issues/101)
+      ## Deprecated. Binary attribute to tell if the record is disguised, i.e. whether the c:type
+      ## is a typedef that doesn't look like a pointer, but is one internally. Its second meaning
+      ## is "private" and is set when any typedef struct is parsed which doesn't also include a
+      ## full struct with fields (https://gitlab.gnome.org/GNOME/gobject-introspection/issues/101)
+      ## Replaced by "opaque" and "pointer".
       attribute disguised { "0" | "1" }?,
+      ## Binary attribute for a typedef struct that does not have a corresponding public structure definition
+      attribute opaque { "0" | "1" }?,
+      ## Binary attribute for a typedef struct pointer, e.g. typedef struct Foo* FooPtr
+      attribute pointer { "0" | "1" }?,
       ## GObject compatible C type of the record
       attribute glib:type-name { xsd:string }?,
       ## Function to get the GObject compatible type of the record
diff --git a/giscanner/ast.py b/giscanner/ast.py
index 2cfd81fc..d3e7d373 100644
--- a/giscanner/ast.py
+++ b/giscanner/ast.py
@@ -1021,6 +1021,8 @@ class Compound(Node, Registered):
                  get_type=None,
                  c_symbol_prefix=None,
                  disguised=False,
+                 opaque=False,
+                 pointer=False,
                  tag_name=None):
         Node.__init__(self, name)
         Registered.__init__(self, gtype_name, get_type)
@@ -1030,6 +1032,8 @@ class Compound(Node, Registered):
         self.fields = []
         self.constructors = []
         self.disguised = disguised
+        self.opaque = opaque
+        self.pointer = pointer
         self.gtype_name = gtype_name
         self.get_type = get_type
         self.c_symbol_prefix = c_symbol_prefix
@@ -1116,6 +1120,8 @@ class Record(Compound):
                  get_type=None,
                  c_symbol_prefix=None,
                  disguised=False,
+                 opaque=False,
+                 pointer=False,
                  tag_name=None):
         Compound.__init__(self, name,
                           ctype=ctype,
@@ -1123,6 +1129,8 @@ class Record(Compound):
                           get_type=get_type,
                           c_symbol_prefix=c_symbol_prefix,
                           disguised=disguised,
+                          opaque=opaque,
+                          pointer=pointer,
                           tag_name=tag_name)
         # If non-None, this record defines the FooClass C structure
         # for some Foo GObject (or similar for GInterface)
@@ -1137,6 +1145,8 @@ class Union(Compound):
                  get_type=None,
                  c_symbol_prefix=None,
                  disguised=False,
+                 opaque=False,
+                 pointer=False,
                  tag_name=None):
         Compound.__init__(self, name,
                           ctype=ctype,
@@ -1144,6 +1154,8 @@ class Union(Compound):
                           get_type=get_type,
                           c_symbol_prefix=c_symbol_prefix,
                           disguised=disguised,
+                          opaque=opaque,
+                          pointer=pointer,
                           tag_name=tag_name)
 
 
diff --git a/giscanner/gdumpparser.py b/giscanner/gdumpparser.py
index 1a0794d4..15c8b94d 100644
--- a/giscanner/gdumpparser.py
+++ b/giscanner/gdumpparser.py
@@ -229,7 +229,10 @@ blob containing data gleaned from GObject's primitive introspection."""
                                                       get_type='intern',
                                                       c_symbol_prefix='variant')
         elif record.name == 'InitiallyUnownedClass':
+            # InitiallyUnowned is just GObject with extra steps, so we alias
+            # it in the introspection data
             record.fields = self._namespace.get('ObjectClass').fields
+            record.opaque = False
             record.disguised = False
 
     # Introspection over the data we get from the dynamic
@@ -519,8 +522,9 @@ different --identifier-prefix.""" % (xmlnode.attrib['name'], self._namespace.ide
             pair_node.add_gtype(boxed.gtype_name, boxed.get_type)
             assert boxed.c_symbol_prefix is not None
             pair_node.c_symbol_prefix = boxed.c_symbol_prefix
-            # Quick hack - reset the disguised flag; we're setting it
-            # incorrectly in the scanner
+            # Backward compatibility hack - reset the disguised flag; we're
+            # setting it incorrectly in the scanner. We don't change the
+            # opaque flag, because it's a new one and we define its behavior
             pair_node.disguised = False
         else:
             return False
diff --git a/giscanner/girparser.py b/giscanner/girparser.py
index edaaa992..f78b9ef3 100644
--- a/giscanner/girparser.py
+++ b/giscanner/girparser.py
@@ -435,6 +435,8 @@ class GIRParser(object):
         compound = cls(node.attrib.get('name'),
                        ctype=node.attrib.get(_cns('type')),
                        disguised=node.attrib.get('disguised') == '1',
+                       opaque=node.attrib.get('opaque') == '1',
+                       pointer=node.attrib.get('pointer') == '1',
                        gtype_name=node.attrib.get(_glibns('type-name')),
                        get_type=node.attrib.get(_glibns('get-type')),
                        c_symbol_prefix=node.attrib.get(_cns('symbol-prefix')))
diff --git a/giscanner/girwriter.py b/giscanner/girwriter.py
index 9ff10d31..90757d42 100644
--- a/giscanner/girwriter.py
+++ b/giscanner/girwriter.py
@@ -585,6 +585,10 @@ class GIRWriter(XMLWriter):
             attrs.append(('c:type', record.ctype))
         if record.disguised:
             attrs.append(('disguised', '1'))
+        if record.opaque:
+            attrs.append(('opaque', '1'))
+        if record.pointer:
+            attrs.append(('pointer', '1'))
         if record.foreign:
             attrs.append(('foreign', '1'))
         if record.is_gtype_struct_for is not None:
diff --git a/giscanner/transformer.py b/giscanner/transformer.py
index ad2a87f4..3b0bea8a 100644
--- a/giscanner/transformer.py
+++ b/giscanner/transformer.py
@@ -613,7 +613,7 @@ raise ValueError."""
         elif (ctype == CTYPE_FUNCTION):
             node = self._create_typedef_callback(symbol)
         elif (ctype == CTYPE_POINTER and symbol.base_type.base_type.type == CTYPE_STRUCT):
-            node = self._create_typedef_compound(ast.Record, symbol, disguised=True)
+            node = self._create_typedef_compound(ast.Record, symbol, disguised=True, pointer=True)
         elif ctype == CTYPE_STRUCT:
             node = self._create_typedef_compound(ast.Record, symbol)
         elif ctype == CTYPE_UNION:
@@ -801,7 +801,7 @@ raise ValueError."""
         const.add_symbol_reference(symbol)
         return const
 
-    def _create_typedef_compound(self, compound_class, symbol, disguised=False):
+    def _create_typedef_compound(self, compound_class, symbol, disguised=False, pointer=False):
         name = self.strip_identifier(symbol.ident)
         assert symbol.base_type
         if symbol.base_type.name:
@@ -840,12 +840,14 @@ raise ValueError."""
             # Structs with a typedef name are promoted into the main namespace
             # by it being returned to the "parse" function and are also added to
             # the tag namespace if it has a tag_name set.
-            compound = compound_class(name, symbol.ident, disguised=disguised, tag_name=tag_name)
+            compound = compound_class(name, symbol.ident, disguised=disguised, pointer=pointer, 
tag_name=tag_name)
             if tag_name:
-                # Force the struct as disguised for now since we do not yet know
-                # if it has fields that will be parsed. Note that this is using
-                # an erroneous definition of disguised and we should eventually
-                # only look at the field count when needed.
+                # Force the struct as opaque for now since we do not yet know
+                # if it has fields that will be parsed.
+                compound.opaque = True
+                # Set disguised as well, for backward compatibility; the "disguised" field
+                # is used incorrectly, here, but it's too late for us to change this. See
+                # https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/101
                 compound.disguised = True
             else:
                 # Case where we have an anonymous struct which is typedef'd:
@@ -864,12 +866,17 @@ raise ValueError."""
         else:
             compound = compound_class(None, symbol.ident, tag_name=symbol.ident)
 
-        # Make sure disguised is False as we are now about to parse the
-        # fields of the real struct.
-        compound.disguised = False
         # Fields may need to be parsed in either of the above cases because the
         # Record can be created with a typedef prior to the struct definition.
         self._parse_fields(symbol, compound)
+
+        # A compound type with no fields is opaque
+        compound.opaque = len(compound.fields) == 0
+        # Unconditionally set disguised to false, for backward compatibility;
+        # the "disguised" field is used incorrectly, here, but it's too late for
+        # us to change this. See:
+        #   https://gitlab.gnome.org/GNOME/gobject-introspection/-/issues/101
+        compound.disguised = False
         compound.add_symbol_reference(symbol)
         return compound
 
diff --git a/tests/scanner/Identfilter-1.0-expected.gir b/tests/scanner/Identfilter-1.0-expected.gir
index 15cd408a..05af2a56 100644
--- a/tests/scanner/Identfilter-1.0-expected.gir
+++ b/tests/scanner/Identfilter-1.0-expected.gir
@@ -11,10 +11,13 @@ and/or use gtk-doc annotations.  -->
              shared-library=""
              c:identifier-prefixes="Identfilter"
              c:symbol-prefixes="identfilter">
-    <record name="Context" c:type="identfilter_t" disguised="1">
+    <record name="Context" c:type="identfilter_t" disguised="1" opaque="1">
       <source-position filename="identfilter.h" line="4"/>
     </record>
-    <record name="Object" c:type="identfilter_object_t" disguised="1">
+    <record name="Object"
+            c:type="identfilter_object_t"
+            disguised="1"
+            opaque="1">
       <source-position filename="identfilter.h" line="5"/>
       <method name="foo_method" c:identifier="identfilter_object_foo_method">
         <source-position filename="identfilter.h" line="8"/>
diff --git a/tests/scanner/Regress-1.0-expected.gir b/tests/scanner/Regress-1.0-expected.gir
index 99d68271..7ecc1400 100644
--- a/tests/scanner/Regress-1.0-expected.gir
+++ b/tests/scanner/Regress-1.0-expected.gir
@@ -1503,6 +1503,7 @@ it says it's pointer but it's actually a string.</doc>
     </union>
     <record name="FooBoxed"
             c:type="RegressFooBoxed"
+            opaque="1"
             glib:type-name="RegressFooBoxed"
             glib:get-type="regress_foo_boxed_get_type"
             c:symbol-prefix="foo_boxed">
@@ -1549,6 +1550,7 @@ it says it's pointer but it's actually a string.</doc>
     <record name="FooBufferClass"
             c:type="RegressFooBufferClass"
             disguised="1"
+            opaque="1"
             glib:is-gtype-struct-for="FooBuffer">
       <source-position filename="foo.h" line="52"/>
     </record>
@@ -1574,6 +1576,7 @@ it says it's pointer but it's actually a string.</doc>
     </callback>
     <record name="FooDBusData"
             c:type="RegressFooDBusData"
+            opaque="1"
             glib:type-name="RegressFooDBusData"
             glib:get-type="regress_foo_dbus_data_get_type"
             c:symbol-prefix="foo_dbus_data">
@@ -2243,6 +2246,7 @@ uses a C sugar return type.</doc>
     <record name="FooOtherObjectClass"
             c:type="RegressFooOtherObjectClass"
             disguised="1"
+            opaque="1"
             glib:is-gtype-struct-for="FooOtherObject">
       <source-position filename="foo.h" line="54"/>
     </record>
@@ -2369,7 +2373,8 @@ exposed to language bindings.</doc>
     </record>
     <record name="FooStructPrivate"
             c:type="RegressFooStructPrivate"
-            disguised="1">
+            disguised="1"
+            opaque="1">
       <source-position filename="foo.h" line="325"/>
     </record>
     <interface name="FooSubInterface"
@@ -2633,7 +2638,7 @@ exposed to language bindings.</doc>
       <source-position filename="regress.h" line="520"/>
       <type name="gint" c:type="gint"/>
     </constant>
-    <record name="Intset" c:type="RegressIntset" disguised="1">
+    <record name="Intset" c:type="RegressIntset" disguised="1" opaque="1">
       <doc xml:space="preserve"
            filename="regress.h"
            line="1406">Like telepathy-glib's TpIntset.</doc>
@@ -3122,6 +3127,7 @@ use it should be.</doc>
     </record>
     <record name="TestBoxedD"
             c:type="RegressTestBoxedD"
+            opaque="1"
             glib:type-name="RegressTestBoxedD"
             glib:get-type="regress_test_boxed_d_get_type"
             c:symbol-prefix="test_boxed_d">
@@ -3176,7 +3182,8 @@ use it should be.</doc>
     </record>
     <record name="TestBoxedPrivate"
             c:type="RegressTestBoxedPrivate"
-            disguised="1">
+            disguised="1"
+            opaque="1">
       <source-position filename="regress.h" line="666"/>
     </record>
     <callback name="TestCallback" c:type="RegressTestCallback">
diff --git a/tests/scanner/Symbolfilter-1.0-expected.gir b/tests/scanner/Symbolfilter-1.0-expected.gir
index 80fe0d4b..176b4f07 100644
--- a/tests/scanner/Symbolfilter-1.0-expected.gir
+++ b/tests/scanner/Symbolfilter-1.0-expected.gir
@@ -11,7 +11,7 @@ and/or use gtk-doc annotations.  -->
              shared-library=""
              c:identifier-prefixes="Symbolfilter"
              c:symbol-prefixes="symbolfilter">
-    <record name="Object" c:type="SymbolfilterObject" disguised="1">
+    <record name="Object" c:type="SymbolfilterObject" disguised="1" opaque="1">
       <source-position filename="symbolfilter.h" line="4"/>
       <method name="filterObjectFooMethod"
               c:identifier="SymbolfilterObjectFooMethod">
diff --git a/tests/scanner/Typedefs-1.0-expected.gir b/tests/scanner/Typedefs-1.0-expected.gir
index 05df44b9..94a50e7b 100644
--- a/tests/scanner/Typedefs-1.0-expected.gir
+++ b/tests/scanner/Typedefs-1.0-expected.gir
@@ -26,6 +26,7 @@ and/or use gtk-doc annotations.  -->
     </record>
     <record name="BoxedWithHiddenStruct"
             c:type="TypedefsBoxedWithHiddenStruct"
+            opaque="1"
             glib:type-name="TypedefsBoxedWithHiddenStruct"
             glib:get-type="typedefs_boxed_with_hidden_struct_get_type"
             c:symbol-prefix="boxed_with_hidden_struct">
diff --git a/tests/scanner/test_transformer.py b/tests/scanner/test_transformer.py
index 3feed441..559d5789 100644
--- a/tests/scanner/test_transformer.py
+++ b/tests/scanner/test_transformer.py
@@ -97,6 +97,7 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
         self.assertEqual(len(node.fields), 1)
 
@@ -111,6 +112,7 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
         self.assertEqual(len(node.fields), 1)
 
@@ -125,6 +127,7 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
         self.assertEqual(len(node.fields), 1)
 
@@ -138,6 +141,7 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
         self.assertEqual(len(node.fields), 1)
 
@@ -151,6 +155,7 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
         self.assertEqual(len(node.fields), 1)
 
@@ -167,6 +172,7 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
         self.assertEqual(len(node.fields), 1)
 
@@ -194,6 +200,7 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
         self.assertEqual(len(node.fields), 1)
         self.assertEqual(node.ctype, 'TestStruct')
@@ -218,6 +225,7 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
         self.assertEqual(len(node.fields), 1)
         self.assertEqual(node.ctype, 'TestStruct')
@@ -262,7 +270,9 @@ class TestStructTypedefs(unittest.TestCase):
         node = self.namespace.get('Struct')
         self.assertTrue(node is not None)
         self.assertTrue(isinstance(node, ast.Record))
+        self.assertFalse(node.opaque)
         self.assertFalse(node.disguised)
+        self.assertFalse(node.pointer)
         self.assertEqual(node.ctype, 'TestStruct')
         self.assertEqual(len(node.fields), 1)
 
@@ -270,10 +280,26 @@ class TestStructTypedefs(unittest.TestCase):
         self.assertTrue(ptr is not None)
         # This currently gives a disguised Record instead of an Alias
         self.assertTrue(isinstance(ptr, ast.Record))
+        self.assertTrue(ptr.pointer)
         self.assertTrue(ptr.disguised)
+        self.assertFalse(ptr.opaque)
         self.assertEqual(len(ptr.fields), 0)
         self.assertEqual(ptr.ctype, 'TestStructPtr')
 
+    def test_opaque_struct(self):
+        load_namespace_from_source_string(self.namespace, """
+            typedef struct _TestStruct TestStruct;
+            """)
+        self.assertEqual(len(self.namespace.names), 1)
+
+        node = self.namespace.get('Struct')
+        self.assertTrue(node is not None)
+        self.assertTrue(isinstance(node, ast.Record))
+        self.assertTrue(node.opaque)
+        self.assertTrue(node.disguised)
+        self.assertEqual(node.ctype, 'TestStruct')
+        self.assertEqual(len(node.fields), 0)
+
 
 class TestNestedStructs(unittest.TestCase):
     def setUp(self):
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]