Type.Name instead of Type.CType #45

sundermann · 2018-03-04T00:33:13Z

No description provided.

It seems like Type.Name is more common in gir. For instance, implements refers to the interface by name and not c:type. This is also handy because we do not have to strip off any trailing const or leading pointers.

Sometimes arrays do not have a name. In this case use Type.Name instead

Object is now resolved as Class

decriptor · 2018-03-04T01:44:51Z

src/Gir/Marshal/SymbolTable.Builtin.cs

@@ -12,11 +12,12 @@ void RegisterBuiltIn (bool nativeWin64)
 			AddType (new StringMarshal ("gfilename", "string", "", "IntPtr"));//, "GLib.Marshaller.StringToFilenamePtr({0})", "GLib.Marshaller.FilenamePtrToStringGFree({0})"));
 			AddType (new StringMarshal ("gchar", "string", "", "IntPtr"));//, "GLib.Marshaller.StringToPtrGStrdup({0})", "GLib.Marshaller.PtrToStringGFree({0})"));
 			AddType (new StringMarshal ("char", "string", "", "IntPtr"));//, "GLib.Marshaller.StringToPtrGStrdup({0})", "GLib.Marshaller.PtrToStringGFree({0})"));
+			AddType (new StringMarshal ("utf8", "string", "", "IntPtr"));//, "GLib.Marshaller.StringToPtrGStrdup({0})", "GLib.Marshaller.PtrToStringGFree({0})"));


I just stubbed this out. I'm not sure what else we want here if anything?

We can probaby remove more primitives as per https://wiki.gnome.org/Projects/GObjectIntrospection/Annotations/#Default_Basic_Types

sundermann · 2018-03-04T02:49:17Z

src/Gir.Tests/SymbolTableTests.cs

@@ -48,12 +48,13 @@ public void NoAliasTypeAfterProcessing ()
 		}

 		[Test]
+		[Ignore("there is no void*")]


This test can probably be removed as there are no pointers inside Type.Name

Lets remove it. Ignored tests seem to cause problems with CI for right now.

sundermann · 2018-03-04T02:51:17Z

src/Gir/GenerationOptions.cs

+			SymbolTable = new SymbolTable(Statistics, options.Win64Longs);
+			SymbolTable.AddTypes (repo.GetSymbols());
+			foreach (var repository in resolvedRepos) {
+				SymbolTable.AddTypes (repository.GetSymbols(), repository);


This includes the main repository so the mainrepository is in the symbol table twice: Once without and once with namespace prefix. This is because arrays seem to behave weird. There are array types inside GLib which have GLib.ByteArray as type where it should have been just ByteArray. That's why I added the repository twice.

sundermann · 2018-03-04T02:51:48Z

src/Gir/Marshal/Array.cs

@@ -5,7 +5,12 @@ public partial class Array
 	{
 		public ISymbol GetSymbol (GenerationOptions opts)
 		{
-			return opts.SymbolTable [CType];
+			if (Name != null) {


Sometimes there is for some reason no Name on arrays. Bug in gi maybe?

byte_array_new_take in GLib-2.0.gir

<array length="1" zero-terminated="0" c:type="guint8*"> <type name="guint8" c:type="guint8"/> </array>

Are you sure you haven't modified the gir locally? It does have it for me.

<function name="byte_array_new_take" c:identifier="g_byte_array_new_take" moved-to="ByteArray.new_take" version="2.32"> <doc xml:space="preserve">Create byte array containing the data. The data will be owned by the array and will be freed with g_free(), i.e. it could be allocated using g_strdup().</doc> <return-value transfer-ownership="full"> <doc xml:space="preserve">a new #GByteArray</doc> <array name="GLib.ByteArray" c:type="GByteArray*"> <type name="guint8" c:type="guint8"/> </array> </return-value> <parameters> <parameter name="data" transfer-ownership="full"> <doc xml:space="preserve">byte data for the array</doc> <array length="1" zero-terminated="0" c:type="guint8*"> <type name="guint8" c:type="guint8"/> </array> </parameter> <parameter name="len" transfer-ownership="none"> <doc xml:space="preserve">length of @data</doc> <type name="gsize" c:type="gsize"/> </parameter> </parameters> </function>

It's not there in the data parameter

Well, for that case, I looked at the code, and in that case, it's just a uint8 pointer:
https://github.com/GNOME/glib/blob/90d3337e1d9d8218f1f8509417b6ae65993aa029/glib/garray.c#L1661

I guess when name isn't there, we need to pass in an array of the element type, it's not a specific gtype

So, when we have a array with no name, it's just an Array ISymbol that has T[] as the managed type

sundermann · 2018-03-04T02:52:18Z

src/Gir/Marshal/SymbolTable.Builtin.cs

@@ -12,11 +12,12 @@ void RegisterBuiltIn (bool nativeWin64)
 			AddType (new StringMarshal ("gfilename", "string", "", "IntPtr"));//, "GLib.Marshaller.StringToFilenamePtr({0})", "GLib.Marshaller.FilenamePtrToStringGFree({0})"));
 			AddType (new StringMarshal ("gchar", "string", "", "IntPtr"));//, "GLib.Marshaller.StringToPtrGStrdup({0})", "GLib.Marshaller.PtrToStringGFree({0})"));
 			AddType (new StringMarshal ("char", "string", "", "IntPtr"));//, "GLib.Marshaller.StringToPtrGStrdup({0})", "GLib.Marshaller.PtrToStringGFree({0})"));
+			AddType (new StringMarshal ("utf8", "string", "", "IntPtr"));//, "GLib.Marshaller.StringToPtrGStrdup({0})", "GLib.Marshaller.PtrToStringGFree({0})"));


Not sure if we need the others. It seems like the string type name is utf8 for gchar and char in gir.

I added char because I ran into it. The others I grabbed because of the old generator.

Yep, if we go down this route, utf8 is the only one we need.

sundermann · 2018-03-04T02:53:19Z

src/Gir/Marshal/SymbolTable.Builtin.cs

@@ -54,6 +55,20 @@ void RegisterPrimitives (bool nativeWin64)

 			AddType (new Primitive ("va_list", "...", ""));

+			var alias = new Alias {


This is kind of a hack. It seems like GType is kind of a primitive since in GObject GType is used without a prefix even though the definition is in GLib.

sundermann · 2018-03-04T02:54:08Z

src/Gir/Marshal/SymbolTable.cs

@@ -72,20 +86,20 @@ static string TrimConstAndPointer (string type)
 			return type.Substring (start, end - start);
 		}

-		public ISymbol this [string type] {
-			get {
-				var actualType = TrimConstAndPointer (type);


TrimConstAndPointer is not used anymore now but I kept the method for now. Do we want to remove it?

Since this is a new project lets be proactive in removing stuff that isn't being used anymore.

Let's remove it.

sundermann · 2018-03-04T02:55:00Z

src/Gir/Marshal/SymbolTable.cs

+
+				if (!typeMap.TryGetValue(toType, out target)) {
+					// HACK?!: this might be in a included repository but these are prefixed in symbol table
+					toType = $"{repository}.{alias.Type.Name}";


This is to correctly dealias symbols in included repositories.

sundermann · 2018-03-04T02:55:13Z

src/Gir/Marshal/Varargs.cs

@@ -5,7 +5,7 @@ public partial class Varargs : ISymbol
 	{
 		public string CSharpType => "...";

-		public string CType => "va_list";
+		public string Name => "va_list";


Not sure about this one

This whole class will be rewritten to support overridden data, so it's fine for now.

Therzok · 2018-03-04T12:14:45Z

src/Gir.Tests/ImportantGeneratables/GObject.cs

@@ -23,7 +23,7 @@ public Object (UIntPtr object_type, string first_property_name, ... var_args) :
 		{
 		}

-		static extern IntPtr g_object_newv (UIntPtr object_type, uint n_parameters, Parameter parameters);
+		static extern Object g_object_newv (UIntPtr object_type, uint n_parameters, Parameter parameters);


This doesn't look right. We don't want the pinvoke signature to use the C# type.

True, but Classes use RetVal.CSharpType right now which points to Object

I think the test changed because Object was not resolved correctly?

Aaaah, yes! I thought we used CType, but we don't 👍

sundermann · 2018-03-04T12:38:53Z

Not sure why jenkins is not building this PR

Therzok · 2018-03-04T12:49:22Z

@directhex ^^

Therzok · 2018-03-04T12:51:58Z

@monojenkins build

Therzok · 2018-03-04T12:52:47Z

whitelist

Therzok · 2018-03-04T12:52:52Z

@monojenkins whitelist

Therzok · 2018-03-04T20:29:14Z

CLA was signed before, see #30

sundermann and others added 9 commits March 3, 2018 23:35

Use Type.Name instead of Type.CType in SymbolTable

adaa8e9

It seems like Type.Name is more common in gir. For instance, implements refers to the interface by name and not c:type. This is also handy because we do not have to strip off any trailing const or leading pointers.

Fix alias resolving

96d6cc3

Fix tests to use Type.Name

d05c95c

Fix dealiasing

82d7229

Fix type lookup

3a16a8a

Void type is none

66c7d60

Fix rebase

d2b5ece

Fix array type

1086f8c

Sometimes arrays do not have a name. In this case use Type.Name instead

Fix GObject test

d8d5b1f

Object is now resolved as Class

sundermann requested review from decriptor and Therzok as code owners March 4, 2018 00:33

decriptor reviewed Mar 4, 2018

View reviewed changes

decriptor previously approved these changes Mar 4, 2018

View reviewed changes

sundermann commented Mar 4, 2018

View reviewed changes

sundermann added 3 commits March 4, 2018 12:48

Remove void* test

1309671

Remove unneeded method

5928037

Rename back to AllRepositories

60b0013

sundermann dismissed decriptor’s stale review via 60b0013 March 4, 2018 11:53

Therzok reviewed Mar 4, 2018

View reviewed changes

Some cleanups and add some code comments

8176ed2

sundermann changed the title ~~Type name rebase~~ Type.Name instead of Type.CType Mar 4, 2018

Therzok merged commit a18b001 into master Mar 4, 2018

Therzok deleted the type-name-rebase branch March 4, 2018 20:29

		@@ -54,6 +55,20 @@ void RegisterPrimitives (bool nativeWin64)

		AddType (new Primitive ("va_list", "...", ""));

		var alias = new Alias {

Type.Name instead of Type.CType #45

Type.Name instead of Type.CType #45

Uh oh!

Conversation

sundermann commented Mar 4, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Therzok Mar 4, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sundermann commented Mar 4, 2018

Uh oh!

Therzok commented Mar 4, 2018

Uh oh!

Therzok commented Mar 4, 2018

Uh oh!

Therzok commented Mar 4, 2018

Uh oh!

Therzok commented Mar 4, 2018

Uh oh!

Therzok commented Mar 4, 2018

Uh oh!

Uh oh!

Therzok Mar 4, 2018 •

edited

Loading