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

undo #3536 #3746

Merged
merged 2 commits into from
Oct 13, 2017
Merged

undo #3536 #3746

merged 2 commits into from
Oct 13, 2017

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented Oct 13, 2017

@cartermp Bug #3743 is a regression

This disables the optimization that causes the regression #3536. The testing of the optimization was flawed, in particular the code https://github.com/Microsoft/visualfsharp/pull/3536/files#diff-ebddd0dd8ddd9195acef989b49ecfb67R542 has x > 0 when it should have x = 0 which means that the recursive calls were not being made in the tests.

We should prioritize this as much as possible, e.g. into 15.5 escrow if that's feasible

thanks
don

@cartermp
Copy link
Contributor

cc @Pilchie

This fixes a regression in the F# compiler that has broken people. I'll file the internal bug for shiproom.

@KevinRansom KevinRansom merged commit 503d167 into dotnet:master Oct 13, 2017
@brettfo brettfo mentioned this pull request Oct 13, 2017
brettfo added a commit that referenced this pull request Oct 13, 2017
KevinRansom pushed a commit to KevinRansom/fsharp that referenced this pull request Oct 18, 2017
@cartermp
Copy link
Contributor

cartermp commented Nov 3, 2017

@dsyme Can we publish the latest fix here to https://www.nuget.org/packages/FSharp.Compiler.Tools/ ?

@dsyme
Copy link
Contributor Author

dsyme commented Nov 3, 2017

@cartermp yes, will do

@cartermp
Copy link
Contributor

cartermp commented Nov 3, 2017 via email

@vasily-kirichenko
Copy link
Contributor

4.1.29 was published and it does not have this bug. Thanks!

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.

4 participants