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

Ensures cached lambda has matching return type for UnboundLambda binding #14755

Merged
merged 1 commit into from Oct 27, 2016

Conversation

@phil-scott-78
Copy link
Contributor

phil-scott-78 commented Oct 26, 2016

Cached lambdas were being used in situations where the return type was different. This forces building a new lambda body and binding for those cases.

Honestly I'm not 100% sure if this issue is the use of the cached value, or if the caching itself needs to be tweaked. This broke in commit 6c9e186 by @cston which touched quite a bit in this area so tracking down the precise issue might be beyond me.

This would resolve #14722

Cached lambdas were being used in situations where the return type was
different. This forces building a new lambda body and binding for those
cases.
@cston

This comment has been minimized.

Copy link
Member

cston commented Oct 26, 2016

@enkafan Thanks for identifying the bug and for providing the fix!

It's likely we'll want to fix this in the dev15-rc branch. (Changes to dev15-rc will be merged automatically into master.) If you'd like to cherry-pick the commit to dev15-rc, please feel free. Or alternatively, I can migrate the fix to that branch after this PR against master is approved and merged.

@cston

This comment has been minimized.

Copy link
Member

cston commented Oct 27, 2016

@dotnet/roslyn-compiler please review.

@cston

This comment has been minimized.

Copy link
Member

cston commented Oct 27, 2016

LGTM. Thanks for fixing this!

@phil-scott-78

This comment has been minimized.

Copy link
Contributor Author

phil-scott-78 commented Oct 27, 2016

Cool, thanks for the feedback. Seems things are in motion to get this into master so unless @cston thinks otherwise I'll hang tight and let this change get moved from there to the dev15-rc branch rather than cherry-picking it myself.

@cston cston merged commit cc7e706 into dotnet:master Oct 27, 2016
9 checks passed
9 checks passed
linux_debug_prtest Build finished.
Details
microbuild_prtest Build finished.
Details
perf_correctness_prtest Build finished.
Details
windows_debug_unit32_prtest Build finished.
Details
windows_debug_unit64_prtest Build finished.
Details
windows_determinism_prtest Build finished.
Details
windows_eta_open_prtest Build finished.
Details
windows_release_unit32_prtest Build finished.
Details
windows_release_unit64_prtest Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.