Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RGen] Add a roslyn based implementation of the GetValueTypeSize. #22037

Merged
merged 8 commits into from
Jan 24, 2025

Conversation

mandel-macaque
Copy link
Member

Add a roslyn based implemenation of the GetValueTypeSize method that calulcates the size of a struct type based on the target platform.

Tests are written so that we can compare the output of the reflection based implementation with the roslyn one.

mandel-macaque and others added 3 commits January 23, 2025 16:19
Roslyn has no access to reflection. Add an implementation of the
IsBuiltInType method using the Roslyn APIs and test one method against
the other. Tests show that the implementations return the same values
for the same inputs.
Add a roslyn based implemenation of the GetValueTypeSize method that
calulcates the size of a struct type based on the target platform.

Tests are written so that we can compare the output of the reflection
based implementation with the roslyn one.
Copy link
Contributor

⚠️ Your code has been reformatted. ⚠️

If this is not desired, add the actions-disable-autoformat label, and revert the reformatting commit.

If files unrelated to your change were modified, try reverting the reformatting commit + merging with the target branch (and push those changes).

@mandel-macaque mandel-macaque added the actions-disable-autoformat Disable autoformatting C# code according to our style guidelines label Jan 24, 2025
@samhouts samhouts requested a review from Copilot January 24, 2025 00:44
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

src/rgen/Microsoft.Macios.Generator/Extensions/TypeSymbolExtensions.Core.cs:359

  • [nitpick] The error message for unhandled 'MarshalAs' attributes could be more descriptive. Include more context about the field and the attribute causing the issue.
throw new Exception ($"Unhandled MarshalAs attribute: {marshalAs.Value} on field {field.ToDisplayString ()}");

/// </summary>
/// <param name="symbol">The symbol whose fields to retireve.</param>
/// <returns>an array with all the none static fields of a struct.</returns>
public static IFieldSymbol [] GetStrutFields (this ITypeSymbol symbol)
Copy link
Preview

Copilot AI Jan 24, 2025

Choose a reason for hiding this comment

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

The method name 'GetStrutFields' contains a spelling error. It should be 'GetStructFields'.

Suggested change
public static IFieldSymbol [] GetStrutFields (this ITypeSymbol symbol)
public static IFieldSymbol [] GetStructFields (this ITypeSymbol symbol)

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

Positive Feedback
Negative Feedback

Provide additional feedback

Please help us improve GitHub Copilot by sharing more details about this comment.

Please select one or more of the options
@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

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.

…om:xamarin/xamarin-macios into dev/mandel/stret-getvalue-type-size-roslyn
Base automatically changed from dev/mandel/stret-is-built-in-type-roslyn to main January 24, 2025 12:15
@vs-mobiletools-engineering-service2

This comment has been minimized.

@vs-mobiletools-engineering-service2

This comment has been minimized.

@mandel-macaque
Copy link
Member Author

Failing tests are unrelated to the failure and we have the same tests passing in other platforms.

@mandel-macaque mandel-macaque merged commit b1a1800 into main Jan 24, 2025
43 of 48 checks passed
@mandel-macaque mandel-macaque deleted the dev/mandel/stret-getvalue-type-size-roslyn branch January 24, 2025 22:07
@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [CI Build] Build passed (Build packages) ✅

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [CI Build] Build passed (Detect API changes) ✅

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ [CI Build] Build passed (Build macOS tests) ✅

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ API diff for current PR / commit

.NET ( No breaking changes )

❗ API diff vs stable (Breaking changes)

.NET ( ❗ Breaking changes ❗ )

ℹ️ Generator diff

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

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

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

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

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

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

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

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

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

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

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Tests on macOS arm64 - Mac Sequoia (15) passed 💻

All tests on macOS arm64 - Mac Sequoia (15) passed.

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [CI Build] Windows Integration Tests passed 💻

All Windows Integration Tests passed.

Pipeline on Agent
Hash: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🚀 [CI Build] Test results 🚀

Test results

✅ All tests passed on VSTS: test results.

🎉 All 115 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 5 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 44 tests passed. Html Report (VSDrops) Download
✅ monotouch (iOS): All 8 tests passed. Html Report (VSDrops) Download
✅ monotouch (MacCatalyst): All 11 tests passed. Html Report (VSDrops) Download
✅ monotouch (macOS): All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch (tvOS): All 8 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: b1a1800bf0ffd7e8392e098fd58b0a3858846bd9 [PR build]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actions-disable-autoformat Disable autoformatting C# code according to our style guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants