Skip to content

[RGen] Teach been to ignore rgen types.#21449

Merged
mandel-macaque merged 2 commits intomainfrom
dev/mandel/bgen-ignore-rgen-types
Oct 16, 2024
Merged

[RGen] Teach been to ignore rgen types.#21449
mandel-macaque merged 2 commits intomainfrom
dev/mandel/bgen-ignore-rgen-types

Conversation

@mandel-macaque
Copy link
Copy Markdown
Contributor

Teach our bgen to ignore those types that have been marked as a rgen BindingType. The code adds a new smart enum in the sources that should not be processed by bgen and therefore should not have the smart enum extension method. You can verify that this is try by looking at the API diff:

Screenshot 2024-10-15 at 14 58 00

Teach bgen to ignore the types that have been marked by the
BindingTypeAttribute.
@mandel-macaque mandel-macaque force-pushed the dev/mandel/bgen-ignore-rgen-types branch from e06f41c to 7a90a3f Compare October 15, 2024 19:03
@@ -0,0 +1,44 @@
#pragma warning disable APL0003
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What's this warning? I didn't see it in a search for it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It is defined via a ExperimentalAttribute: https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/proposals/csharp-12.0/experimental-attribute

This was added in a previous PR, which is documented under https://github.com/xamarin/xamarin-macios/blob/main/docs/preview-apis.md#rgen-apl0003 i

Since we are using them, we have to ensure that we do not have a compilation warning.

Comment thread src/bgen/Generator.cs

// check if the type has been marked as a type that will be generated by the new code generator, if that
// is the case, bgen will ignore it allowing the rgen code generator add the type to the final assembly
bool is_rgen_type = AttributeManager.HasAttribute<BindingTypeAttribute> (type);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Will this predicate be used in other places and if so, does it justify putting it in its own method?

Copy link
Copy Markdown
Contributor Author

@mandel-macaque mandel-macaque Oct 15, 2024

Choose a reason for hiding this comment

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

Are you talking about AttributeManager.HasAttribute<BindingTypeAttribute> that is a generic method that we use to verify if a class/method as an attribute. If you are talking about is_rgen_type, we can do that in the if.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

✅ API diff for current PR / commit

.NET (No breaking changes)

✅ API diff vs stable

.NET (No breaking changes)

ℹ️ Generator diff

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

Pipeline on Agent
Hash: c741ac47c5cfd5c3ed597e1d360d800b854a31d0 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Monterey (12) passed 💻

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

Pipeline on Agent
Hash: c741ac47c5cfd5c3ed597e1d360d800b854a31d0 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

💻 [CI Build] Tests on macOS X64 - Mac Sonoma (14) passed 💻

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

Pipeline on Agent
Hash: c741ac47c5cfd5c3ed597e1d360d800b854a31d0 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

💻 [CI Build] Tests on macOS M1 - Mac Ventura (13) passed 💻

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

Pipeline on Agent
Hash: c741ac47c5cfd5c3ed597e1d360d800b854a31d0 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 101 tests passed 🎉

Tests counts

✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (iOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (MacCatalyst): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (macOS): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (Multiple platforms): All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests (tvOS): All 1 tests passed. Html Report (VSDrops) Download
✅ framework: All 2 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 4 tests passed. Html Report (VSDrops) Download
✅ generator: All 3 tests passed. Html Report (VSDrops) Download
✅ interdependent-binding-projects: All 4 tests passed. Html Report (VSDrops) Download
✅ introspection: All 4 tests passed. Html Report (VSDrops) Download
✅ linker: All 40 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 7 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 9 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 7 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 4 tests passed. Html Report (VSDrops) Download
✅ xtro: All 1 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: c741ac47c5cfd5c3ed597e1d360d800b854a31d0 [PR build]

[SupportedOSPlatform ("maccatalyst17.0")]
[SupportedOSPlatform ("macos14.0")]
public enum AVCaptureReactionType {
[Field ("AVCaptureReactionTypeThumbsUp")]
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

RGen doesn't do all it's supposed: the [Field] attributes are supposed to generate a bunch of additional code, which isn't happening.

If that's coming later, then the [Field] attribute should be removed now, and added back when the [Field] support is back.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sure, but the idea was to add the field and verify the bgen was not generating anything. So it is 100% on purpose that we do not have the extension methods

@vs-mobiletools-engineering-service2
Copy link
Copy Markdown
Collaborator

💻 [CI Build] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: c741ac47c5cfd5c3ed597e1d360d800b854a31d0 [PR build]

@mandel-macaque mandel-macaque merged commit 8e0cec3 into main Oct 16, 2024
@mandel-macaque mandel-macaque deleted the dev/mandel/bgen-ignore-rgen-types branch October 16, 2024 17:11
haritha-mohan pushed a commit that referenced this pull request Oct 19, 2024
Teach our bgen to ignore those types that have been marked as a rgen
BindingType. The code adds a new smart enum in the sources that should
not be processed by bgen and therefore should not have the smart enum
extension method. You can verify that this is try by looking at the API
diff:

<img width="560" alt="Screenshot 2024-10-15 at 14 58 00"
src="https://github.com/user-attachments/assets/83790ae6-d94a-424f-8f70-e33bde7c4f22">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants