Fix incorrect tuple handling #2763
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I don't know the compiler well and I went off what I knew and believe I found the root cause for our tuple bugs that started with 9222f08. Which was in turn fixing #2735. However, a number of bugs came from that fix because, it didn't fix the root issue. Which was `gen_localdecl` returning GEN_NOVALUE where GEN_NOTNEEDED was needed. This is why, prior to 9222f08, `gen_tuple` was returning GEN_NOTNEEDED when the value it has was GEN_NOVALUE. Fixes #2760
This was referenced Jun 8, 2018
Closed
SeanTAllen
added
the
changelog - fixed
|
I see that your change basically reverts 9222f08, but keeps the regression test from it, which is still passing. This confused me at first, so I went to look at the other code that had been changed. It seems like #2757 is responsible for the real fix here that 9222f08 was trying to acheive, so it makes sense to me now. |
jemc
approved these changes
Jun 9, 2018
|
@jemc yeah, its a two parter. i fixed one bug with the first commit but it left other bugs. took me a while to figure out what was going on. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I don't know the compiler well and I went off what I knew and believe I
found the root cause for our tuple bugs that started with 9222f08. Which
was in turn fixing #2735.
However, a number of bugs came from that fix because, it didn't fix the
root issue. Which was
gen_localdeclreturning GEN_NOVALUE whereGEN_NOTNEEDED was needed. This is why, prior to 9222f08,
gen_tuplewasreturning GEN_NOTNEEDED when the value it has was GEN_NOVALUE.
Fixes #2760