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

[RFC FS-1069] Implicit yields (allow dropping yield in list, array, sequence and computation expressions) #6806

Merged
merged 41 commits into from
Jul 4, 2019

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented May 22, 2019

Continuation of #6304 from feature branch

This is the implemntation for RFC FS-1069 Implict yields, i.e. the "implicit yields" language suggestion. This allows implicit yield in list, array, sequence and those computation expressions supporting Yield/Combine/Zero/Delay.

See the RFC for notes and discussion

@dsyme
Copy link
Contributor Author

dsyme commented Jun 7, 2019

This is ready up to getting it green

@cartermp
Copy link
Contributor

cartermp commented Jun 7, 2019

@dsyme test failure here: https://dev.azure.com/dnceng/public/_build/results?buildId=216204&view=ms.vss-test-web.build-test-results-tab

(side note: love that @brettfo has this level of integration with our tests set up!)

@dsyme
Copy link
Contributor Author

dsyme commented Jun 14, 2019

side note: love that @brettfo has this level of integration with our tests set up!

@brettfo That is amazing!!

@cartermp Shoud be fixed by latest push

@cartermp
Copy link
Contributor

It's not clear why the 7k line FSComp file is being included here. Is this a clean diff?

@cartermp cartermp added this to the .NET Core 3.0 milestone Jun 17, 2019
@dsyme
Copy link
Contributor Author

dsyme commented Jul 3, 2019

It's not clear why the 7k line FSComp file is being included here. Is this a clean diff?

I cleaned up the diff, thanks

@dsyme
Copy link
Contributor Author

dsyme commented Jul 3, 2019

It also introduces names that are not debuggable; these should change.

What were you referring to for this? thanks!

@dsyme dsyme closed this Jul 3, 2019
@dsyme dsyme reopened this Jul 3, 2019
@dsyme
Copy link
Contributor Author

dsyme commented Jul 3, 2019

We had a transient test failure here, of a kind I've not seen before

Error Message:
 System.ObjectDisposedException : The CancellationTokenSource has been disposed.
Stack Trace:
   at System.Threading.CancellationTokenSource.ThrowObjectDisposedException()
   at FSharp.Compiler.SourceCodeServices.Reactor.SetBackgroundOp(FSharpOption`1 bgOpOpt) in D:\a\1\s\src\fsharp\service\Reactor.fs:line 137
   at FSharp.Compiler.SourceCodeServices.BackgroundCompiler.CheckProjectInBackground(FSharpProjectOptions options, String userOpName) in D:\a\1\s\src\fsharp\service\service.fs:line 827
   at FSharp.Compiler.SourceCodeServices.FSharpChecker.StartBackgroundCompile(FSharpProjectOptions options, FSharpOption`1 userOpName) in D:\a\1\s\src\fsharp\service\service.fs:line 1165
   at Microsoft.VisualStudio.FSharp.LanguageService.FSharpLanguageServiceBackgroundRequests_DEPRECATED.ExecuteBackgroundRequest(FSharpBackgroundRequest_DEPRECATED req, IFSharpSource_DEPRECATED source) in D:\a\1\s\vsintegration\src\FSharp.LanguageService\BackgroundRequests.fs:line 269
   at Salsa.Salsa.Privates.SimpleOpenFile.TryExecuteBackgroundRequest(FSharpBackgroundRequest_DEPRECATED pr) in D:\a\1\s\vsintegration\tests\Salsa\Salsa.fs:line 1129
   at Salsa.Salsa.Privates.SimpleOpenFile.OnIdleTypeCheck() in D:\a\1\s\vsintegration\tests\Salsa\Salsa.fs:line 1158
   at Salsa.Salsa.Privates.SimpleOpenFile.OnIdle() in D:\a\1\s\vsintegration\tests\Salsa\Salsa.fs:line 1115
   at Salsa.Salsa.Privates.SimpleVisualStudio.Salsa-Salsa-IVisualStudio-OnIdle() in D:\a\1\s\vsintegration\tests\Salsa\Salsa.fs:line 848
   at Salsa.Salsa.Privates.SimpleVisualStudio.Salsa-Salsa-IVisualStudio-TakeCoffeeBreak() in D:\a\1\s\vsintegration\tests\Salsa\Salsa.fs:line 856
   at UnitTests.TestLib.LanguageService.LanguageServiceBaseTests.CreateSingleFileProject(FSharpList`1 content, FSharpOption`1 references, FSharpOption`1 defines, FSharpOption`1 fileKind, FSharpOption`1 disabledWarnings, FSharpOption`1 fileName) in D:\a\1\s\vsintegration\tests\UnitTests\TestLib.LanguageService.fs:line 325
   at UnitTests.TestLib.LanguageService.LanguageServiceBaseTests.CreateSingleFileProject(String content, FSharpOption`1 references, FSharpOption`1 defines, FSharpOption`1 fileKind, FSharpOption`1 disabledWarnings, FSharpOption`1 fileName) in D:\a\1\s\vsintegration\tests\UnitTests\TestLib.LanguageService.fs:line 287
   at Tests.LanguageService.AutoCompletion.UsingMSBuild.VerifyDotCompListContainAllAtStartOfMarker(String fileContents, String marker, FSharpList`1 list, FSharpOption`1 addtlRefAssy, FSharpOption`1 coffeeBreak)
   at Tests.LanguageService.AutoCompletion.UsingMSBuild.QueryExpression.DotCompletionSystematic1() in D:\a\1\s\vsintegration\tests\UnitTests\LegacyLanguageService\Tests.LanguageService.Completion.fs:line 7604

@dsyme
Copy link
Contributor Author

dsyme commented Jul 3, 2019

@KevinRansom @TIHan Please advise on adding the /langversion checks for this PR. This needs to be done asap, thanks. I can do it, just let me know.

@KevinRansom
Copy link
Member

@dsyme, there appears to be some cancellation token issue with Linux. A bunch of meregs have failed because of it..

@KevinRansom
Copy link
Member

@dsyme, I had planned to work through all of the PR's doing the langversion change to fgharp47 and fsharp5 is broken at the moment due to some issue with cancellationtokens and linux. Have you seen this failure on a platform other than linux?

…t-yields

Merge master to feature/implicit-yields
@cartermp cartermp dismissed their stale review July 4, 2019 01:26

stale

@KevinRansom KevinRansom closed this Jul 4, 2019
@KevinRansom KevinRansom reopened this Jul 4, 2019
@KevinRansom KevinRansom merged commit cfb414f into release/fsharp47 Jul 4, 2019
@KevinRansom
Copy link
Member

Merging, I will add in the langversion support post merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants