Combine deconstruction assignment and declaration, and support discards (merging to master) #15842
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ds. (dotnet#15548) * Combine deconstruction assignment and declaration, and support discards. - Combine deconstruction assignment and declaration, and support discards. - Add wildcards.work.md to track outstanding work. - Bind each type syntax once in a deconstruction. - Because tuples may contain declarations, adjust lambda disambiguation and adjust parsing of argument lists. - Diagnose tuple element names on the left of a deconstruction. - Add relational operators to disambiguating tokens in 7.5.4.2 * Disallow deconstruction declarations except at the statement level. This is now a semantic restriction (until we decide to remove it). * Revise logic to detect `var` in a declaration expression. Plus other changes per code review. * Add a test that GetTypeInfo on a discard expression doesn't crash. * Small changes per code review. * Add (skipped) test for var invocation in parens. * Rename "Discarded" to "Discard" * Changes recommended via code review. * Minor changes to the handling of declaration expressions per code review. * Addressing blocking feedback while Neal is OOF Fixes dotnet#14794 Fixes dotnet#14832
jcouv
added
Area-Compilers
New Language Feature - Pattern Matching
jcouv
commented
Dec 12, 2016
| @@ -430,14 +430,14 @@ class Variable | |||
| Await state.AssertSelectedCompletionItem(displayText:="as", isSoftSelected:=True) | |||
|
|
|||
| state.SendTypeChars(", va") | |||
| Await state.AssertSelectedCompletionItem(displayText:="var", isSoftSelected:=True) | |||
| Await state.AssertSelectedCompletionItem(displayText:="var", isHardSelected:=True) | |||
There was a problem hiding this comment.
FYI @CyrusNajmabadi Merging Neal's latest parsing changes with the completion tests I had added in master requires those tests to be updated. I think the updated expectations are an improvement.
|
@dotnet/roslyn-compiler This is a merge from |
|
The fix-up LGTM, it is my understanding the the initial commit has been signed-off on earlier. |
|
CC @MattGertz for ask mode approval |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is merging Neal's recent change in the features/wildcard branch (#15548) back into master.
Customer scenario
This parsing fix addresses various ambiguities that would be backcompat issues once we allow mixing of deconstruction declarations and assignments. Those scenarios still report errors for now.
For instance,
(a < b, c > d, e) = ...;. Are there 2 or 3 elements on the left?Bugs this fixes:
Fixes #14794
Fixes #14832
Workarounds, if any
No workaround. If we let customers type such code in C#7.0, we can't change how it gets interpreted in a later version.
Risk
Low. The change is mostly limited to deconstruction code paths.
Performance impact
Low. The parsing has actually become more local (less parsing ahead) which should be a minor improvement.
Is this a regression from a previous update?
No.
Root cause analysis:
This is a ripple effect of our decision to rely on declaration expressions, rather than specialized deconstruction syntax. The declarative expressions change was made due to direct customer feedback about the design of the feature.
How was the bug found?
Know work item (listed in work breakdown #14832)