From a11e7dd24497960cabc44607fd948d1303b6021f Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Thu, 30 Jan 2025 08:38:30 -0500 Subject: [PATCH] [Rgen] Add properties in the Property data model to decide if a field is needed. Add some new properties, to be only used by the generator, in the property data model to decide the inclusion of a backing field and if this field needs to be checked. We have moved the ToString method to the .Generator and .Transformer files so that we can reflect the two new properties. The same has happened for the Equals method. --- src/ObjCBindings/ExportTag.cs | 7 ++ .../DataModel/Property.Generator.cs | 77 +++++++++++++++++ .../DataModel/Property.cs | 18 +--- .../DataModel/Property.Transformer.cs | 19 +++++ .../DataModel/PropertyTests.cs | 82 +++++++++++++++++++ .../TestDataFactory.cs | 17 ++-- 6 files changed, 198 insertions(+), 22 deletions(-) diff --git a/src/ObjCBindings/ExportTag.cs b/src/ObjCBindings/ExportTag.cs index 36267814ba7..83b909c6d33 100644 --- a/src/ObjCBindings/ExportTag.cs +++ b/src/ObjCBindings/ExportTag.cs @@ -127,6 +127,13 @@ public enum Property : Int64 { /// IsThreadSafe = 1 << 7, + /// + /// If this falgs is applied to a property, we do not generate a + /// backing field. See bugzilla #3359 and Assistly 7032 for some + /// background information + /// + Transient = 1 << 8, + } } diff --git a/src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs b/src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs index 74f39100914..a18c630d26d 100644 --- a/src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs +++ b/src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs @@ -3,11 +3,14 @@ using System.Collections.Immutable; using System.Diagnostics.CodeAnalysis; +using System.Linq; +using System.Text; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.Macios.Generator.Attributes; using Microsoft.Macios.Generator.Context; using Microsoft.Macios.Generator.Extensions; +using ObjCRuntime; namespace Microsoft.Macios.Generator.DataModel; @@ -50,6 +53,50 @@ public bool IsNotification public bool MarshalNativeExceptions => IsProperty && ExportPropertyData.Value.Flags.HasFlag (ObjCBindings.Property.MarshalNativeExceptions); + /// + /// True if the property should be generated without a backing field. + /// + public bool IsTransient => IsProperty && ExportPropertyData.Value.Flags.HasFlag (ObjCBindings.Property.Transient); + + readonly bool? needsBackingField = null; + /// + /// States if the property, when generated, needs a backing field. + /// + public bool NeedsBackingField { + get { + if (needsBackingField is not null) + return needsBackingField.Value; + var isWrapped = ReturnType.IsWrapped || + ReturnType is { IsArray: true, ArrayElementTypeIsWrapped: true }; + return isWrapped && !IsTransient; + } + // Added to allow testing. This way we can set the correct expectation in the test factory + init => needsBackingField = value; + } + + readonly bool? requiresDirtyCheck = null; + /// + /// States if the property, when generated, should have a dirty check. + /// + public bool RequiresDirtyCheck { + get { + if (requiresDirtyCheck is not null) + return requiresDirtyCheck.Value; + if (!IsProperty) + return false; + switch (ExportPropertyData.Value.ArgumentSemantic) { + case ArgumentSemantic.Copy: + case ArgumentSemantic.Retain: + case ArgumentSemantic.None: + return NeedsBackingField; + default: + return false; + } + } + // Added to allow testing. This way we can set the correct expectation in the test factory + init => requiresDirtyCheck = value; + } + static FieldInfo? GetFieldInfo (RootBindingContext context, IPropertySymbol propertySymbol) { // grab the last port of the namespace @@ -122,4 +169,34 @@ public static bool TryCreate (PropertyDeclarationSyntax declaration, RootBinding }; return true; } + + /// + public bool Equals (Property other) + { + if (!CoreEquals (other)) + return false; + if (IsTransient != other.IsTransient) + return false; + if (NeedsBackingField != other.NeedsBackingField) + return false; + return RequiresDirtyCheck == other.RequiresDirtyCheck; + } + + /// + public override string ToString () + { + var sb = new StringBuilder ( + $"Name: '{Name}', Type: {ReturnType}, Supported Platforms: {SymbolAvailability}, ExportFieldData: '{ExportFieldData?.ToString () ?? "null"}', ExportPropertyData: '{ExportPropertyData?.ToString () ?? "null"}', "); + sb.Append ($"IsTransient: '{IsTransient}', "); + sb.Append ($"NeedsBackingField: '{NeedsBackingField}', "); + sb.Append ($"RequiresDirtyCheck: '{RequiresDirtyCheck}', "); + sb.Append ("Attributes: ["); + sb.AppendJoin (",", Attributes); + sb.Append ("], Modifiers: ["); + sb.AppendJoin (",", Modifiers.Select (x => x.Text)); + sb.Append ("], Accessors: ["); + sb.AppendJoin (",", Accessors); + sb.Append (']'); + return sb.ToString (); + } } diff --git a/src/rgen/Microsoft.Macios.Generator/DataModel/Property.cs b/src/rgen/Microsoft.Macios.Generator/DataModel/Property.cs index 1b8f31ec340..e9799dd974d 100644 --- a/src/rgen/Microsoft.Macios.Generator/DataModel/Property.cs +++ b/src/rgen/Microsoft.Macios.Generator/DataModel/Property.cs @@ -2,9 +2,7 @@ // Licensed under the MIT License. using System; using System.Collections.Immutable; -using System.Linq; using System.Runtime.InteropServices; -using System.Text; using Microsoft.CodeAnalysis; using Microsoft.Macios.Generator.Availability; @@ -96,8 +94,7 @@ internal Property (string name, TypeInfo returnType, Accessors = accessors; } - /// - public bool Equals (Property other) + bool CoreEquals (Property other) { // this could be a large && but ifs are more readable if (Name != other.Name) @@ -151,17 +148,4 @@ public override int GetHashCode () return !left.Equals (right); } - /// - public override string ToString () - { - var sb = new StringBuilder ( - $"Name: '{Name}', Type: {ReturnType}, Supported Platforms: {SymbolAvailability}, ExportFieldData: '{ExportFieldData?.ToString () ?? "null"}', ExportPropertyData: '{ExportPropertyData?.ToString () ?? "null"}' Attributes: ["); - sb.AppendJoin (",", Attributes); - sb.Append ("], Modifiers: ["); - sb.AppendJoin (",", Modifiers.Select (x => x.Text)); - sb.Append ("], Accessors: ["); - sb.AppendJoin (",", Accessors); - sb.Append (']'); - return sb.ToString (); - } } diff --git a/src/rgen/Microsoft.Macios.Transformer/DataModel/Property.Transformer.cs b/src/rgen/Microsoft.Macios.Transformer/DataModel/Property.Transformer.cs index 682ca922622..a876c858c67 100644 --- a/src/rgen/Microsoft.Macios.Transformer/DataModel/Property.Transformer.cs +++ b/src/rgen/Microsoft.Macios.Transformer/DataModel/Property.Transformer.cs @@ -1,7 +1,9 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Collections; using System.Diagnostics.CodeAnalysis; +using System.Text; using Microsoft.Macios.Transformer.Attributes; namespace Microsoft.Macios.Generator.DataModel; @@ -36,4 +38,21 @@ readonly partial struct Property { /// True if the method was exported with the MarshalNativeExceptions flag allowing it to support native exceptions. /// public bool MarshalNativeExceptions => throw new NotImplementedException (); + + /// + public bool Equals (Property other) => Comparer.Equals (this, other); + + /// + public override string ToString () + { + var sb = new StringBuilder ( + $"Name: '{Name}', Type: {ReturnType}, Supported Platforms: {SymbolAvailability}, ExportFieldData: '{ExportFieldData?.ToString () ?? "null"}', ExportPropertyData: '{ExportPropertyData?.ToString () ?? "null"}' Attributes: ["); + sb.AppendJoin (",", Attributes); + sb.Append ("], Modifiers: ["); + sb.AppendJoin (",", Modifiers.Select (x => x.Text)); + sb.Append ("], Accessors: ["); + sb.AppendJoin (",", Accessors); + sb.Append (']'); + return sb.ToString (); + } } diff --git a/tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests.cs b/tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests.cs index 41666d793dc..c8200bd726b 100644 --- a/tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests.cs +++ b/tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests.cs @@ -1248,6 +1248,88 @@ public static partial string Name { ), ]) ]; + + const string nsObjectProperty = @" +using System; +using Foundation; +using ObjCBindings; + +namespace Test; + +public class TestClass { + + [Export(""name"")] + public NSObject Name { get; } +} +"; + yield return [ + nsObjectProperty, + new Property ( + name: "Name", + returnType: ReturnTypeForNSObject (), + symbolAvailability: new (), + attributes: [ + new (name: "ObjCBindings.ExportAttribute", arguments: ["name"]), + ], + modifiers: [ + SyntaxFactory.Token (kind: SyntaxKind.PublicKeyword), + ], + accessors: [ + new ( + accessorKind: AccessorKind.Getter, + symbolAvailability: new (), + exportPropertyData: null, + attributes: [], + modifiers: [] + ) + ] + ) { + NeedsBackingField = true, + RequiresDirtyCheck = true, + ExportPropertyData = new (selector: "name"), + } + ]; + + const string nsObjectArrayProperty = @" +using System; +using Foundation; +using ObjCBindings; + +namespace Test; + +public class TestClass { + + [Export(""name"")] + public NSObject[] Name { get; } +} +"; + yield return [ + nsObjectArrayProperty, + new Property ( + name: "Name", + returnType: ReturnTypeForArray ("Foundation.NSObject"), + symbolAvailability: new (), + attributes: [ + new (name: "ObjCBindings.ExportAttribute", arguments: ["name"]), + ], + modifiers: [ + SyntaxFactory.Token (kind: SyntaxKind.PublicKeyword), + ], + accessors: [ + new ( + accessorKind: AccessorKind.Getter, + symbolAvailability: new (), + exportPropertyData: null, + attributes: [], + modifiers: [] + ) + ] + ) { + NeedsBackingField = true, + RequiresDirtyCheck = true, + ExportPropertyData = new (selector: "name"), + } + ]; } IEnumerator IEnumerable.GetEnumerator () diff --git a/tests/rgen/Microsoft.Macios.Generator.Tests/TestDataFactory.cs b/tests/rgen/Microsoft.Macios.Generator.Tests/TestDataFactory.cs index 965d6981530..8dc85bd8e67 100644 --- a/tests/rgen/Microsoft.Macios.Generator.Tests/TestDataFactory.cs +++ b/tests/rgen/Microsoft.Macios.Generator.Tests/TestDataFactory.cs @@ -300,16 +300,23 @@ public static TypeInfo ReturnTypeForDelegate (string delegateName) ] }; - public static TypeInfo ReturnTypeForNSObject (string nsObjectName, bool isNullable = false) + public static TypeInfo ReturnTypeForNSObject (string? nsObjectName = null, bool isNullable = false) => new ( - name: nsObjectName, + name: nsObjectName ?? "Foundation.NSObject", isNullable: isNullable, - isArray: false + isArray: false, + isReferenceType: true ) { IsNSObject = true, IsINativeObject = true, - Parents = ["Foundation.NSObject", "object"], - Interfaces = ["ObjCRuntime.INativeObject"] + Parents = nsObjectName is null ? ["object"] : ["Foundation.NSObject", "object"], + Interfaces = [ + "ObjCRuntime.INativeObject", + $"System.IEquatable<{nsObjectName ?? "Foundation.NSObject"}>", + "System.IDisposable", + "Foundation.INSObjectFactory", + "Foundation.INSObjectProtocol" + ] }; public static TypeInfo ReturnTypeForINativeObject (string nativeObjectName, bool isNullable = false)