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

C# 7.x: ref locals and returns #213

Merged
merged 24 commits into from
May 19, 2023
Merged

Conversation

RexJaeschke
Copy link
Contributor

This (Draft) PR is not quite complete.

@RexJaeschke RexJaeschke added this to the C# 7.x milestone Jan 24, 2021
@RexJaeschke RexJaeschke marked this pull request as ready for review June 1, 2021 20:19
@RexJaeschke
Copy link
Contributor Author

Completed edits. Removed Draft status.

@KalleOlaviNiemitalo
Copy link
Contributor

Missing:

  • => ref in method_body and similar. However, the draft-v7 branch does not even allow => there yet; C# 6.0 feature: expression bodied members #4 was merged to draft-v6 but those changes are not in draft-v7. C# 7.x: Adding more support for expression-bodied members #69 does not add them, either.

  • ref return type in property_declaration and indexer_declaration. Roslyn allows these, and at least System.Span<T> and System.Span<T>.Enumerator have public members like that.

  • ref return type in operator_declaration. AFAIK, Roslyn does not allow this, so perhaps you'll decide to leave this out, even though I don't foresee any specification problem in allowing this in unary_operator_declarator and binary_operator_declarator. In contrast, conversion_operator_declarator converting to a reference seems like it would require changes elsewhere in the standard.

@KalleOlaviNiemitalo
Copy link
Contributor

As I wrote in #169 (comment), the standard should describe the runtime array element type checks required for ref locals and returns. Moreover, it should describe how those checks are skipped when ref readonly is used. See also #264 (comment) regarding type checks for in parameters.

@RexJaeschke RexJaeschke added the Review: pending Proposal is available for review label Sep 16, 2021
@333fred
Copy link
Member

333fred commented Sep 27, 2021

One thing I don't see in this spec, @RexJaeschke, is how to handle a case like dotnet/roslyn#53113 (forgive me if it's already subsumed by other parts of the spec; if it is, I couldn't find what does). Based on how we handle M(ref *(int*)0), it seems like such code should be allowed and should not dereference the pointer, but I didn't see anything in the ref locals section talking about it.

@BillWagner BillWagner marked this pull request as draft February 3, 2022 15:21
@BillWagner
Copy link
Member

