Skip to content

Fix for UseDeconstruction with default literal#26140

Merged
jcouv merged 3 commits intodotnet:masterfrom
Neme12:useDeconstructionDefaultLiteralFix
Apr 16, 2018
Merged

Fix for UseDeconstruction with default literal#26140
jcouv merged 3 commits intodotnet:masterfrom
Neme12:useDeconstructionDefaultLiteralFix

Conversation

@Neme12
Copy link
Copy Markdown
Contributor

@Neme12 Neme12 commented Apr 13, 2018

fixes #25260

The previous PR #25282 didn't actually fix the issue with a default literal. The test (which asserts that "use deconstruction" isn't offered) actually passes because it is using the default language version - 7.0, which doesn't allow default literals, therefore the code is incorrect. With C# 7.1 enabled, it is offered (and shouldn't be). The implementation is wrong. It is currently checking the type of the expression but a default literal does have an inferred type.

The failure was discovered in #26115 where the test fails when default language version is changed to 8.0.

@Neme12 Neme12 requested a review from a team as a code owner April 13, 2018 15:03
@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 13, 2018

Tagging @jcouv @CyrusNajmabadi @jinujoseph who reviewed the original PR.

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 13, 2018

Can we get this resolved soon so that I don't have to feel bad / ashamed about this for too long? 😄

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 13, 2018

retest windows_release_unit32_prtest please

if (conversion.Exists &&
!conversion.IsIdentity &&
!conversion.IsTupleConversion &&
!conversion.IsTupleLiteralConversion)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

probably would benefit from a comment.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

comment added

@Neme12
Copy link
Copy Markdown
Contributor Author

Neme12 commented Apr 13, 2018

retest windows_debug_vs-integration_prtest please

@jcouv jcouv added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Apr 16, 2018
@jcouv jcouv added this to the 15.8 milestone Apr 16, 2018
@jcouv jcouv merged commit cd10095 into dotnet:master Apr 16, 2018
@Neme12 Neme12 deleted the useDeconstructionDefaultLiteralFix branch April 17, 2018 08:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Deconstruct variable declaration" introduces error.

4 participants