Skip to content

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented Dec 4, 2015

This fixes bug #740

As can be seen from the implementation, the restriction checking for lambdas with an implicit "base" variable (which allows base calls) were too strong - if the base variable is not used then the lambda can be propagated, which allows the inline keyword to be respected.

Suggestions for further testing welcome.

@abelbraaksma
Copy link
Contributor

Since the original bug was reported by me, I wouldn't mind checking the fix, but I'm not quite sure where to start. I'd assume I need a copy of this commit (easy enough), compile and then somehow use it to compile the projects / tests I have that showed exemplified this problem. My guess is the typical way to do this is by taking the compiled binaries and call fsc.exe directly? Or can I configure MSBuild to use another location for fsc?

Sorry if this sounds basic, but I've been a consumer of F# until now, as opposed to being a contributor (other than reporting a handful of bugs).

@KevinRansom
Copy link
Contributor

@abelbraaksma Take a look at the test cases here: https://github.com/Microsoft/visualfsharp/tree/master/tests/fsharpqa/Source/Optimizations/Inlining

If you can add your test cases here then it will be in about the right place.

Thank you for helping out

Kevin

@abelbraaksma
Copy link
Contributor

According to @mpetruska, this PR also fixes #614, next to #740, as he mentions in that issue.

@KevinRansom
Copy link
Contributor

@dsyme Is this PR ready to pull?

@dsyme
Copy link
Contributor Author

dsyme commented Jan 24, 2016

Yes, it's ready

@KevinRansom
Copy link
Contributor

Thanks for this contribution

KevinRansom added a commit that referenced this pull request Feb 25, 2016
Fix inlining on subtypes
@KevinRansom KevinRansom merged commit 0f925ac into dotnet:master Feb 25, 2016
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