Conversation
Joe4evr
left a comment
There was a problem hiding this comment.
One more typo and a grammatical nit.
333fred
left a comment
There was a problem hiding this comment.
Some style and grammar comments.
Co-authored-by: Fred Silberberg <frsilb@microsoft.com>
|
Consider moving https://github.com/dotnet/csharplang/blob/main/proposals/fixed-sized-buffers.md to the Rejected folder. |
|
|
||
| When a fixed-size buffer member is static or the outermost containing struct variable of a fixed-size buffer member is a static variable, an instance variable of a class instance, or an array element, the elements of the fixed-size buffer are automatically initialized to their default values. In all other cases, the initial content of a fixed-size buffer is undefined. | ||
|
|
||
| ## Alternatives |
There was a problem hiding this comment.
Perhaps also remove "safe fixed size buffers" section from the C# 11 proposal? https://github.com/dotnet/csharplang/blob/main/proposals/csharp-11.0/low-level-struct-improvements.md#safe-fixed-size-buffers And/or add link there pointing to this new proposal?
proposals/safe-fixed-size-buffers.md
Outdated
| ``` C# | ||
| void M1(Buffer10<int> x) | ||
| { | ||
| ref int a = ref x[0]; // Ok, equivalent to `ref int a = ref x.AsSpan()[0]` |
There was a problem hiding this comment.
The JIT should already be able to turn this into the equivalent of ref x._element0, but we should double-check it indeed does always do all the necessary inlining and the like.
proposals/safe-fixed-size-buffers.md
Outdated
| We might want to specially recognize ```System.Runtime.InteropServices.MemoryMarshal.CreateReadOnlySpan``` method and allow | ||
| passing a reference to a readonly location as its first argument. This should allow users to define their own fixed-size buffer | ||
| types. For example, in order to be able to share a type across multiple assemblies and also include custom APIs into its definition. | ||
| This might also help framework to define our shared fixed-size buffer types. |
There was a problem hiding this comment.
Developers can do this today with:
MemoryMarshal.CreateReadOnlySpan(ref Unsafe.AsRef(in location), length);But this is also an example of an API that's motivation for allowing ref readonly on parameters; we would change CreateReadOnlySpan to accept a ref readonly instead of a ref, existing usage would continue to work, but it would also then work for creating a span around readonly fields.
|
Partly discussed in LDM: https://github.com/dotnet/csharplang/blob/main/meetings/2023/LDM-2023-04-03.md#fixed-size-buffers |
…e-fixed-size-buffers.md
…fe-fixed-size-buffers.md
proposals/safe-fixed-size-buffers.md
Outdated
| } | ||
| ``` | ||
|
|
||
| Also, quite possibly compiler will be able to omit the ```Unsafe.As``` and ```Unsafe.AsRef``` calls in IL. |
There was a problem hiding this comment.
Once you do that, the AsSpan and AsReadOnlySpan helpers do not provide any IL size benefits.
There was a problem hiding this comment.
Once you do that, the
AsSpanandAsReadOnlySpanhelpers do not provide any IL size benefits.
After an offline discussion with @VSadov, I decided to not eliminate the calls. I will remove this statement from the spec.
There was a problem hiding this comment.
Can you share the reasoning behind it? It looks be perfectly fine to me to skip these unnecessary Unsafe.As calls in the generated IL.
There was a problem hiding this comment.
I'm curious about this as well. What was the rationale?
|
|
||
| Language will provide a type-safe/ref-safe way for accessing elements of inline array types. The access will be span based. | ||
| This limits support to inline array types with element types that can be used as a type argument. | ||
| For example, a pointer type cannot be used as an element type. Other examples the span types. |
There was a problem hiding this comment.
"Other examples the span types."
Not sure hwat this means :)
No description provided.