-
Notifications
You must be signed in to change notification settings - Fork 34
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
Remove redundant check from drop_operand #429
Conversation
@@ -119,7 +119,7 @@ inline void drop_operand( | |||
static_cast<int>(operand_stack.size()) < frame.parent_stack_height + 1) | |||
throw validation_error{"stack underflow"}; | |||
|
|||
if (frame.unreachable && static_cast<int>(operand_stack.size()) == frame.parent_stack_height) |
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.
This check was redundant, becase the case of reachable frame + stack equal to parent stack is eliminated by the stack underflow condition above.
In other words, by this point not having a value on stack is possible only if it's unreachable.
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.
Why not leave an assertion here?
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.
I considered assertion, but it just seems too obvious, because the previous check implies it's true.
"(frame unreachable OR no stack underflow) AND stack underflow" implies "frame unreachable".
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.
Yeah if you read it carefully, but I think an assertion is much more explicit.
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.
Ok, will add an assertion.
Codecov Report
@@ Coverage Diff @@
## master #429 +/- ##
=======================================
Coverage 99.32% 99.32%
=======================================
Files 50 50
Lines 13989 13992 +3
=======================================
+ Hits 13894 13897 +3
Misses 95 95 |
c4de939
to
10b32f9
Compare
No description provided.