Fix inlining regression: https://github.com/dotnet/fsharp/issues/17161 #17189
Conversation
|
KevinRansom
added
the
NO_RELEASE_NOTES
|
Y'all, you gotta stop merging PRs that are waiting for servicing approval 😅 Now this needs to be reverted again and a new PR prepped so that Tactics can review at today's servicing meeting. |
Reverting PRs back and forth messed up our CI, previous mergws were to revert this change in main. This fixes it. I cancelled the build for insertion. If it's not approved, we will revert it and deal with merges. |
|
The problem is that merging this makes is disappear from the list that Tactics reviews during the meeting - https://tacticsview.azurewebsites.net/ is powered by looking for PRs that are open with the servicing-consider label. The typical process would be to do the work in main, cherry pick to a release branch, get tactics approval on that PR, then merge and let code flow happen to SDK. |
Yes, and it wasn't done like that, hence issues with code flow. Revert was undoing the fix in both 17.11 and main. |
Description
A recent feature impacting the IL Visibility of some F# types had an unintended impact to the F# optimization feature. The optimizer is incorrectly unable to inline some code that has "private" visibility, even when the inlined variables are "in-scope". The fix removes the code that aggressively and incorrectly detects this private visibility.
Regression? (was it working in a previous release or preview?)
Yes
this is a regression, it has been observed internally, as well as by external developers using the 8.0.300 preview SDK.
Risk (see taxonomy)
Low: it removes an unnecessary, incorrect warning.
Testing
New automated regression tests added.
Original issue: #17161
PR into main: 9d05359