Skip to content

Commit

Permalink
Address code review feedback
Browse files Browse the repository at this point in the history
Commit migrated from dotnet/corefx@b1aba46
  • Loading branch information
terrajobst committed Nov 7, 2017
1 parent 9d506a3 commit ae7588d
Showing 1 changed file with 17 additions and 9 deletions.
26 changes: 17 additions & 9 deletions docs/libraries/api-guidelines/System.Memory.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,34 +11,42 @@ specs can be found here:

* `ReadOnlySpan<T>` is effectively the universal receiver, in that `T[]`, `T*`,
`Memory<T>`, `ReadOnlyMemory<T>`, `Span<T>`, `ArraySegment<T>` can all be
converted to it. So if you can declare your API to accept a `ReadOnlySpan<T>`
converted to it. So if you can declare your API to accept a `ReadOnlySpan<T>`
and behave efficiently, that's best, as any of these inputs can be used with
your method.
* Similarly for `Span<T>`, if you need write access in the implementation.
* It allows building safe public APIs that can operate on unmanaged memory
without forcing all consumers to use pointers (and thus becoming unsafe) while
still getting equivalent performance.
without forcing all consumers to use pointers (and thus becoming unsafe). The
implementation can still extract a raw pointer, therefore getting equivalent
performance if necessary.
* It's generally best for a synchronous method to accept `Span<T>` or
`ReadOnlySpan<T>`. However, due to `ReadOnlySpan<T>`/`Span<T>` limitations,
this may be limiting for the implementation. In particular, if the
`ReadOnlySpan<T>`. However, since `ReadOnlySpan<T>`/`Span<T>` are stack-only
[1], this may be too limiting for the implementation. In particular, if the
implementation needs to be able to store the argument for later usage, such as
with an async method or an iterator, `ReadOnlySpan<T>`/`Span<T>` is
with an asynchronous method or an iterator, `ReadOnlySpan<T>`/`Span<T>` is
inappropriate. `ReadOnlyMemory<T>`/`Memory<T>` should be used in such
situations.


[1] *stack-only* isn't the best way to put it. Strictly speaking, these types
are called `ref`-like types. These types must be structs, cannot be fields
in classes, cannot be boxed, and cannot be used to instantiate generic
types. Value types containing fields of `ref`-like types must themselves be
`ref`-like types.

## Guidance

* **DO NOT** use pointers for methods operating on buffers. Instead, use
appropriate type form below. In performance critical code where bounds
appropriate type from below. In performance critical code where bounds
checking is unacceptable, the method's implementation can still pin the span
and get the raw pointer if necessary. The key is that you don't spread the
pointer through the public API.
- Synchronous, read-only access needed: `ReadOnlySpan<T>`
- Synchronous, writable access needed: `Span<T>`
- Asynchronous, read-only access needed: `ReadOnlyMemory<T>`
- Asynchronous, writable access needed: `Memory<T>`
* **CONSIDER** using `stackalloc` with `Span<T>` when you need temporary storage
but you need to avoid allocations and associated life-time management.
* **CONSIDER** using `stackalloc` with `Span<T>` when you need small temporary
storage but you need to avoid allocations and associated life-time management.
* **AVOID** providing overloads for both `ReadOnlySpan<T>` and `Span<T>` as `Span<T>`
can be implicitly converted to `ReadOnlySpan<T>`.
* **AVOID** providing overloads for both `ReadOnlySpan<T>`/`Span<T>` as well as
Expand Down

0 comments on commit ae7588d

Please sign in to comment.