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: out Variables #44

Closed
wants to merge 20 commits into from
Closed

Conversation

RexJaeschke
Copy link
Contributor

The handling of discards in tuple deconstruction and pattern matching is deferred to the PRs that add support for tuples and pattern matching, respectively. This PR only handles the out parameter and assignment scenarios.

A couple of > TODO placeholders will need attention once those other PRs are created.

@RexJaeschke RexJaeschke mentioned this pull request Jun 19, 2022
@RexJaeschke RexJaeschke changed the title C# 7.x: Discards and out Variables C# 7.x: out Variables Jun 20, 2022
@@ -541,7 +541,7 @@ argument_name
argument_value
: expression
| 'ref' variable_reference
| 'out' variable_reference
| 'out' local_variable_type? identifier
Copy link
Member

Choose a reason for hiding this comment

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

The speclets for out vars and deconstructions relied on declaration_expression.
But in this doc, I see 'out' local_variable_type? identifier and in the deconstruction draft I see tuple_deconstruction_expression.
I'm not sure what is the benefit of this structure.

I think having a declaration_expression that appears in multiple contexts is a good factorization. Then we don't need new syntax for deconstructions (it's declaration_expression and tuple_expression).

Tagging @cston since I'll be away.

Copy link
Member

Choose a reason for hiding this comment

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

Some more context on syntax shape (was discussed with LDM): #285

Copy link
Member

Choose a reason for hiding this comment

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

Updated to use declaration_expression.

Tagging @Nigel-Ecma for grammar checks.

Copy link
Contributor

Choose a reason for hiding this comment

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

declaration_expression is different and cannot simply be substituted in this location. Here local_variable_type is optional, it is not in declaration_expression. I'll find the change and add a suggestion there…

standard/expressions.md Outdated Show resolved Hide resolved
@BillWagner BillWagner marked this pull request as ready for review October 5, 2022 22:18
@BillWagner BillWagner self-assigned this Dec 1, 2022
RexJaeschke and others added 8 commits April 14, 2023 11:02
Some of this text has been moved to PR 596
- Overhauled argument list text
- Added better conversion text
- Removed discards-in-assignment text
@BillWagner
Copy link
Member

BillWagner commented Apr 14, 2023

@jskeet This one is now ready for the upcoming meeting.

Latest edits:

  • rebase
  • update all links
  • remove any discard related text (that has been done in a separate PR.)

@BillWagner BillWagner closed this Apr 14, 2023
@BillWagner BillWagner reopened this Apr 14, 2023
Copy link
Contributor

@Nigel-Ecma Nigel-Ecma left a comment

Choose a reason for hiding this comment

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

I haven't done a full review just looked at the grammar change as requested – unfortunately the change was wrong so needs reverting, suggestion in comment.

standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
standard/expressions.md Outdated Show resolved Hide resolved
@@ -1033,6 +1081,8 @@ Given an implicit conversion `C₁` that converts from an expression `E` to a ty
- `E` exactly matches both or neither of `T₁` and `T₂`, and `T₁` is a better conversion target than `T₂` ([§12.6.4.6](expressions.md#12646-better-conversion-target))
- `E` is a method group ([§12.2](expressions.md#122-expression-classifications)), `T₁` is compatible ([§20.4](delegates.md#204-delegate-compatibility)) with the single best method from the method group for conversion `C₁`, and `T₂` is not compatible with the single best method from the method group for conversion `C₂`

The conversion from an implicitly-typed out variable declaration is not considered better than any other conversion from expression.
Copy link
Contributor

Choose a reason for hiding this comment

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

While I know what this means, it doesn't feel terribly clear - maybe we just need a reference to the "conversion from expression" section?

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 also not sure it adds anything. Should this paragraph be removed? For discussion.

standard/lexical-structure.md Outdated Show resolved Hide resolved
@@ -6051,7 +6101,7 @@ assignment_operator

The left operand of an assignment shall be an expression classified as a variable, a property access, an indexer access, an event access or a tuple. A declaration expression is not directly permitted as a left operand, but may occur as a step in the evaluation of a deconstructing assignment.

The `=` operator is called the ***simple assignment operator***. It assigns the value or values of the right operand to the variable, property, indexer element or tuple elements given by the left operand. The left operand of the simple assignment operator shall not be an event access (except as described in [§15.8.2](classes.md#1582-field-like-events)). The simple assignment operator is described in [§12.21.2](expressions.md#12212-simple-assignment).
The `=` operator is called the ***simple assignment operator***. When the left operand is not a discard (§9.2.8.1), this operator assigns the value of the right operand to the variable, property, or indexer element given by the left operand. Otherwise, the right operand is evaluated with its value going unused. The left operand of the simple assignment operator shall not be an event access (except as described in [§15.8.2](classes.md#1582-field-like-events)). The simple assignment operator is described in [§12.21.2](expressions.md#12212-simple-assignment).
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this change and the next one intended to be in this PR? It's not really about out parameters... was it just something else that should have been in a discards PR?

Copy link
Member

Choose a reason for hiding this comment

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

That's correct. This PR in its original form included both discards and out parameters. I left this change in place because it wasn't in the updated discards and tuples PR.

Let's discuss at the meeting if it should be removed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Action from 2023-04-19: @BillWagner to remove from this PR, but add it to the issue that @MadsTorgersen has already created for polishing on discards.

standard/lexical-structure.md Outdated Show resolved Hide resolved
@BillWagner BillWagner added the meeting: discuss This issue should be discussed at the next TC49-TG2 meeting label Apr 19, 2023
Copy link
Contributor

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Some simple final changes, then let's merge.

standard/expressions.md Outdated Show resolved Hide resolved
@@ -6051,7 +6101,7 @@ assignment_operator

The left operand of an assignment shall be an expression classified as a variable, a property access, an indexer access, an event access or a tuple. A declaration expression is not directly permitted as a left operand, but may occur as a step in the evaluation of a deconstructing assignment.

The `=` operator is called the ***simple assignment operator***. It assigns the value or values of the right operand to the variable, property, indexer element or tuple elements given by the left operand. The left operand of the simple assignment operator shall not be an event access (except as described in [§15.8.2](classes.md#1582-field-like-events)). The simple assignment operator is described in [§12.21.2](expressions.md#12212-simple-assignment).
The `=` operator is called the ***simple assignment operator***. When the left operand is not a discard (§9.2.8.1), this operator assigns the value of the right operand to the variable, property, or indexer element given by the left operand. Otherwise, the right operand is evaluated with its value going unused. The left operand of the simple assignment operator shall not be an event access (except as described in [§15.8.2](classes.md#1582-field-like-events)). The simple assignment operator is described in [§12.21.2](expressions.md#12212-simple-assignment).
Copy link
Contributor

Choose a reason for hiding this comment

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

Action from 2023-04-19: @BillWagner to remove from this PR, but add it to the issue that @MadsTorgersen has already created for polishing on discards.

standard/lexical-structure.md Outdated Show resolved Hide resolved
BillWagner and others added 2 commits April 20, 2023 10:58
Co-authored-by: Jon Skeet <jonskeet@google.com>
Co-authored-by: Nigel-Ecma <perryresearch@zoot.net.nz>
@MadsTorgersen
Copy link
Contributor

To the best of my analysis, this PR is fully replaced by #664.

@jskeet
Copy link
Contributor

jskeet commented May 17, 2023

Closing as obsolete.

@jskeet jskeet closed this May 17, 2023
@jskeet jskeet removed meeting: discuss This issue should be discussed at the next TC49-TG2 meeting Review: pending Proposal is available for review labels May 17, 2023
@RexJaeschke RexJaeschke deleted the RexJaeschke-v7-discards branch June 9, 2023 11:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: 🏗 In progress
Status: 🔖 Ready
Development

Successfully merging this pull request may close these issues.

7 participants