[src] Add [Un]SupportedSimulator attributes.#25347
Conversation
Add two attributes to specify whether an API is available in the simulator:
* [UnsupportedSimulator ("<platform>")]: API is not available.
* [SupportedSimulator ("<platform>[osversion]")]: API is available, optionally
only in the specified OS version or not.
If no attribute is found, then the API is available in the simulator.
Unlike the normal availability attributes, specifying simulator availability for
one platform does not mean anything for any other platforms.
There was a problem hiding this comment.
Pull request overview
This PR introduces two new attributes ([SupportedSimulator] / [UnsupportedSimulator]) to describe simulator-only availability for APIs, wires the logic into both the binding generator (bgen) and introspection tests, and replaces a number of simulator-specific skip-lists with attribute-driven behavior (notably for newer Metal/MetalFX/VideoToolbox APIs).
Changes:
- Added
SupportedSimulatorAttribute/UnsupportedSimulatorAttributetoObjCRuntime, and trimmed them during linking. - Updated bgen to emit simulator-availability attributes for iOS/tvOS only, with new bgen unit tests to validate emission behavior.
- Updated introspection tests to honor simulator-availability attributes, and annotated relevant Metal/MetalFX/VideoToolbox APIs as unsupported in iOS/tvOS simulators.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/introspection/ApiCtorInitTest.cs | Removes hardcoded simulator skip cases now handled via simulator-availability attributes. |
| tests/introspection/ApiBaseTest.cs | Extends skip logic to consider simulator availability (IsAvailableInSimulator). |
| tests/common/PlatformInfo.cs | Adds reflection-based evaluation of [SupportedSimulator] / [UnsupportedSimulator] during simulator runs. |
| tests/bgen/tests/simulator-availability-attributes.cs | Adds a dedicated bgen fixture file exercising simulator availability attributes. |
| tests/bgen/BGenTests.cs | Adds tests verifying simulator attributes are emitted for iOS/tvOS and not emitted for macOS/Mac Catalyst. |
| src/videotoolbox.cs | Marks specific new VideoToolbox APIs as unsupported on iOS/tvOS simulators. |
| src/TrimAttributes.LinkDescription.xml | Ensures simulator-availability attributes are removed by the linker (attribute-instance trimming). |
| src/ObjCRuntime/SupportedSimulatorAttribute.cs | Introduces the new public attributes in ObjCRuntime. |
| src/metalfx.cs | Marks MetalFX descriptor APIs as unsupported on iOS/tvOS simulators. |
| src/Metal/MTLEnums.cs | Marks selected Metal enums as unsupported on iOS/tvOS simulators. |
| src/metal.cs | Marks many Metal (incl. Metal 4) APIs as unsupported on iOS/tvOS simulators. |
| src/frameworks.sources | Adds the new attribute source file to the shared core sources list. |
| src/bgen/Generator.cs | Adds emission of simulator-availability attributes (filtered to current platform; ignored on Mac platforms). |
| src/bgen/Enums.cs | Ensures simulator-availability attributes are printed for enum extension generation as well. |
| src/bgen/bgen.csproj | Includes the new attribute definitions in the bgen build. |
| src/bgen/AttributeManager.cs | Teaches attribute loading to resolve the new attribute types. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
/review |
|
✅ .NET for Apple Platforms PR Reviewer completed successfully! |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
This PR introduces two new attributes ([SupportedSimulator] and [UnsupportedSimulator]) to specify whether APIs are available in iOS/tvOS simulators. The implementation is well-structured and follows the repository's patterns.
✅ What's Good
- Clear architecture: The attributes are properly integrated into bgen, with filtering logic that only emits attributes relevant to the current platform
- Comprehensive testing: Includes bgen tests, introspection test integration, and proper handling in
PlatformInfo.IsAvailableInSimulator() - Proper trimming configuration: The attributes are correctly configured for removal during app trimming in
TrimAttributes.LinkDescription.xml - Consistent usage: Applied systematically to MLCompute, Metal, MetalFX, and VideoToolbox APIs that aren't available in simulators
- Test cleanup: Removes numerous simulator-specific workarounds from introspection tests now that the framework properly tracks simulator availability
📊 Issues Found
⚠️ 1 warning: Performance issue with string allocations inIsAvailableInSimulator()hot path- 💡 4 suggestions: Code duplication, error handling, documentation clarity, and immutability patterns
Verdict
No blocking issues. The warnings and suggestions are optional improvements that would enhance code quality but aren't required for merge.
CI status: Some test failures present (monotouch_tvos, xcframework, interdependent-binding-projects), but these appear unrelated to this change since the core simulator and introspection tests pass.
Generated by .NET for Apple Platforms PR Reviewer for issue #25347 · ● 875K
|
|
||
| public void PrintSimulatorAvailabilityAttributes (ICustomAttributeProvider? provider) | ||
| { | ||
| switch (CurrentPlatform) { |
There was a problem hiding this comment.
🤖 💡 Code organization — Consider extracting the platform check logic into a helper method.
The switch statement checking CurrentPlatform against iOS/tvOS appears in three places in this file (lines 5592-5601, 5615-5628, 5642-5655). Consider extracting this into a helper method like:
bool IsSimulatorAvailabilityRelevantPlatform()
{
return CurrentPlatform == PlatformName.iOS || CurrentPlatform == PlatformName.TvOS;
}This would reduce duplication and make the intent clearer.
Rule: Code duplication
|
|
||
| // There's a SupportedSimulator attribute for the current platform - check version | ||
| foreach (var attr in supportedAttrs) { | ||
| var versionString = attr.PlatformName.Substring (platformPrefix.Length); |
There was a problem hiding this comment.
🤖
The Substring call allocates a new string on every iteration. Consider using AsSpan() or extract the substring once before the loop:
var versionString = attr.PlatformName.AsSpan(platformPrefix.Length);
if (versionString.IsEmpty)
return true;
if (Version.TryParse(versionString, out var version) && PlatformInfo.Host.Version >= version)
return true;Or cache it once:
foreach (var attr in supportedAttrs) {
var versionString = attr.PlatformName.Substring (platformPrefix.Length);
if (string.IsNullOrEmpty (versionString)) {
return true; // supported, no version constraint
} else if (Version.TryParse (versionString, out var version) && PlatformInfo.Host.Version >= version) {
return true;
}
}This method is called in test introspection for every API member, so avoiding allocations matters.
Rule: Avoid unnecessary allocations
| var versionString = attr.PlatformName.Substring (platformPrefix.Length); | ||
| if (string.IsNullOrEmpty (versionString)) | ||
| return true; // supported, no version constraint | ||
| if (Version.TryParse (versionString, out var version) && PlatformInfo.Host.Version >= version) |
There was a problem hiding this comment.
🤖 💡 Error handling — Consider handling Version.TryParse failure more explicitly.
If Version.TryParse fails (returns false), the current code silently continues to the next attribute. This means a malformed version string like "ios17.0.invalid" would be ignored rather than causing an error.
Consider logging or failing fast if version parsing fails:
if (!string.IsNullOrEmpty(versionString)) {
if (!Version.TryParse (versionString, out var version))
throw new InvalidOperationException ($"Invalid version string in SupportedSimulator attribute: '{attr.PlatformName}'");
if (PlatformInfo.Host.Version >= version)
return true;
}This would catch binding definition errors earlier rather than silently skipping them.
Rule: Include actionable details in exceptions
| [AttributeUsage (AttributeTargets.Class | AttributeTargets.Interface | AttributeTargets.Enum | AttributeTargets.Method | AttributeTargets.Property | AttributeTargets.Constructor | AttributeTargets.Field, AllowMultiple = true)] | ||
| public sealed class SupportedSimulatorAttribute : Attribute { | ||
| /// <summary>Initializes a new <see cref="SupportedSimulatorAttribute" /> attribute.</summary> | ||
| /// <param name="platformName">The platform where this API is supported in the simulator. Optionally specifies a platform version.</param> |
There was a problem hiding this comment.
🤖 💡 Documentation — The XML docs could be clearer about the version format.
The <param> documentation says "The platform where this API is supported in the simulator. Optionally specifies a platform version." but doesn't explain the expected format.
Consider adding an example to clarify:
/// <param name="platformName">The platform where this API is supported in the simulator.
/// Format: platform name (e.g., "ios", "tvos") optionally followed by a version number (e.g., "ios17.0").</param>This would help binding authors use the attribute correctly.
Rule: Documentation
…rove docs - Extract IsSimulatorAvailabilityRelevantPlatform() helper in Generator.cs to reduce duplicated platform switch logic. - Use AsSpan() instead of Substring() in PlatformInfo.IsAvailableInSimulator to avoid string allocations in the hot path. - Throw InvalidOperationException on malformed version strings instead of silently continuing. - Improve XML doc for platformName param with format examples. - Make PlatformName properties init-only for immutability. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
Adds ApiSimulatorAvailabilityTest which verifies that: - P/Invokes and [Field] properties marked with [UnsupportedSimulator] truly have unavailable native symbols in the simulator. - P/Invokes and [Field] properties NOT marked as unsupported don't have missing symbols (detecting cases where [UnsupportedSimulator] is needed). The test only runs in the simulator and is ignored on device/desktop. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #fc0a48a] Build passed (Detect API changes) ✅Pipeline on Agent |
✅ [PR Build #fc0a48a] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [PR Build #fc0a48a] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #fc0a48a] Test results 🔥Test results❌ Tests failed on VSTS: test results 0 tests crashed, 24 tests failed, 145 tests passed. Failures❌ interdependent-binding-projects tests2 tests failed, 2 tests passed.Failed tests
Html Report (VSDrops) Download ❌ linker tests22 tests failed, 22 tests passed.Failed tests
Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
Add two attributes to specify whether an API is available in the simulator:
only in the specified OS version or not.
If no attribute is found, then the API is available in the simulator.
Unlike the normal availability attributes, specifying simulator availability for
one platform does not mean anything for any other platforms.