Converted to draft status (C# 7 feature)

@BillWagner BillWagner force-pushed the RexJaeschke-patch-1 branch 2 times, most recently from 430fef4 to f3426b7 Compare April 2, 2022 23:10
@KalleOlaviNiemitalo
Copy link
Contributor

If #104 is merged before this, I suppose this will need to add => ref for local functions, too.

@gafter
Copy link
Member

gafter commented Jul 19, 2022

This still needs to safety rules to be incorporated. See #334 and https://github.com/dotnet/csharplang/blob/standard-proposals/proposals/csharp-7.2/span-safety.md#draft-language-specification . Those safety rules are for ref return, ref locals, and ref structs.

@BillWagner BillWagner merged commit ee0115c into v7-ref-features May 19, 2023
@BillWagner BillWagner deleted the RexJaeschke-patch-1 branch May 19, 2023 18:43
BillWagner added a commit that referenced this pull request Jun 7, 2023
* Add rules for `ref` safety (#742)

* first pass at safety rules

This mostly incorporates the feature spec language.

* Add rule for `out` parameters

Out parameters have a ref safe to escape scope of the current method, not the calling method.

* Add readonly rule.

* fix headers

no code fences in headers

* Apply suggestions from code review

Co-authored-by: Jon Skeet <skeet@pobox.com>

* updates from reviews

Updates from code review

* style fix

* respond to feedback

Ref like fields do have storage, but that storage may refer to a variable that is a struct parameter or varialbe.

* respond to feedback

Respond to existing feedback.

* Introduce definitions

Introduce better definitions for the "lifetime" of reference variables. I avoided the term "lifetime' because of its runtime connotation. Instead, I used the "scope" where a "variable declaration" is valid.

From there, next define the safe scopes and the ref safe scopes for different variable classifications.

Finally, I shortened the terms *safe-to-escape-scope* and *ref-safe-to-escape-scope* to "*safe-scope* and *ref-safe-scope*. The text doesn't make it clear why the "escape" term is used, and it doesn't seem to add clarity.

* add examples

* respond to feedback.

* Apply suggestions from code review

Co-authored-by: Jon Skeet <skeet@pobox.com>

* address review comments

This addresses most of the comments in the most recent reviews.

The next commit will make an attempt to use a single term for *ref_safe_scope*.

* Remove *safe_scope* rules

Once I push these, I'll add notes about which rules must be added to #601

* Update standard/variables.md

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* Update standard/variables.md

* Respond to review comments.

* Feedback from April meeting

This include comments, and fixing the formatting after consulting with Rex.

* remove ref struct descriptions

These belong in the PR for ref structs, and in the section on ref structs.

* Apply suggestions from code review

Co-authored-by: Jon Skeet <skeet@pobox.com>

* respond to feedback.

* Apply suggestions from code review

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* remove missing xref

This needs to be added to #213 once this PR is merged.

* Updates from 5/17 meeting.

Updates from the 5/17 meeting.

* fix build warning

---------

Co-authored-by: Jon Skeet <skeet@pobox.com>
Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* C# 7.x: ref struct (#601)

* Split out `ref struct` from #33.

* Add ref safety rules as they apply to ref structs

Declare that `ref structs` have a *ref_safe_scope* that matches their initializing expressions. Restrict copying of a `ref struct` (by value) to its *ref_safe_scope*. Then, define what the *ref_safe_scope* is depending on the initializing expression.

* forgot to finish one sentence.

* Add note on iterators and async methods

The previous normative language was

* respond to review feedback.

* Respond to meeting feedback, part 1

Respond to all meeting feedback *except* updating the rules on *safe_to_escape*.

That's coming in the next commit.

* incorporate safe rules.

Pull all safety rules related to safe-scope from PR on ref variables
into the section on ref structs.

* respond to feedback.

* Apply suggestions from code review

Co-authored-by: Jon Skeet <jonskeet@google.com>

* respond to feedback.

* Apply suggestions from code review

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* respond to feedback.

* Update per 5/17 committee meeting.

* update definition

* found one more comment to address

---------

Co-authored-by: Neal Gafter <nmgafter@fb.com>
Co-authored-by: Bill Wagner <wiwagn@microsoft.com>
Co-authored-by: Jon Skeet <jonskeet@google.com>
Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* C# 7.x: ref locals and returns (#213)

* Update expressions.md

* Update statements.md

* Update classes.md

* Update delegates.md

* Relocate ('ref' 'readonly'?)? to local_variable_declaration

I had this grammar extension in the wrong place

* Add support for ref readonly iteration variables to foreach

* Minor tweak to v7 spec for ? ref : ref

* build fixes

* fix merge error

* one more build issue

* light editing

* fix section references

* fix link errors

* edit to address feedback, link to ref safety rules.

* fix markdown lint error

* Apply suggestions from code review

Co-authored-by: Jon Skeet <jonskeet@google.com>

* respond to feedback

* respond to feedback.

* Apply suggestions from code review

Co-authored-by: Jon Skeet <jonskeet@google.com>

* Update per April meeting feedback.

* formatting while checking grammar

* updates from 5/17 committee meeting.

* remove blank line.

* fix warnings

---------

Co-authored-by: Bill Wagner <wiwagn@microsoft.com>
Co-authored-by: Jon Skeet <jonskeet@google.com>

* C# 7.x: in parameter mode (#219)

* Update basic-concepts.md

* Update variables.md

* Update conversions.md

* Update structs.md

* Update interfaces.md

* Update delegates.md

* Update unsafe-code.md

* Update documentation-comments.md

* Update classes.md

* Update expressions.md

* include support for local functions

* fix merge tag

* fix build warnings

* build fixes

* build fixes, round 1

* build fixes, part 2

* one last link fix

* light edits based on earlier feedback.

* respond to remaining feedbac,.

* one final edit....

* clarification on `ref` extension methods

* Apply suggestions from code review

Co-authored-by: Jon Skeet <jonskeet@google.com>

* respond to feedback

* Update standard/expressions.md

Co-authored-by: Jon Skeet <jonskeet@google.com>

* Update standard/expressions.md

* Apply suggestions from code review

Co-authored-by: Jon Skeet <jonskeet@google.com>

* Update per April meeting notes.

* respond to feedback.

* Exclude dynamic implicit conversions

A dynamic expression can't be passed as an `in` parameter if an implicit conversion is required.

* Clarify restrictions on `in` parameters

Dynamically bound expressions can't use the `in` modifier.

* Apply suggestions from code review

Co-authored-by: Neal Gafter <neal@gafter.com>

* Updates from 5/17 committee meeting.

* fix warning

---------

Co-authored-by: Bill Wagner <wiwagn@microsoft.com>
Co-authored-by: Jon Skeet <jonskeet@google.com>
Co-authored-by: Neal Gafter <neal@gafter.com>

* C# 7.x: Add initializer list to `stackalloc` (#238)

* Update unsafe-code.md

* Update unsafe-code.md

* Move most of stackalloc spec from unsafe to here

* Impact of moving most of stackalloc spec from unsafe to expressions

* Moved most of stackalloc spec to expressions

* Fix links to new stackalloc spec location

* Add Span & ReadOnlySpan types

* fix build issues

* address feedback

* Stack initializers are only allowed as local variable initializers

This clarifies and simplifies some of the language for this PR.

* fix markdown lint issue

* respond to feedback.

* fix build issues

* one more round of build issues

* respond to feedback.

* decisions from 5/17 meeting.

* add safe context rules.

---------

Co-authored-by: Bill Wagner <wiwagn@microsoft.com>

* fix build warnings

* Apply suggestions from code review

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* fix references

* add closing backticks

One grammar rule was missing the closing backticks.

This caused several build errors.

* respond to feedback through clause 15 (classes)

This commit responds to feedback with the 👍 emoji through clause 15.

* respond to feedback.

* Apply suggestions from code review

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* rearrange conditional specification

* Update standard/expressions.md

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* Update standard/variables.md

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* edits based on feedback.

* Update standard/expressions.md

Co-authored-by: Jon Skeet <jonskeet@google.com>

* respond to feedback comments

* Apply suggestions from code review

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* more feedback

* Update standard/classes.md

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* Update standard/classes.md

Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>

* Update standard/expressions.md

* Update standard/expressions.md

* Update standard/expressions.md

Co-authored-by: Bill Wagner <wiwagn@microsoft.com>

* Update standard/variables.md

* edits during the meeting.

* Update standard/expressions.md

Co-authored-by: KalleOlaviNiemitalo <kon@iki.fi>

* edits, part 1

* respond to meeting discussion and feedback

Address all remaining conversations for this PR.

Addresses feedback from 06/05/2023 ECMA committee meeting.

---------

Co-authored-by: Jon Skeet <skeet@pobox.com>
Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>
Co-authored-by: Neal Gafter <neal@gafter.com>
Co-authored-by: Neal Gafter <nmgafter@fb.com>
Co-authored-by: Jon Skeet <jonskeet@google.com>
Co-authored-by: Rex Jaeschke <rex@RexJaeschke.com>
Co-authored-by: KalleOlaviNiemitalo <kon@iki.fi>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meeting: discuss This issue should be discussed at the next TC49-TG2 meeting Review: pending Proposal is available for review
Projects
No open projects
Status: 🏗 In progress
Status: 🏗 In progress
Development

Successfully merging this pull request may close these issues.

6 participants