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

[src] Make BlockLiteral disposable and update usage accordingly. #17597

Merged
merged 5 commits into from
Feb 24, 2023

Conversation

rolfbjarne
Copy link
Member

  • Make BlockLiteral disposable.

    This also means to modify the cleanup logic so that it's safe to call
    Dispose more than once.

  • Create a helper class to create a block for a simple Action delegate.

    This allows us to simplify a good chunk of code.

  • Update all block creation to use the new block API, where blocks are
    disposable. This makes the code pattern a lot simpler.

    I've changed all the P/Invokes to use an unsafe 'BlockLiteral*' pointer,
    because 'using' variables can't be passed as ref arguments, so the choice
    was either to make the parameter type 'IntPtr' and cast away the pointer:

      using var block = new BlockLiteral ();
      PInvoke ((IntPtr) &block);
    

    or make the parameter an unsafe 'BlockLiteral*' pointer:

      unsafe {
          using var block = new BlockLiteral ();
          PInvoke (&block);
      }
    

    The upcoming support for function pointers don't have this choice:
    function pointers are always unsafe, so I chose to go the unsafe route
    here as well, since it makes the code simpler once support for function
    pointers has been implemented.

Contributes towards #15783.

This PR might be easier to review commit-by-commit.

This also means to modify the cleanup logic so that it's safe to call Dispose more
than once.
…tion delegate.

This allows us to simplify a good chunk of code.
Update all block creation to use the new block API, where blocks are disposable.
This makes the code pattern a lot simpler.

I've changed all the P/Invokes to use an unsafe 'BlockLiteral*' pointer, because
'using' variables can't be passed as ref arguments, so the choice was either to make
the parameter type 'IntPtr' and cast away the pointer:

    using var block = new BlockLiteral ();
    PInvoke ((IntPtr) &block);

or make the parameter an unsafe 'BlockLiteral*' pointer:

    unsafe {
        using var block = new BlockLiteral ();
        PInvoke (&block);
    }

The upcoming support for function pointers don't have this choice: function pointers
are always unsafe, so I chose to go the unsafe route here as well, since it makes
the code simpler once support for function pointers has been implemented.
@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).

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

✅ API diff for current PR / commit

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

✅ API diff vs stable

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

✅ Generator diff

Generator diff is empty

Pipeline on Agent
Hash: 9fa621f68bcb96c50d3599accb554858afae193a [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

📚 [PR Build] Artifacts 📚

Packages generated

View packages

Pipeline on Agent XAMBOT-1042.Monterey'
Hash: 9fa621f68bcb96c50d3599accb554858afae193a [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

❌ [PR Build] Tests on macOS M1 - Mac Ventura (13.0) failed ❌

Failed tests are:

  • xammac_tests

Pipeline on Agent
Hash: 9fa621f68bcb96c50d3599accb554858afae193a [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

💻 [PR Build] Tests on macOS M1 - Mac Big Sur (11.5) passed 💻

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

Pipeline on Agent
Hash: 9fa621f68bcb96c50d3599accb554858afae193a [PR build]

@vs-mobiletools-engineering-service2
Copy link
Collaborator

🔥 [CI Build] Test results 🔥

Test results

❌ Tests failed on VSTS: simulator tests

0 tests crashed, 2 tests failed, 223 tests passed.

Failures

❌ xammac tests

2 tests failed, 1 tests passed.
  • xammac tests/Mac Modern/Release: Failed (Test run failed.
    Tests run: 2830 Passed: 2695 Inconclusive: 11 Failed: 1 Ignored: 134)
  • xammac tests/Mac Modern/Release (all optimizations): Failed (Test run failed.
    Tests run: 2830 Passed: 2697 Inconclusive: 11 Failed: 2 Ignored: 131)

Html Report (VSDrops) Download

Successes

✅ bcl: All 69 tests passed. Html Report (VSDrops) Download
✅ cecil: All 1 tests passed. Html Report (VSDrops) Download
✅ dotnettests: All 1 tests passed. Html Report (VSDrops) Download
✅ fsharp: All 7 tests passed. Html Report (VSDrops) Download
✅ framework: All 8 tests passed. Html Report (VSDrops) Download
✅ generator: All 2 tests passed. Html Report (VSDrops) Download
✅ interdependent_binding_projects: All 7 tests passed. Html Report (VSDrops) Download
✅ install_source: All 1 tests passed. Html Report (VSDrops) Download
✅ introspection: All 8 tests passed. Html Report (VSDrops) Download
✅ linker: All 65 tests passed. Html Report (VSDrops) Download
✅ mac_binding_project: All 1 tests passed. Html Report (VSDrops) Download
✅ mmp: All 2 tests passed. Html Report (VSDrops) Download
✅ mononative: All 12 tests passed. Html Report (VSDrops) Download
✅ monotouch: All 25 tests passed. Html Report (VSDrops) Download
✅ msbuild: All 2 tests passed. Html Report (VSDrops) Download
✅ mtouch: All 1 tests passed. Html Report (VSDrops) Download
✅ xcframework: All 8 tests passed. Html Report (VSDrops) Download
✅ xtro: All 2 tests passed. Html Report (VSDrops) Download

Pipeline on Agent
Hash: 9fa621f68bcb96c50d3599accb554858afae193a [PR build]

@rolfbjarne
Copy link
Member Author

Test failure is unrelated (https://github.com/xamarin/maccore/issues/2630).

@rolfbjarne rolfbjarne merged commit ee5d176 into xamarin:main Feb 24, 2023
@rolfbjarne rolfbjarne deleted the disposable-blocks branch February 24, 2023 09:28
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.

None yet

3 participants