Skip to content

Commit

Permalink
[Rgen] Boolean properties do not need a temp variable. (#22207)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@mandel-macaque
mandel-macaque authored Feb 20, 2025
1 parent 357d2f7 commit 50be5f0
Show file tree
Hide file tree
Showing 7 changed files with 54 additions and 52 deletions.
3 changes: 1 addition & 2 deletions src/rgen/Microsoft.Macios.Generator/DataModel/Property.Generator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions src/rgen/Microsoft.Macios.Generator/Emitters/BindingSyntaxFactory.Property.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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),
};
Expand Down
24 changes: 8 additions & 16 deletions tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/iOSExpectedPropertyTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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")]
Expand All @@ -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")]
Expand Down
24 changes: 8 additions & 16 deletions tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/macOSExpectedPropertyTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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")]
Expand All @@ -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")]
Expand Down
24 changes: 8 additions & 16 deletions tests/rgen/Microsoft.Macios.Generator.Tests/Classes/Data/tvOSExpectedPropertyTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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")]
Expand All @@ -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;
}
}

Expand Down Expand Up @@ -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")]
Expand All @@ -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")]
Expand Down
2 changes: 1 addition & 1 deletion tests/rgen/Microsoft.Macios.Generator.Tests/DataModel/PropertyTests/NeedsTempReturnTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public class TestClass {
";
yield return [
boolProperty,
true
false
];

const string charProperty = @"
Expand Down
26 changes: 25 additions & 1 deletion tests/rgen/Microsoft.Macios.Generator.Tests/Emitters/BindingSyntaxFactoryPropertyTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ public IEnumerator<object []> 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);",
Expand Down Expand Up @@ -169,6 +168,31 @@ public IEnumerator<object []> 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 ();
Expand Down

7 comments on commit 50be5f0

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

Sign in to view

@vs-mobiletools-engineering-service2
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥 [CI Build #50be5f0] Build failed (CodeQL) 🔥

Build failed for the job 'CodeQL' (with job status 'Failed')

Pipeline on Agent
Hash: 50be5f02498c4c82547393b2ea699476cbd26bda [CI build]

Please sign in to comment.