- 
                Notifications
    You must be signed in to change notification settings 
- Fork 1.1k
Add first draft of unsafe evolution specification #9781
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
base: main
Are you sure you want to change the base?
Conversation
* unsafe-temp: Feedback and more rules expansion
|  | ||
| * The _stackalloc_expression_ is being converted to a `Span<T>` or a `ReadOnlySpan<T>`. | ||
| * The _stackalloc_expression_ does not have a _stackalloc_initializer_. | ||
| * The _stackalloc_expression_ is used within a member that has `SkipLocalsInitAttribute` applied. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that the C# spec is being changed? Currently by C# spec, all stackallocs have undefined contents, regardless of [SkipLocalsInit]. Or is it instead going to become a roslyn implementation detail that it has the semantics of localloc.
The content of the newly allocated memory is undefined. You should initialize it, either with a stackalloc initializer, or a method like Span.Clear before it's used.
https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/stackalloc
And also from the latest draft spec:
// Memory uninitialized
Span<int> span1 = stackalloc int[3];There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think at this point it's safe to say that stackalloc is expected to emit a localloc that is always zeroed (unless SkipLocalsInit is set) and make the corresponding adjustments in the specs?
| unsafe | ||
| { | ||
| Console.WriteLine(*ptr); // Dereference of memory not managed by the runtime. This is unsafe. | ||
| ref int intRef = Unsafe.AsRef(ptr); // Conversion of memory not managed by the runtime to a `ref`. This is unsafe. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another way to say this (to avoid introducing new rules) is that all managed references are always implicitly dereferenceable (e.g. GC can interrupt at any point and dereference it), so Unsafe.AsRef(ptr) effectively dereferences the ptr right away
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree - the textual dereference is what should be interpreted as making it unsafe, which in this case is that Unsafe.AsRef<T> is equivalent to ref *(T*)ptr or ref ((T*)ptr)[0], and thus textually de-references the pointer, making it unsafe.
| * `stackalloc` under the conditions defined [below](#stack-allocation) | ||
|  | ||
| In addition to these expressions, expressions can also conditionally introduce a memory safety state of unsafe if they depend on any symbol that is marked as `unsafe`. For example, calling a method | ||
| that is marked as `unsafe` will cause the _invocation_expression_ to have a memory safety state of unsafe, even if the receiver and all arguments have a memory safety state of safe. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you capture an unsafe method into a normal delegate (like Action/Func). Or will there need to be unsafe versions of this? Or can you only grab the delegate in an unsafe block?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will there also need to be rules blocking converting the "delegate created from unsafe method" to System.Delegate?
| Note: I see my questions came up later. | 
| #### Pointer types | ||
|  | ||
| As mentioned, pointers become no longer inherently unsafe. Any references to unsafe contexts in [§24.3][pointer-types-spec] are deleted. Pointer types exist in normal C# and do not require `unsafe` | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does a change to allow pointer types in async (e.g., as parameters) also need to be made, or is that in another part already & I missed it? E.g., https://godbolt.org/z/WT85xGh95
| see any addition of `unsafe` by a derived implementation. This rule may add friction as we adopt `unsafe` across the ecosystem, and may need to be revisited to be moved down to a warning if it proves | ||
| to be a significant adoption blocker. | ||
|  | ||
| ### Delegates and lambdas | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unclear where to place this so I'll try here. I assume that DllImport definitions are going to be considered unsafe. Does this mean that keywords like extern will also require access via an unsafe context?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe an attribute can be placed on attributes to make them unsafe, or a new property added to AttributeUsageAttribute? This would then be immediately usable for DllImport, LibraryImport, SkipLocalsInit, UnsafeAccessor, custom IL post-processor attributes, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess another question would be if I want to use these attributes on my method, but I promise that the resulting method would end up safe, is there any way to express that? Or do I have to move everything to a local method & pass parameters around?
Note: this is still very in progress. Will aim to have it ready for review before 10/29.