Skip to content

Commit

Permalink
[ObjCRuntime] Refactor the Block.SetupBlock(Delegate,Delegate) implem…
Browse files Browse the repository at this point in the history
…entation a bit. (#20353)

* Extract code the trimmer wants to warn about into a separate method.
* Suppress the trimmer warning we get.

Contributes towards #10405.
  • Loading branch information
rolfbjarne authored Mar 22, 2024
1 parent 26d2405 commit 24f1054
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 23 deletions.
13 changes: 12 additions & 1 deletion src/ObjCRuntime/Blocks.cs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,17 @@ public void SetupBlock (Delegate trampoline, Delegate userDelegate)
if (trampoline is null)
ObjCRuntime.ThrowHelper.ThrowArgumentNullException (nameof (trampoline));

VerifyBlockDelegates (trampoline, userDelegate);

SetupBlock (trampoline, userDelegate, safe: true);
}

#if NET
// IL2075: 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String)'. The return value of method 'ObjCRuntime.MonoPInvokeCallbackAttribute.DelegateType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to.
[UnconditionalSuppressMessage ("", "IL2075", Justification = "Calling GetMethod('Invoke') on a delegate type will always find something, because the invoke method can't be linked away for a delegate.")]
#endif
void VerifyBlockDelegates (Delegate trampoline, Delegate userDelegate)
{
#if !MONOMAC && !__MACCATALYST__
// Check that:
// * The trampoline is static
Expand Down Expand Up @@ -338,7 +349,7 @@ public void SetupBlock (Delegate trampoline, Delegate userDelegate)
}
}
#endif
SetupBlock (trampoline, userDelegate, safe: true);

}

public void CleanupBlock ()
Expand Down
23 changes: 1 addition & 22 deletions tests/dotnet/UnitTests/TrimmerWarningsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,28 +28,7 @@ public void TrimmerWarningsStaticRegistrar (ApplePlatform platform, string runti
[TestCase (ApplePlatform.TVOS, "tvos-arm64")]
public void TrimmerWarningsDynamicRegistrar (ApplePlatform platform, string runtimeIdentifiers)
{
ExpectedBuildMessage [] expectedWarnings;
switch (platform) {
case ApplePlatform.iOS:
case ApplePlatform.TVOS:
expectedWarnings = new ExpectedBuildMessage [] {
new ExpectedBuildMessage ("src/ObjCRuntime/Blocks.cs" /* line 313 */, "ObjCRuntime.BlockLiteral.SetupBlock(Delegate, Delegate): 'this' argument does not satisfy 'DynamicallyAccessedMemberTypes.PublicMethods' in call to 'System.Type.GetMethod(String)'. The return value of method 'ObjCRuntime.MonoPInvokeCallbackAttribute.DelegateType.get' does not have matching annotations. The source value must declare at least the same requirements as those declared on the target location it is assigned to."),
};
break;
case ApplePlatform.MacOSX:
expectedWarnings = new ExpectedBuildMessage [] {
};
break;
case ApplePlatform.MacCatalyst:
expectedWarnings = new ExpectedBuildMessage [] {
};
break;
default:
Assert.Fail ($"Unknown platform: {platform}");
return;
}

TrimmerWarnings (platform, runtimeIdentifiers, "dynamic", expectedWarnings);
TrimmerWarnings (platform, runtimeIdentifiers, "dynamic", Array.Empty<ExpectedBuildMessage> ());
}

void TrimmerWarnings (ApplePlatform platform, string runtimeIdentifiers, string registrar, params ExpectedBuildMessage [] expectedWarnings)
Expand Down

8 comments on commit 24f1054

@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] Tests on macOS X64 - Mac Sonoma (14) passed 💻

All tests on macOS X64 - Mac Sonoma (14) passed.

Pipeline on Agent
Hash: [CI build]

@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] Tests on macOS M1 - Mac Monterey (12) passed 💻

All tests on macOS M1 - Mac Monterey (12) passed.

Pipeline on Agent
Hash: [CI build]

@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] Tests on macOS M1 - Mac Ventura (13) passed 💻

All tests on macOS M1 - Mac Ventura (13) passed.

Pipeline on Agent
Hash: [CI build]

@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] Tests on macOS M1 - Mac Big Sur (11) passed 💻

All tests on macOS M1 - Mac Big Sur (11) passed.

Pipeline on Agent
Hash: [CI build]

@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] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: 24f10546356ae7fccb9b9e2daf2e8c392f5359ae [CI build]

@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.

✅ API diff for current PR / commit

Legacy Xamarin (No breaking changes)
  • iOS (no change detected)
  • tvOS (no change detected)
  • watchOS (no change detected)
  • macOS (no change detected)
NET (empty diffs)
  • iOS: (empty diff detected)
  • tvOS: (empty diff detected)
  • MacCatalyst: (empty diff detected)
  • macOS: (empty diff detected)

✅ API diff vs stable

Legacy Xamarin (No breaking changes)
.NET (No breaking changes)
Legacy Xamarin (stable) vs .NET

ℹ️ Generator diff

Generator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes)

Pipeline on Agent
Hash: 24f10546356ae7fccb9b9e2daf2e8c392f5359ae [CI build]

@vs-mobiletools-engineering-service2

This comment was marked as outdated.

@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] Artifacts 📚

Artifacts were not provided.

Pipeline on Agent
Hash: [CI build]

Please sign in to comment.