Skip to content

Fix 1749 #1795

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.

Sign up for GitHub

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

Jump to bottom
Merged
dsyme merged 5 commits into from
Nov 22, 2016
Merged

Fix 1749 #1795

dsyme merged 5 commits into from
Nov 22, 2016

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Nov 21, 2016

This fixes the code generation bug #1749

After stripping the lambda being provided for the delegate we also need to bind any discarded unit parameters into the expression bodies. We do that with BindUnitVars

I'll dig into why this changed in F# 4.1 - the code has moved around in a file split which makes it harder to work it out.

@forki The test cases you added needed to be compiled under --optimize-

@dsyme dsyme mentioned this pull request Nov 21, 2016
Closed
@KevinRansom
Copy link
Contributor

KevinRansom commented Nov 21, 2016

Failure: FSharp-Tests-Core+CoreTests.queriesLeafExpressionConvert

Tests Not Run

1) Ignored : FSharp-Tests-Core+CoreTests.genericmeasures-GENERATED_SIGNATURE
incorrect signature file generated, test has been disabled a long time

2) Ignored : FSharp-Tests-Core+CoreTests.libtest-GENERATED_SIGNATURE
incorrect signature file generated, test has been disabled a long time

Errors and Failures

1) Failed : FSharp-Tests-Core+CoreTests.queriesLeafExpressionConvert
Error running command 'D:\j\workspace\release_ci_pa---866fd2c3\tests\fsharp\..\..\release\net40\bin\fsiAnyCpu.exe' with args '-r:System.Core.dll --nowarn:20  --define:INTERACTIVE --maxerrors:1 --abortonerror test.fsx' in directory 'D:\j\workspace\release_ci_pa---866fd2c3\tests\fsharp\core\queriesLeafExpressionConvert'. ERRORLEVEL 1
at TestFramework.checkResult(CmdResult result) in D:\j\workspace\release_ci_pa---866fd2c3\tests\fsharp\test-framework.fs:line 243
at [email protected](CmdResult result) in D:\j\workspace\release_ci_pa---866fd2c3\tests\fsharp\test-framework.fs:line 422
at [email protected](String flags, FSharpList`1 sources) in D:\j\workspace\release_ci_pa---866fd2c3\tests\fsharp\test-framework.fs:line 443
at FSharp-Tests-Core.CoreTests.queriesLeafExpressionConvert() in D:\j\workspace\release_ci_pa---866fd2c3\tests\fsharp\tests.fs:line 940

Run Settings
    RuntimeFramework: V4.0
    WorkDirectory: D:\j\workspace\release_ci_pa---866fd2c3\release\net40\bin
    Verbose: True
    NumberOfTestWorkers: 4

Test Run Summary
    Overall result: Failed
   Tests run: 184, Passed: 183, Errors: 0, Failures: 1, Inconclusive: 0
     Not run: 2, Invalid: 0, Ignored: 2, Explicit: 0, Skipped: 0
  Start time: 2016-11-21 21:46:17Z
    End time: 2016-11-21 22:38:48Z
    Duration: 3150.344 seconds

@forki
Copy link
Contributor

forki commented Nov 21, 2016

I guess we should have both versions then with and without optimization. Just to make sure we don't generate invalid IL

@dsyme
Copy link
Contributor Author

dsyme commented Nov 21, 2016 •
edited
Loading

I guess we should have both versions then with and without optimization. Just to make sure we don't generate invalid IL

@forki Yep, added the debug version as part of this PR

@dsyme dsyme merged commit 234ebff into dotnet:master Nov 22, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dsyme @KevinRansom @forki @msftclas