From 50a0bd907a8ad098e4b64c35b9e7dab6dcb864f5 Mon Sep 17 00:00:00 2001 From: badcel <1218031+badcel@users.noreply.github.com> Date: Tue, 28 May 2024 22:19:10 +0200 Subject: [PATCH 1/2] Records: Improve long handling On windows long has always 32 bit. On Unix it has 32 / 64 bit depending on the system architecture. This commit generates structs with a field of type "CLong" / "CULong" instead of "long" / "ulong" to match this behavior. --- .../Renderer/Internal/Field/Converter/Long.cs | 25 +++++ .../Internal/Field/Converter/UnsignedLong.cs | 25 +++++ .../Renderer/Internal/Field/Fields.cs | 2 + .../Renderer/Public/Field/Converter/Long.cs | 36 ++++++ .../Public/Field/Converter/UnsignedLong.cs | 36 ++++++ .../Generator/Renderer/Public/Field/Fields.cs | 2 + .../GirTestLib/girtest-long-record-tester.c | 17 +++ .../GirTestLib/girtest-long-record-tester.h | 17 +++ src/Native/GirTestLib/girtest.h | 1 + src/Native/GirTestLib/meson.build | 2 + .../Libs/GirTest-0.1.Tests/LongRecordTest.cs | 106 ++++++++++++++++++ 11 files changed, 269 insertions(+) create mode 100644 src/Generation/Generator/Renderer/Internal/Field/Converter/Long.cs create mode 100644 src/Generation/Generator/Renderer/Internal/Field/Converter/UnsignedLong.cs create mode 100644 src/Generation/Generator/Renderer/Public/Field/Converter/Long.cs create mode 100644 src/Generation/Generator/Renderer/Public/Field/Converter/UnsignedLong.cs create mode 100644 src/Native/GirTestLib/girtest-long-record-tester.c create mode 100644 src/Native/GirTestLib/girtest-long-record-tester.h create mode 100644 src/Tests/Libs/GirTest-0.1.Tests/LongRecordTest.cs diff --git a/src/Generation/Generator/Renderer/Internal/Field/Converter/Long.cs b/src/Generation/Generator/Renderer/Internal/Field/Converter/Long.cs new file mode 100644 index 000000000..0b70ef722 --- /dev/null +++ b/src/Generation/Generator/Renderer/Internal/Field/Converter/Long.cs @@ -0,0 +1,25 @@ +namespace Generator.Renderer.Internal.Field; + +internal class Long : FieldConverter +{ + public bool Supports(GirModel.Field field) + { + return field.AnyTypeOrCallback.TryPickT0(out var anyType, out _) && anyType.Is(); + } + + public RenderableField Convert(GirModel.Field field) + { + return new RenderableField( + Name: Model.Field.GetName(field), + Attribute: null, + NullableTypeName: GetNullableTypeName(field) + ); + } + + private static string GetNullableTypeName(GirModel.Field field) + { + return field.IsPointer + ? Model.Type.Pointer + : "CLong"; + } +} diff --git a/src/Generation/Generator/Renderer/Internal/Field/Converter/UnsignedLong.cs b/src/Generation/Generator/Renderer/Internal/Field/Converter/UnsignedLong.cs new file mode 100644 index 000000000..7269e71f2 --- /dev/null +++ b/src/Generation/Generator/Renderer/Internal/Field/Converter/UnsignedLong.cs @@ -0,0 +1,25 @@ +namespace Generator.Renderer.Internal.Field; + +internal class UnsignedLong : FieldConverter +{ + public bool Supports(GirModel.Field field) + { + return field.AnyTypeOrCallback.TryPickT0(out var anyType, out _) && anyType.Is(); + } + + public RenderableField Convert(GirModel.Field field) + { + return new RenderableField( + Name: Model.Field.GetName(field), + Attribute: null, + NullableTypeName: GetNullableTypeName(field) + ); + } + + private static string GetNullableTypeName(GirModel.Field field) + { + return field.IsPointer + ? Model.Type.Pointer + : "CULong"; + } +} diff --git a/src/Generation/Generator/Renderer/Internal/Field/Fields.cs b/src/Generation/Generator/Renderer/Internal/Field/Fields.cs index c999f08af..12382baeb 100644 --- a/src/Generation/Generator/Renderer/Internal/Field/Fields.cs +++ b/src/Generation/Generator/Renderer/Internal/Field/Fields.cs @@ -19,6 +19,8 @@ internal static class Fields new Field.Pointer(), new Field.PointerAlias(), new Field.PointerArray(), + new Field.Long(), //Must be before primitive value + new Field.UnsignedLong(), //Must be before primitive value new Field.PrimitiveValueType(), new Field.PrimitiveValueTypeAlias(), new Field.PrimitiveValueTypeArray(), diff --git a/src/Generation/Generator/Renderer/Public/Field/Converter/Long.cs b/src/Generation/Generator/Renderer/Public/Field/Converter/Long.cs new file mode 100644 index 000000000..2c07f6f35 --- /dev/null +++ b/src/Generation/Generator/Renderer/Public/Field/Converter/Long.cs @@ -0,0 +1,36 @@ +namespace Generator.Renderer.Public.Field; + +internal class Long : FieldConverter +{ + public bool Supports(GirModel.Field field) + { + return field.AnyTypeOrCallback.TryPickT0(out var anyType, out _) && anyType.Is(); + } + + public RenderableField Convert(GirModel.Field field) + { + return new RenderableField( + Name: Model.Field.GetName(field), + NullableTypeName: GetNullableTypeName(field), + SetExpression: SetExpression, + GetExpression: GetExpression + ); + } + + private static string GetNullableTypeName(GirModel.Field field) + { + return field.IsPointer + ? Model.Type.Pointer + : Model.Type.GetName(field.AnyTypeOrCallback.AsT0.AsT0); + } + + private static string SetExpression(GirModel.Record record, GirModel.Field field) + { + return $"Handle.Set{Model.Field.GetName(field)}(new CLong((nint)value))"; + } + + private static string GetExpression(GirModel.Record record, GirModel.Field field) + { + return $"Handle.Get{Model.Field.GetName(field)}().Value"; + } +} diff --git a/src/Generation/Generator/Renderer/Public/Field/Converter/UnsignedLong.cs b/src/Generation/Generator/Renderer/Public/Field/Converter/UnsignedLong.cs new file mode 100644 index 000000000..cb6d1e575 --- /dev/null +++ b/src/Generation/Generator/Renderer/Public/Field/Converter/UnsignedLong.cs @@ -0,0 +1,36 @@ +namespace Generator.Renderer.Public.Field; + +internal class UnsignedLong : FieldConverter +{ + public bool Supports(GirModel.Field field) + { + return field.AnyTypeOrCallback.TryPickT0(out var anyType, out _) && anyType.Is(); + } + + public RenderableField Convert(GirModel.Field field) + { + return new RenderableField( + Name: Model.Field.GetName(field), + NullableTypeName: GetNullableTypeName(field), + SetExpression: SetExpression, + GetExpression: GetExpression + ); + } + + private static string GetNullableTypeName(GirModel.Field field) + { + return field.IsPointer + ? Model.Type.Pointer + : Model.Type.GetName(field.AnyTypeOrCallback.AsT0.AsT0); + } + + private static string SetExpression(GirModel.Record record, GirModel.Field field) + { + return $"Handle.Set{Model.Field.GetName(field)}(new CULong((nuint)value))"; + } + + private static string GetExpression(GirModel.Record record, GirModel.Field field) + { + return $"Handle.Get{Model.Field.GetName(field)}().Value"; + } +} diff --git a/src/Generation/Generator/Renderer/Public/Field/Fields.cs b/src/Generation/Generator/Renderer/Public/Field/Fields.cs index 36f32e3ee..af897c8fd 100644 --- a/src/Generation/Generator/Renderer/Public/Field/Fields.cs +++ b/src/Generation/Generator/Renderer/Public/Field/Fields.cs @@ -8,6 +8,8 @@ internal static class Fields { new Field.Bitfield(), new Field.Enumeration(), + new Field.Long(), //Must be before PrimitiveValueType + new Field.UnsignedLong(), //Must be before PrimitiveValueType new Field.PrimitiveValueType(), new Field.String(), }; diff --git a/src/Native/GirTestLib/girtest-long-record-tester.c b/src/Native/GirTestLib/girtest-long-record-tester.c new file mode 100644 index 000000000..7f09cc612 --- /dev/null +++ b/src/Native/GirTestLib/girtest-long-record-tester.c @@ -0,0 +1,17 @@ +#include "girtest-long-record-tester.h" + +/** + * GirTestLongRecordTester: + * + * Test records with long fields + */ + +gsize girtest_long_record_tester_get_sizeof_l(GirTestLongRecordTester* record) +{ + return sizeof(record->l); +} + +gsize girtest_long_record_tester_get_sizeof_ul(GirTestLongRecordTester* record) +{ + return sizeof(record->ul); +} \ No newline at end of file diff --git a/src/Native/GirTestLib/girtest-long-record-tester.h b/src/Native/GirTestLib/girtest-long-record-tester.h new file mode 100644 index 000000000..0c5c187dc --- /dev/null +++ b/src/Native/GirTestLib/girtest-long-record-tester.h @@ -0,0 +1,17 @@ +#pragma once + +#include + +G_BEGIN_DECLS + +typedef struct _GirTestLongRecordTester GirTestLongRecordTester; + +struct _GirTestLongRecordTester +{ + long l; + unsigned long ul; +}; + +gsize girtest_long_record_tester_get_sizeof_l(GirTestLongRecordTester* record); +gsize girtest_long_record_tester_get_sizeof_ul(GirTestLongRecordTester* record); +G_END_DECLS \ No newline at end of file diff --git a/src/Native/GirTestLib/girtest.h b/src/Native/GirTestLib/girtest.h index db10f15c7..c976c6e17 100644 --- a/src/Native/GirTestLib/girtest.h +++ b/src/Native/GirTestLib/girtest.h @@ -10,6 +10,7 @@ #include "girtest-enum-tester.h" #include "girtest-error-tester.h" #include "girtest-integer-array-tester.h" +#include "girtest-long-record-tester.h" #include "girtest-opaque-typed-record-tester.h" #include "girtest-opaque-untyped-record-tester.h" #include "girtest-platform-string-array-null-terminated-tester.h" diff --git a/src/Native/GirTestLib/meson.build b/src/Native/GirTestLib/meson.build index f1c0f5c4e..02894a2d9 100644 --- a/src/Native/GirTestLib/meson.build +++ b/src/Native/GirTestLib/meson.build @@ -13,6 +13,7 @@ header_files = [ 'girtest-enum-tester.h', 'girtest-error-tester.h', 'girtest-integer-array-tester.h', + 'girtest-long-record-tester.h', 'girtest-opaque-typed-record-tester.h', 'girtest-opaque-untyped-record-tester.h', 'girtest-platform-string-array-null-terminated-tester.h', @@ -39,6 +40,7 @@ source_files = [ 'girtest-enum-tester.c', 'girtest-error-tester.c', 'girtest-integer-array-tester.c', + 'girtest-long-record-tester.c', 'girtest-opaque-typed-record-tester.c', 'girtest-opaque-untyped-record-tester.c', 'girtest-platform-string-array-null-terminated-tester.c', diff --git a/src/Tests/Libs/GirTest-0.1.Tests/LongRecordTest.cs b/src/Tests/Libs/GirTest-0.1.Tests/LongRecordTest.cs new file mode 100644 index 000000000..10c20ca52 --- /dev/null +++ b/src/Tests/Libs/GirTest-0.1.Tests/LongRecordTest.cs @@ -0,0 +1,106 @@ +using System; +using System.Runtime.InteropServices; +using FluentAssertions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace GirTest.Tests; + +[TestClass, TestCategory("BindingTest")] +public class LongRecordTest : Test +{ + [TestMethod] + public void ShouldUseCLongCULongInInternalStructData() + { + var data = new GirTest.Internal.LongRecordTesterData(); + data.Ul.Should().BeOfType(); + data.L.Should().BeOfType(); + } + + [TestMethod] + public unsafe void GetSizeOfLShouldBeSizeOfCLong() + { + var sizeOfCLong = sizeof(CLong); + var obj = new LongRecordTester(); + obj.GetSizeofL().Should().Be((nuint) sizeOfCLong); + } + + [DataTestMethod] + [DataRow(long.MaxValue)] + [DataRow(long.MinValue)] + public void ShouldHandleMaxMinLongValue(long value) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || !Environment.Is64BitOperatingSystem) + Assert.Inconclusive("Only supported on 64 Bit Unix operating Systems"); + + var obj = new LongRecordTester { L = value }; + obj.L.Should().Be(value); + } + + [DataTestMethod] + [DataRow(int.MaxValue)] + [DataRow(int.MinValue)] + public void ShouldHandleMaxMinIntValue(int value) + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && Environment.Is64BitOperatingSystem) + Assert.Inconclusive("Only supported on windows or 32 bit unix operating Systems"); + + var obj = new LongRecordTester { L = value }; + obj.L.Should().Be(value); + } + + [DataTestMethod] + [DataRow(long.MaxValue)] + [DataRow(long.MinValue)] + public void ShouldThrowIfValueExceedsIntegerSize(long value) + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && Environment.Is64BitOperatingSystem) + Assert.Inconclusive("Only supported on windows or 32 bit unix operating Systems"); + + var obj = new LongRecordTester(); + var act = () => obj.L = value; + act.Should().Throw(); + } + + [TestMethod] + public unsafe void GetSizeOfULShouldBeSizeOfCULong() + { + var sizeOfCULong = sizeof(CULong); + var obj = new LongRecordTester(); + obj.GetSizeofUl().Should().Be((nuint) sizeOfCULong); + } + + [DataTestMethod] + [DataRow(ulong.MaxValue)] + [DataRow(ulong.MinValue)] + public void ShouldHandleMaxMinULongValue(ulong value) + { + if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows) || !Environment.Is64BitOperatingSystem) + Assert.Inconclusive("Only supported on 64 Bit Unix operating Systems"); + + var obj = new LongRecordTester { Ul = value }; + obj.Ul.Should().Be(value); + } + + [DataTestMethod] + [DataRow(uint.MaxValue)] + [DataRow(uint.MinValue)] + public void ShouldHandleMaxMinUIntValue(uint value) + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && Environment.Is64BitOperatingSystem) + Assert.Inconclusive("Only supported on windows or 32 bit unix operating Systems"); + + var obj = new LongRecordTester { Ul = value }; + obj.Ul.Should().Be(value); + } + + [TestMethod] + public void ShouldThrowIfValueExceedsUnsignedIntegerSize() + { + if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows) && Environment.Is64BitOperatingSystem) + Assert.Inconclusive("Only supported on windows or 32 bit unix operating Systems"); + + var obj = new LongRecordTester(); + var act = () => obj.Ul = ulong.MaxValue; + act.Should().Throw(); + } +} From 31a2e200117d048344efc8fe81e7cb8e09ce1ff3 Mon Sep 17 00:00:00 2001 From: badcel <1218031+badcel@users.noreply.github.com> Date: Tue, 28 May 2024 22:28:47 +0200 Subject: [PATCH 2/2] Untyped record: Remove obsolete unsafe keyword --- .../Generator/Renderer/Internal/UntypedRecordHandle.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Generation/Generator/Renderer/Internal/UntypedRecordHandle.cs b/src/Generation/Generator/Renderer/Internal/UntypedRecordHandle.cs index 7b60df325..8934bcb33 100644 --- a/src/Generation/Generator/Renderer/Internal/UntypedRecordHandle.cs +++ b/src/Generation/Generator/Renderer/Internal/UntypedRecordHandle.cs @@ -255,7 +255,7 @@ private static string RenderFieldGetter(GirModel.Record record, GirModel.Field f var typePrefix = field.AnyTypeOrCallback.IsT1 ? $"{Model.UntypedRecord.GetDataName(record)}." : string.Empty; var dataName = Model.UntypedRecord.GetDataName(record); - return @$"public unsafe {typePrefix}{renderableField.NullableTypeName} Get{renderableField.Name}() + return @$"public {typePrefix}{renderableField.NullableTypeName} Get{renderableField.Name}() {{ if (IsClosed || IsInvalid) throw new InvalidOperationException(""Handle is closed or invalid""); @@ -268,7 +268,7 @@ private static string RenderFieldSetter(GirModel.Record record, GirModel.Field f { var dataName = Model.UntypedRecord.GetDataName(record); - return @$"public unsafe void Set{renderableField.Name}({renderableField.NullableTypeName} value) + return @$"public void Set{renderableField.Name}({renderableField.NullableTypeName} value) {{ if (IsClosed || IsInvalid) throw new InvalidOperationException(""Handle is closed or invalid"");