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

Create inline-arrays.md #7064

Merged
merged 39 commits into from
Apr 14, 2023
Merged

Create inline-arrays.md #7064

merged 39 commits into from
Apr 14, 2023

Conversation

AlekseyTs
Copy link
Contributor

No description provided.

@AlekseyTs AlekseyTs requested a review from a team as a code owner March 16, 2023 16:08
Copy link
Contributor

@Joe4evr Joe4evr left a comment

Choose a reason for hiding this comment

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

One more typo and a grammatical nit.

proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

Some style and grammar comments.

proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved
@333fred
Copy link
Member

333fred commented Mar 16, 2023

proposals/safe-fixed-sized-buffers.md Outdated Show resolved Hide resolved

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
Copy link
Member

Choose a reason for hiding this comment

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

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?

@AlekseyTs AlekseyTs changed the title Create safe-fixed-sized-buffers.md Create safe-fixed-size-buffers.md Mar 17, 2023
``` C#
void M1(Buffer10<int> x)
{
ref int a = ref x[0]; // Ok, equivalent to `ref int a = ref x.AsSpan()[0]`
Copy link
Member

Choose a reason for hiding this comment

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

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.

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.
Copy link
Member

@stephentoub stephentoub Apr 3, 2023

Choose a reason for hiding this comment

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

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.

#6010

@333fred
Copy link
Member

333fred commented Apr 3, 2023

}
```

Also, quite possibly compiler will be able to omit the ```Unsafe.As``` and ```Unsafe.AsRef``` calls in IL.
Copy link
Member

Choose a reason for hiding this comment

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

Once you do that, the AsSpan and AsReadOnlySpan helpers do not provide any IL size benefits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once you do that, the AsSpan and AsReadOnlySpan helpers 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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

I'm curious about this as well. What was the rationale?

@AlekseyTs AlekseyTs changed the title Create safe-fixed-size-buffers.md Create inline-arrays.md Apr 14, 2023
@AlekseyTs AlekseyTs merged commit c215493 into main Apr 14, 2023
@AlekseyTs AlekseyTs deleted the AlekseyTs-patch-5 branch April 14, 2023 20:09

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.
Copy link
Member

Choose a reason for hiding this comment

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

"Other examples the span types."

Not sure hwat this means :)

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

Successfully merging this pull request may close these issues.