-
Notifications
You must be signed in to change notification settings - Fork 277
Add {binary,unary_overflow_exprt} #5897
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
Conversation
daedb6e
to
9497b06
Compare
Codecov Report
@@ Coverage Diff @@
## develop #5897 +/- ##
========================================
Coverage 73.09% 73.10%
========================================
Files 1428 1428
Lines 155075 155102 +27
========================================
+ Hits 113357 113388 +31
+ Misses 41718 41714 -4
Continue to review full report at Codecov.
|
Create a dedicated exprt to construct overflow expressions for improved type checking.
9497b06
to
5f3a55d
Compare
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... this has the feel of "why didn't we always do it like this" which is a good sign.
I hate to be that person but ... what about divide? Specifically INT_MIN / -1
.
|
||
if(expr.operands().size()>=3) | ||
// The overflow checks are binary. | ||
for(std::size_t i = 1; i < expr.operands().size(); i++) |
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 is not a request for this PR but more a general observation that this code seems to be not covered by the regression tests. I have a feeling that at one point Matt had tests for this code. Maybe @cristina-david could help?
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.
We have regression/acceleration, it just seems the script in there needs some love to make this work. Adding this to my to-do list.
I'm also having horrible thoughts about what happens with signs when taking modulo. Is it possible to get an overflow with INT_{MAX,MIN} % INT_{MAX,MIN} ? |
We do generate some of these checks in |
Create a dedicated exprt to construct overflow expressions for improved
type checking.