-
Notifications
You must be signed in to change notification settings - Fork 19
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
clean concolic code #351
clean concolic code #351
Conversation
15d3b0c
to
ae24c01
Compare
@chambart it seems to be looping indefinitely when running the tests, I need to investigate 😅 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved with nitpick
@@ -203,7 +185,8 @@ let rec add_trace pc node (trace : trace) to_update : eval_tree list = | |||
| Trap Unreachable -> begin | |||
match node.node with | |||
| Not_explored -> node.node <- Unreachable | |||
| Unreachable -> () | |||
| Unreachable -> | |||
(* it means we explored two times the same branch ! :S *) assert false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(* it means we explored two times the same branch ! :S *) assert false | |
(* This check is not needed for correctness. If it ends up being too | |
annoying to satisfy, especially when going multithreaded, we could | |
remove it *) | |
failwith "Branch shouldn't have been explored twice" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really ? My understanding is that reaching this point means that we did not marked a node as "explored" properly and it's really something we want to avoid, right?
@krtab, I can't merge as the tests are looping indefinitely with this change.. |
Closing in favor of #406. |
Fix #312 and other things