Skip to content

Update ref-in-async speclet#8056

Merged
jjonescz merged 13 commits intodotnet:mainfrom
jjonescz:RefInAsync-02
May 2, 2024
Merged

Update ref-in-async speclet#8056
jjonescz merged 13 commits intodotnet:mainfrom
jjonescz:RefInAsync-02

Conversation

@jjonescz
Copy link
Member

No description provided.

@jjonescz jjonescz marked this pull request as ready for review April 17, 2024 11:43
@jjonescz jjonescz requested a review from a team as a code owner April 17, 2024 11:43
AlekseyTs
AlekseyTs previously approved these changes Apr 24, 2024
Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 9)

}
local();
}
partial IEnumerable<int> M5(); // unsafe context, no iterator body
Copy link
Contributor

Choose a reason for hiding this comment

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

unsafe

Does that mean an explicit unsafe will be allowed on the definition part only for iterators with language version 13?

partial IEnumerable<int> M6();
partial unsafe IEnumerable<int> M6() { yield break; }

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, exactly.

@AlekseyTs AlekseyTs dismissed their stale review April 25, 2024 14:18

Obsolete

@AlekseyTs
Copy link
Contributor

Done with review pass (commit 11)

Copy link
Contributor

@AlekseyTs AlekseyTs left a comment

Choose a reason for hiding this comment

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

LGTM (commit 12)

> ~~It is a compile-time error for an iterator block to contain an unsafe context ([§23.2][unsafe-contexts]).
> An iterator block always defines a safe context, even when its declaration is nested in an unsafe context.~~
> **The iterator block used to implement an iterator ([§15.14][iterators])
> always defines a safe context, even when its declaration is nested in an unsafe context.**
Copy link
Contributor

@cston cston Apr 25, 2024

Choose a reason for hiding this comment

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

even when its declaration

Consider using "even when the iterator declaration" to clarify "its" here. #Closed

- The `set` accessor of an iterator property or indexer (i.e., its `get` accessor is implemented via an iterator block)
"inherits" its safe/unsafe scope from the declaration.
- This does not affect partial declarations without implementation as they are only signatures and cannot have an iterator body.
- In C# 12 it is an error to have an iterator method marked with the `unsafe` modifier, but that is allowed in C# 13.
Copy link
Contributor

Choose a reason for hiding this comment

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

In C# 12 it is an error to have an iterator method marked with the unsafe modifier

Why does that follow? From the spec above it sounds like the following is valid regardless of language version:

unsafe IEnumerable F() // unsafe context
{ // safe context
    yield return 42;
}

Copy link
Member Author

@jjonescz jjonescz Apr 26, 2024

Choose a reason for hiding this comment

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

Well, you're right, the C# 12/13 thing does not follow just from the spec but also from this:

The following changes are tied to LangVersion, i.e., C# 12 and lower will continue to disallow
ref-like locals and `unsafe` blocks in async methods and iterators,
and C# 13 will lift these restrictions as described below.
However, spec clarifications which match the existing Roslyn implementation should hold across all LangVersions.

I'm not sure, but I thought the spec is usually not conditional on langversion, the whole spec is for one specific langversion, hence I left this out of the spec itself. Anyway I will clarify this, thanks.

@jjonescz jjonescz merged commit 88342d6 into dotnet:main May 2, 2024
@jjonescz jjonescz deleted the RefInAsync-02 branch May 2, 2024 08:16
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.

3 participants