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

Expose {ReadOnly}Memory from System.Runtime#23841

Merged
stephentoub merged 2 commits into
dotnet:masterfrom
ahsonkhan:AddToSystemRuntime
Sep 9, 2017
Merged

Expose {ReadOnly}Memory from System.Runtime#23841
stephentoub merged 2 commits into
dotnet:masterfrom
ahsonkhan:AddToSystemRuntime

Conversation

@ahsonkhan
Copy link
Copy Markdown

Part of dotnet/corefxlab#1711

Adding the following types:

  • Memory<T>
  • ReadOnlyMemory<T>
  • OwnedMemory<T>
  • MemoryHandle
  • IRetainable

Order of changes:

  1. Add to mscorlib (Adding {ReadOnly}Memory, OwnedMemory, MemoryHandle, and IRetainable coreclr#13583).
  2. After the type has been added to mscorlib, add impl (for portable) and type forwarders to System.Memory (Adding Memory, OwnedMemory, MemoryHandle, and IRetainable & Tests #23701) and System.Runtime (this PR) here in corefx.
  3. Remove impl from corefxlab and update uses/references in Pipelines/Buffers.Experimental.
  4. Other changes/additions like review/add APIs from MemoryExtensions

cc @shiftylogic, @KrzysztofCwalina, @davidfowl, @jkotas, @stephentoub, @atsushikan, @ericstj

@ahsonkhan
Copy link
Copy Markdown
Author

I am not sure how to resolve this error:
System.Runtime.cs(3815,108): error CS0234: The type or namespace name 'GCHandle' does not exist in the namespace 'System.Runtime.InteropServices' (are you missing an assembly reference?) [D:\GitHub\Fork\corefx\src\System.Runtime\ref\System.Runtime.csproj]

GCHandle was not in .NET Standard 1.0 (it was added in .NET Standard 1.1).
https://github.com/dotnet/standard/blob/master/docs/versions/netstandard1.1_diff.md
Any ideas?

@stephentoub
Copy link
Copy Markdown
Member

Any ideas?

GCHandle is currently defined in System.Runtime.InteropServices:


Presumably it'll need to be forwarded down to System.Runtime.
cc: @weshaggard

@weshaggard
Copy link
Copy Markdown
Member

Presumably it'll need to be forwarded down to System.Runtime.

Yes it would need to be moved down to resolve this dependency.

@ahsonkhan have these APIs been reviewed and approved yet? If so can you point me to the issue for that if not can you please file an issue to track the review of these APIs?

@ahsonkhan
Copy link
Copy Markdown
Author

ahsonkhan commented Sep 7, 2017

have these APIs been reviewed and approved yet? If so can you point me to the issue for that

Yes, we had the API review iterations offline back in June and it was approved. I don't have an API specific issue (this is the issue for moving the types to corefx - dotnet/corefxlab#1711).

cc @terrajobst

if not can you please file an issue to track the review of these APIs?

Should I file an issue regardless?
Edit: https://github.com/dotnet/corefx/issues/23862

@mmitche
Copy link
Copy Markdown
Member

mmitche commented Sep 8, 2017

@dotnet-bot test this please (accidentlaly deleted VMs while in use)

@ahsonkhan
Copy link
Copy Markdown
Author

ahsonkhan commented Sep 9, 2017

Is the PR good to go?
@stephentoub, @wes

@stephentoub
Copy link
Copy Markdown
Member

Thanks, @ahsonkhan. LGTM.

@stephentoub stephentoub merged commit df221ec into dotnet:master Sep 9, 2017
@ahsonkhan ahsonkhan deleted the AddToSystemRuntime branch September 14, 2017 21:47
@karelz karelz added this to the 2.1.0 milestone Oct 11, 2017
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
Expose {ReadOnly}Memory from System.Runtime

Commit migrated from dotnet/corefx@df221ec
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.

6 participants