Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Remove replaced Span/ReadOnlySpan methods. #16086

Closed
wants to merge 2 commits into from
Closed

Remove replaced Span/ReadOnlySpan methods. #16086

wants to merge 2 commits into from

Conversation

ianhays
Copy link

@ianhays ianhays commented Jan 29, 2018

Continuation of #15941

This PR removes the old functions that are no longer being exposed in corefx as of the merging of dotnet/corefx#26467.

cc: @ahsonkhan @stephentoub

@ianhays ianhays added this to the 2.1.0 milestone Jan 29, 2018
@ianhays ianhays self-assigned this Jan 29, 2018
@jkotas
Copy link
Member

jkotas commented Jan 30, 2018

Could you please also delete DangerousGetPinnableReference while you are on it?

@jkotas
Copy link
Member

jkotas commented Jan 30, 2018

The few places that started using it again should just use _pointer.Value directly.

Copy link
Member

@ahsonkhan ahsonkhan left a comment

Choose a reason for hiding this comment

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

lgtm

@ianhays
Copy link
Author

ianhays commented Jan 30, 2018

Could you please also delete DangerousGetPinnableReference while you are on it?

Sure thing, done.

@ianhays
Copy link
Author

ianhays commented Jan 30, 2018

It would appear that some of the tests are using an older version of system.private.corelib. I'm getting errors that functions I added last week aren't available:

SpanBench.cs(509,42): error CS0117: 'MemoryMarshal' does not contain a definition for 'CreateSpan' [D:\j\workspace\x64_checked_w---76911707\tests\src\JIT\Performance\CodeQuality\Span\SpanBench.csproj]
SpanBench.cs(1005,44): error CS0117: 'MemoryMarshal' does not contain a definition for 'Cast' [D:\j\workspace\x64_checked_w---76911707\tests\src\JIT\Performance\CodeQuality\Span\SpanBench.csproj]

@jkotas
Copy link
Member

jkotas commented Jan 30, 2018

Yes, you need to get #16091 through first.

@ianhays
Copy link
Author

ianhays commented Jan 30, 2018

Ah, that explains it. Thanks. I'll push the test change to that PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants