Fixing bugs in C# TypeInferrer by Neme12 · Pull Request #30211 · dotnet/roslyn

Neme12 · 2018-09-28T12:46:06Z

This PR simplifies the C# TypeInferrer and fixes 3 bugs along the way. The vast majority of additions are new tests. Can be reviewed commit by commit to see more granular changes and each of the 3 fixes individually.

Fixes #30232
Fixes #30235
Fixes #27647

…xpression

…eLambdaExpression

It seems that removing it doesn't cause any failures, so the branch is currently untested. Adding 2 enum completion tests which would now fail if the condition was removed (NotInSimpleLambdaAfterAsync and NotInParenthesizedLambdaAfterAsync) + a few other tests.

Fixes dotnet#30232

Fixes dotnet#30235

Fixes dotnet#27647

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs

CyrusNajmabadi · 2018-10-01T19:47:43Z

src/EditorFeatures/CSharpTest/Diagnostics/GenerateMethod/GenerateMethodTests.cs

    static void Main(string[] args)
    {
-        Func<int, int> f = x => Goo(x);
+        Func<string, int> f = x => Goo(x);


these tests seem to have changed? is thatbecause new similar tests were added? if not, can we restore the tests to the original state and add new tests?

I changed it so that the return type is different than the parameter type. This would help catch bugs if they got mixed up during the generation.

can you do that as new tests?

If you don't like this change, I'd prefer to revert it instead. I don't think adding a new one adds any value...

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs

… statement

Neme12 · 2018-10-03T17:08:04Z

@sharwell This PR may look huge, but it's really just a huge amount of new tests. I hope it doesn't take much of your time to review this. Thanks.

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs

CyrusNajmabadi

Could use a little clarity/comments in one location. But otherwise looks great!

Neme12 · 2018-10-10T15:19:06Z

retest windows_debug_vs-integration_prtest please

Neme12 · 2018-10-10T15:19:15Z

retest windows_release_vs-integration_prtest please

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs

sharwell · 2018-10-16T23:48:32Z

src/EditorFeatures/CSharpTest/TypeInferrer/TypeInferrerTests.cs


-        [Fact, Trait(Traits.Feature, Traits.Features.TypeInferenceService)]
-        [WorkItem(4486, "https://github.com/dotnet/roslyn/issues/4486")]
-        public async Task TestReturnInAsyncLambda1()


📝 Covered by TestReturnInAsyncTaskOfTParenthesizedLambda

sharwell · 2018-10-16T23:49:08Z

src/EditorFeatures/CSharpTest/TypeInferrer/TypeInferrerTests.cs

-
-        [Fact, Trait(Traits.Feature, Traits.Features.TypeInferenceService)]
-        [WorkItem(4486, "https://github.com/dotnet/roslyn/issues/4486")]
-        public async Task TestReturnInAsyncLambda2()


📝 Covered by TestReturnInAsyncTaskOfTAnonymousMethod

Changing InferTypeInLambdaExpression to InferTypeInAnonymousFunctionE…

67de9b0

…xpression

Neme12 requested a review from a team as a code owner September 28, 2018 12:46

Neme12 added 5 commits September 28, 2018 14:48

Using InferTypeInAnonymousFunctionExpression for anonymous methods

06e896e

Merging identical branches

ea0d3e0

Merging InferTypeInParenthesizedLambdaExpression and InferTypeInSimpl…

68dce91

…eLambdaExpression

Adding expression tree tests

6ab5672

jinujoseph added Area-IDE Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Sep 28, 2018

jinujoseph assigned sharwell Sep 28, 2018

Neme12 force-pushed the typeInferrer branch from 960d675 to 6ab5672 Compare September 28, 2018 20:16

Neme12 added 2 commits September 29, 2018 17:47

Adding more TypeInferrerTests

e2cca85

Fix to unwrap Task in async expression lambda

af1b5da

Fixes dotnet#30232

Neme12 force-pushed the typeInferrer branch from 7ddebc9 to af1b5da Compare September 29, 2018 17:13

Neme12 added 2 commits September 29, 2018 21:44

Adding more tests with return statements in async methods

5639005

Fix for yield return in iterator local function

037b68f

Fixes dotnet#30235

Neme12 force-pushed the typeInferrer branch from 40c5e7b to 037b68f Compare September 29, 2018 21:12

Neme12 added 7 commits September 30, 2018 00:10

Fix to unwrap Task in async expression bodied methods

992ebec

Fixes dotnet#27647

Extracting common code into a new method

69a752f

Using IsReturnableConstruct

59abf2e

Adding feature tests for generating from async expression bodied methods

9f29e9c

Adding feature tests for generating from async expression lambdas

50e592c

Adding more tests

81614bd

Fix to unwrap non-generic Task to void

f0ec22a

Neme12 changed the title ~~[WIP] Simplifying TypeInferrer~~ Simplifying TypeInferrer Sep 30, 2018

Neme12 commented Sep 30, 2018

View reviewed changes

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs Show resolved Hide resolved

Neme12 changed the title ~~Simplifying TypeInferrer~~ Fixing bugs in C# TypeInferrer Sep 30, 2018

jcouv mentioned this pull request Sep 30, 2018

[Umbrella] Work items and test plan for async streams #24037

Closed

CyrusNajmabadi reviewed Oct 1, 2018

View reviewed changes

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs Outdated Show resolved Hide resolved

CyrusNajmabadi reviewed Oct 1, 2018

View reviewed changes

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs Outdated Show resolved Hide resolved

Neme12 added 2 commits October 3, 2018 10:33

Rewriting UnwrapTaskLike using if statements

638f14d

Fixing TypeInferrer crash with return or yield return as a global…

7003dfe

… statement

CyrusNajmabadi reviewed Oct 3, 2018

View reviewed changes

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs Show resolved Hide resolved

CyrusNajmabadi reviewed Oct 3, 2018

View reviewed changes

src/Workspaces/CSharp/Portable/LanguageServices/CSharpTypeInferenceService.TypeInferrer.cs Outdated Show resolved Hide resolved

CyrusNajmabadi approved these changes Oct 3, 2018

View reviewed changes

Renaming & adding a comment

220ce00

sharwell approved these changes Oct 16, 2018

View reviewed changes

sharwell added this to the 16.0.P2 milestone Oct 16, 2018

sharwell merged commit 0949ca8 into dotnet:master Oct 17, 2018

sharwell mentioned this pull request Oct 17, 2018

Handle task-like types in type inference #30559

Closed

Neme12 deleted the typeInferrer branch October 17, 2018 15:32

Neme12 mentioned this pull request Nov 8, 2018

Fixing inferred type for yield return in async iterator methods #31046

Merged

Neme12 mentioned this pull request Jan 13, 2019

async function generates wrong return #32429

Closed

Conversation

Neme12 commented Sep 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

CyrusNajmabadi Oct 1, 2018

Choose a reason for hiding this comment

Uh oh!

Neme12 Oct 1, 2018

Choose a reason for hiding this comment

Uh oh!

CyrusNajmabadi Oct 1, 2018

Choose a reason for hiding this comment

Uh oh!

Neme12 Oct 1, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Neme12 commented Oct 3, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CyrusNajmabadi left a comment

Choose a reason for hiding this comment

Uh oh!

Neme12 commented Oct 10, 2018

Uh oh!

Neme12 commented Oct 10, 2018

Uh oh!

Uh oh!

sharwell Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

sharwell Oct 16, 2018

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Neme12 commented Sep 28, 2018 •

edited

Loading

Neme12 commented Oct 3, 2018 •

edited

Loading