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

[net8.0] added a test for blittability of pinvokes, partial #15684 #16525

Merged

Conversation

stephen-hawley
Copy link
Contributor

This adds a unit test to check for blittability of arguments to pinvokes.

What's going on here?
Loop over every unit pinvoke and checks each arg and return value for blittability.
An argument is blittable if it's:

  1. a type that we like
  2. a pointer
  3. a struct

a struct is blittable if all its non-static fields are blittable.

This test runs, but fails on about 1200 cases per platform. Most of these failures are with enums.

The rules might need to be adjusted to account for ref or out args and 1 dimensional arrays of blittables.

@stephen-hawley stephen-hawley added skip-all-tests Skip all the tests not-notes-worthy Ignore for release notes labels Oct 31, 2022
@github-actions
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).

@rolfbjarne rolfbjarne added not-notes-worthy Ignore for release notes and removed not-notes-worthy Ignore for release notes labels Nov 2, 2022
@rolfbjarne
Copy link
Member

This test runs, but fails on about 1200 cases per platform. Most of these failures are with enums.

A quick test shows that enums are allowed: https://gist.github.com/rolfbjarne/716fcf0d8842639848fc0656bf3deac9

while ref/out/array types are not (see https://gist.github.com/rolfbjarne/6f9b252c84bf89d8770a6420cd56a4e5 for ref types).

@github-actions
Copy link
Contributor

github-actions bot commented Dec 5, 2022

⚠️ 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).

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🌱 [CI Build] Test results 🌱

All tests have been skipped because the label 'skip-all-tests' was set.

Pipeline on Agent
Hash: 17998ace9a49a02c1e17acb1407c1926668b5e1d [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [PR Build] Artifacts 📚

Artifacts were not provided.

Pipeline on Agent XAMBOT-1103.Monterey'
Hash: 17998ace9a49a02c1e17acb1407c1926668b5e1d [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

✅ 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 (Breaking changes)

Legacy Xamarin (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
.NET (:heavy_exclamation_mark: Breaking changes :heavy_exclamation_mark:)
Legacy Xamarin (stable) vs .NET

✅ Generator diff

Generator diff is empty

Pipeline on Agent
Hash: 17998ace9a49a02c1e17acb1407c1926668b5e1d [PR build]

Copy link
Contributor

@tj-devel709 tj-devel709 left a comment

Choose a reason for hiding this comment

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

LGTM!

@stephen-hawley stephen-hawley merged commit 3c512a8 into xamarin:main Dec 6, 2022
@stephen-hawley stephen-hawley deleted the check-blittable-pinvokes branch December 6, 2022 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
not-notes-worthy Ignore for release notes skip-all-tests Skip all the tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants