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

Fix assertion failure in getFollowBlockForLhs() (#206) #210

Closed
wants to merge 1 commit into from
Closed

Fix assertion failure in getFollowBlockForLhs() (#206) #210

wants to merge 1 commit into from

Conversation

niklas88
Copy link
Member

This is currently WiP, the first commit is in preparation for tackling the assertion failure in #206.
Turns out fixing this function looks a bit harder as it's not clear what
to return so the rest doesn't crash. Currently the offending query
triggers the AD_CHECK(false) so we still need to figure out what to do
about it

This is in preparation for tackling the assertion failure in #206.
Turns out fixing this function looks a bit harder as it's not clear what
to return so the rest doesn't crash. Currently the offending query
triggers the AD_CHECK(false) so we still need to figure out what to do
about it
@niklas88 niklas88 requested a review from joka921 March 18, 2019 17:58
@joka921
Copy link
Member

joka921 commented Mar 18, 2019

Checkout #211 which would be my suggestion.

@niklas88
Copy link
Member Author

@joka921 oh great, thanks for your work didn't have the patience yesterday to figure out what to do outside the function yesterday. Can't we combine this change with your changes to scanXXX though? I really don't like this going back one block only to go forward one block a few lines down? I will test this.

@niklas88 niklas88 closed this Mar 19, 2019
@niklas88
Copy link
Member Author

@joka921 nevermind I missed the case where it == _blocks.end() and we indeed go back one block and use that instead of just not going one forward

@niklas88 niklas88 deleted the fix_block_begin_assert_two branch October 1, 2019 15:53
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.

None yet

2 participants