Allow unsafe blocks in iterators #73046
Conversation
dotnet-issue-labeler
bot
added
Area-Compilers
untriaged
jjonescz
force-pushed
the
RefInAsync-03-UnsafeInIterators
branch
from
cdb5a49
to
badd0a9
Compare
Yes, thanks, I will fix it up - the same is repeated across several theories. In reply to: 2085479313 Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs:991 in 4817390. [](commit_id = 4817390, deletion_comment = False) |
Since we do not expect any differences in behavior for C# 12, consider verifying behavior for that language version too. This is applicable to Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs:1002 in 4817390. [](commit_id = 4817390, deletion_comment = False) |
This error doesn't really allow us to observe the fact that the context is unsafe. Consider using the following body instead: |
|
Done with review pass (commit 16) |
| return (this.Flags.Includes(BinderFlags.UnsafeRegion) || !modifiers.Any(SyntaxKind.UnsafeKeyword)) | ||
| ? this | ||
| : new Binder(this, this.Flags | BinderFlags.UnsafeRegion); | ||
| // In C# 13 and above, iterator bodies define a safe context even when nested in an unsafe context. |
There was a problem hiding this comment.
Directly in iterators it might appear so because using any unsafe constructs is an error anyway, but if we use local functions for example, it can be seen that in C# 12 iterators do not define safe context, for example:
unsafe class C // unsafe context
{
System.Collections.Generic.IEnumerable<int> M() // iterator
{
yield return local();
int local() => sizeof(nint); // allowed, we are in unsafe context
}
}This is a pre-existing spec violation.
| ? this | ||
| : new Binder(this, this.Flags | BinderFlags.UnsafeRegion); | ||
| // In C# 13 and above, iterator bodies define a safe context even when nested in an unsafe context. | ||
| // In C# 12 and below, we keep the behavior that nested iterator bodies (e.g., local functions) |
There was a problem hiding this comment.
Does this mean nested functions within iterator methods, or nested functions that are also iterators, or something else? (It looks like nested iterators are defined in a safe context in C#12 - see sharplab.io.) #Resolved
There was a problem hiding this comment.
nested iterator bodies
Hm, that's a weird phrase, I don't know why I wrote that. In C# 12 basically only nested local functions inherit unsafe context. I will rewrite the comment to say that.
It looks like nested iterators are defined in a safe context in C# 12
You're right, iterators always appear to be in a safe context due to there being errors for unsafe constructs in iterators. But in reality, they do not remove the UnsafeRegion binder flag, so they are in a safe context as can be seen in nested non-iterator local functions (see my comment above).
"SafeContext_"? Same comment for next test. (I don't feel strongly about the names, but it's not clear whether "UnsafeContext_" and "SafeContext_" apply to the use (or not) of the Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs:1369 in 8fc5539. [](commit_id = 8fc5539, deletion_comment = False) |
In reply to: 2086258970 Refers to: src/Compilers/CSharp/Test/Semantic/Semantics/UnsafeTests.cs:1369 in 8fc5539. [](commit_id = 8fc5539, deletion_comment = False) |
Test plan: #72662
Speclet: https://github.com/dotnet/csharplang/blob/main/proposals/ref-unsafe-in-iterators-async.md
Speclet update: dotnet/csharplang#8056
yield returnis disallowed in unsafe blocks.unsafeclasses could never containyield returns as those are now disallowed in unsafe contexts. Plus it would be a breaking change as iterators are allowed in unsafe classes in C# 12.