From 50be5f02498c4c82547393b2ea699476cbd26bda Mon Sep 17 00:00:00 2001 From: Manuel de la Pena Date: Wed, 19 Feb 2025 20:41:32 -0500 Subject: [PATCH] [Rgen] Boolean properties do not need a temp variable. (#22207) The use of a temp variable in boolean properties was due to a limitation of how bgen wrote the code. We copied the behaviour to make sure we were compatible, yet that was not needed. This change adds supports to boolean getters in properties without the need of a temp var. --- .../DataModel/Property.Generator.cs | 3 +-- .../Emitters/BindingSyntaxFactory.Property.cs | 3 +++ .../Classes/Data/iOSExpectedPropertyTests.cs | 24 ++++++----------- .../Data/macOSExpectedPropertyTests.cs | 24 ++++++----------- .../Classes/Data/tvOSExpectedPropertyTests.cs | 24 ++++++----------- .../PropertyTests/NeedsTempReturnTests.cs | 2 +- .../BindingSyntaxFactoryPropertyTests.cs | 26 ++++++++++++++++++- 7 files changed, 54 insertions(+), 52 deletions(-) diff --git a/src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs b/src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs index 2a03f895ac9..54457e27708 100644 --- a/src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs +++ b/src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs @@ -120,8 +120,7 @@ public bool UseTempReturn { { ReturnType: { IsVoid: false, NeedsStret: true } } => true, { ReturnType: { IsVoid: false, IsWrapped: true } } => true, { ReturnType.IsNativeEnum: true } => true, - { ReturnType.SpecialType: SpecialType.System_Boolean - or SpecialType.System_Char or SpecialType.System_Delegate } => true, + { ReturnType.SpecialType: SpecialType.System_Char or SpecialType.System_Delegate } => true, { ReturnType.IsDelegate: true } => true, { ReturnType.IsWrapped: true } => true, // default will be false diff --git a/src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Property.cs b/src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Property.cs index 10a7b593af1..71ed5072749 100644 --- a/src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Property.cs +++ b/src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Property.cs @@ -88,6 +88,9 @@ ExpressionStatementSyntax GetterInvocation (InvocationExpressionSyntax objMsgSen { SpecialType: SpecialType.System_String, IsNullable: false } => ExpressionStatement (SuppressNullableWarning (StringFromHandle ([Argument (objMsgSend), BoolArgument (false)]))), + // bool => global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("canDraw")) != 0; + { SpecialType: SpecialType.System_Boolean } => ExpressionStatement ( ByteToBool (objMsgSend)), + // general case, just return the result of the send message _ => ExpressionStatement (objMsgSend), }; diff --git a/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/iOSExpectedPropertyTests.cs b/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/iOSExpectedPropertyTests.cs index b4d63d87018..f99c4aded4d 100644 --- a/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/iOSExpectedPropertyTests.cs +++ b/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/iOSExpectedPropertyTests.cs @@ -195,13 +195,11 @@ public virtual partial bool CanDraw [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("canDraw")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("canDraw")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] @@ -227,13 +225,11 @@ public virtual partial bool ContainsAttachments [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("containsAttachments")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("containsAttachments")) != 0; } - return ret; } } @@ -271,13 +267,11 @@ public virtual partial bool ForPersonMassUse [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("isForPersonMassUse")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("isForPersonMassUse")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] @@ -303,13 +297,11 @@ public virtual partial bool IsLenient [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("isLenient")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("isLenient")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] diff --git a/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/macOSExpectedPropertyTests.cs b/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/macOSExpectedPropertyTests.cs index d67fa6f5d13..728c685166c 100644 --- a/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/macOSExpectedPropertyTests.cs +++ b/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/macOSExpectedPropertyTests.cs @@ -195,13 +195,11 @@ public virtual partial bool CanDraw [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("canDraw")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("canDraw")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] @@ -227,13 +225,11 @@ public virtual partial bool ContainsAttachments [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("containsAttachments")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("containsAttachments")) != 0; } - return ret; } } @@ -271,13 +267,11 @@ public virtual partial bool ForPersonMassUse [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("isForPersonMassUse")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("isForPersonMassUse")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] @@ -303,13 +297,11 @@ public virtual partial bool IsLenient [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("isLenient")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("isLenient")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] diff --git a/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/tvOSExpectedPropertyTests.cs b/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/tvOSExpectedPropertyTests.cs index d67fa6f5d13..728c685166c 100644 --- a/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/tvOSExpectedPropertyTests.cs +++ b/tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/tvOSExpectedPropertyTests.cs @@ -195,13 +195,11 @@ public virtual partial bool CanDraw [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("canDraw")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("canDraw")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] @@ -227,13 +225,11 @@ public virtual partial bool ContainsAttachments [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("containsAttachments")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("containsAttachments")) != 0; } - return ret; } } @@ -271,13 +267,11 @@ public virtual partial bool ForPersonMassUse [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("isForPersonMassUse")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("isForPersonMassUse")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] @@ -303,13 +297,11 @@ public virtual partial bool IsLenient [SupportedOSPlatform ("maccatalyst13.1")] get { - bool ret; if (IsDirectBinding) { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle ("isLenient")) != 0; } else { - ret = throw new NotImplementedException(); + return global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle ("isLenient")) != 0; } - return ret; } [SupportedOSPlatform ("macos")] diff --git a/tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests/NeedsTempReturnTests.cs b/tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests/NeedsTempReturnTests.cs index 24fd41b5912..5e45a913171 100644 --- a/tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests/NeedsTempReturnTests.cs +++ b/tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests/NeedsTempReturnTests.cs @@ -61,7 +61,7 @@ public class TestClass { "; yield return [ boolProperty, - true + false ]; const string charProperty = @" diff --git a/tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryPropertyTests.cs b/tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryPropertyTests.cs index 42ed947fbd8..3f332becc74 100644 --- a/tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryPropertyTests.cs +++ b/tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryPropertyTests.cs @@ -63,7 +63,6 @@ public IEnumerator GetEnumerator () ExportPropertyData = new ("myProperty", ArgumentSemantic.None, ObjCBindings.Property.IsThreadSafe) }; - yield return [ property, "CFString.FromHandle (global::ObjCRuntime.Messaging.NativeHandle_objc_msgSend (this.Handle, Selector.GetHandle (\"myProperty\")), false);", @@ -169,6 +168,31 @@ public IEnumerator GetEnumerator () "global::ObjCRuntime.Messaging.uint_objc_msgSend (this.Handle, Selector.GetHandle (\"myProperty\"));", "global::ObjCRuntime.Messaging.uint_objc_msgSendSuper (this.Handle, Selector.GetHandle (\"myProperty\"));" ]; + + property = new Property ( + name: "MyProperty", + returnType: ReturnTypeForBool (), + symbolAvailability: new (), + attributes: [], + modifiers: [], + accessors: [ + new ( + accessorKind: AccessorKind.Getter, + symbolAvailability: new (), + exportPropertyData: null, + attributes: [], + modifiers: [] + ) + ] + ) { + ExportPropertyData = new ("myProperty", ArgumentSemantic.None, ObjCBindings.Property.IsThreadSafe) + }; + + yield return [ + property, + "global::ObjCRuntime.Messaging.bool_objc_msgSend (this.Handle, Selector.GetHandle (\"myProperty\")) != 0;", + "global::ObjCRuntime.Messaging.bool_objc_msgSendSuper (this.Handle, Selector.GetHandle (\"myProperty\")) != 0;" + ]; } IEnumerator IEnumerable.GetEnumerator () => GetEnumerator ();