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

Refactor pass with Unsafe.NullRef<T> and Unsafe.IsNullRef<T> #3509

Closed
Sergio0694 opened this issue Sep 28, 2020 · 0 comments · Fixed by #3510
Closed

Refactor pass with Unsafe.NullRef<T> and Unsafe.IsNullRef<T> #3509

Sergio0694 opened this issue Sep 28, 2020 · 0 comments · Fixed by #3510
Assignees
Labels
Completed 🔥 improvements ✨ .NET Components which are .NET based (non UWP specific)
Milestone

Comments

@Sergio0694
Copy link
Member

Sergio0694 commented Sep 28, 2020

Overview

Follow up from @michael-hawker's comment here: #3380 (comment)

There are places in the toolkit were we have verbose expressions to check for null refs or get null refs, eg:

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/070d69014d8e7e85aa2be2359c0ee564e25b66bc/Microsoft.Toolkit.HighPerformance/Buffers/StringPool.cs#L497

https://github.com/windows-toolkit/WindowsCommunityToolkit/blob/49a88c7d570854026855becbd25ae53c1873417f/Microsoft.Toolkit.HighPerformance/Extensions/ArrayExtensions.cs#L47-L50

There are now portable .NET Standard APIs to do this in a less verbose manner (added in dotnet/runtime#40008), which will be in stable release when .NET 5 drops in November (though they'll just target .NET Standard like the current System.Runtime.CompilerServices.Unsafe package). This issue is for tracking a refactoring pass on the whole toolkit to update all the current instances of those workarounds mentioned above, using these new APIs. Notes:

  • No target framework changes
  • No API changes
  • Purely a refactoring
@Sergio0694 Sergio0694 added improvements ✨ .NET Components which are .NET based (non UWP specific) labels Sep 28, 2020
@Sergio0694 Sergio0694 added this to the 7.0 milestone Sep 28, 2020
@Sergio0694 Sergio0694 self-assigned this Sep 28, 2020
@ghost ghost added the In-PR 🚀 label Sep 28, 2020
@ghost ghost closed this as completed in #3510 Nov 10, 2020
ghost pushed a commit that referenced this issue Nov 10, 2020
## Close #3509 
<!-- Add the relevant issue number after the "#" mentioned above (for ex: Fixes #1234) which will automatically close the issue once the PR is merged. -->

<!-- Add a brief overview here of the feature/bug & fix. -->

## PR Type
What kind of change does this PR introduce?
<!-- Please uncomment one or more that apply to this PR. -->

- Refactoring (no functional changes, no api changes)
<!-- - Build or CI related changes -->
<!-- - Documentation content changes -->
<!-- - Sample app changes -->
<!-- - Other... Please describe: -->


## What is the current behavior?
<!-- Please describe the current behavior that you are modifying, or link to a relevant issue. -->
There are a number of usages of `Unsafe.AsRef<T>(null)` and similar that are verbose.

## What is the new behavior?
<!-- Describe how was this issue resolved or changed? -->
These occurrences are replaced using the new `Unsafe.NullRef<T>()` and `Unsafe.IsNullRef<T>(...)` APIs.

## Notes

Leaving this as draft as the new `System.Runtime.CompilerServices.Unsafe` package is still in release candidate.

## PR Checklist

Please check if your PR fulfills the following requirements:

- [X] Tested code with current [supported SDKs](../readme.md#supported)
- [ ] ~~Pull Request has been submitted to the documentation repository [instructions](..\contributing.md#docs). Link: <!-- docs PR link -->~~
- [ ] ~~Sample in sample app has been added / updated (for bug fixes / features)~~
    - [ ] ~~Icon has been created (if new sample) following the [Thumbnail Style Guide and templates](https://github.com/windows-toolkit/WindowsCommunityToolkit-design-assets)~~
- [X] Tests for the changes have been added (for bug fixes / features) (if applicable)
- [X] Header has been added to all new source files (run *build/UpdateHeaders.bat*)
- [X] Contains **NO** breaking changes
@ghost ghost added Completed 🔥 and removed In-PR 🚀 labels Nov 10, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Jan 9, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Completed 🔥 improvements ✨ .NET Components which are .NET based (non UWP specific)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant