-
Notifications
You must be signed in to change notification settings - Fork 274
API change: from_integer for negative naturals #3016
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
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 think this is good, except the use of PRECONDITION
feels a bit weird and I'd thus suggest changes as below.
src/util/arith_tools.cpp
Outdated
} | ||
else if(type_id==ID_pointer) | ||
{ | ||
if(int_value==0) | ||
return null_pointer_exprt(to_pointer_type(type)); | ||
else | ||
PRECONDITION(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.
I think this should be written as
PRECONDITION(int_value == 0);
return null_pointer_exprt(to_pointer_type(type));
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
src/util/arith_tools.cpp
Outdated
@@ -158,11 +152,15 @@ constant_exprt from_integer( | |||
return false_exprt(); | |||
else if(int_value==1) | |||
return true_exprt(); | |||
else | |||
PRECONDITION(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.
I'd do:
PRECONDITION(int_value == 0 || int_value == 1);
if(int_value == 0)
return false_exprt();
else
return true_exprt();
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
This changes behavior for the case that from_integer is asked to produce a natural number that is negative. Previously, a malformed constant_exprt with nil id was returned; now a precondition fails.
a53b7a5
to
92eab68
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.
This PR failed Diffblue compatibility checks (cbmc commit: a53b7a5).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/85596705
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
Common spurious failures:
- the cbmc commit has disappeared in the mean time (e.g. in a force-push)
- the author is not in the list of contributors (e.g. first-time contributors).
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.
Passed Diffblue compatibility checks (cbmc commit: 92eab68).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/85599486
This changes behavior for the case that from_integer is asked to produce a
natural number that is negative. Previously, a malformed constant_exprt
with nil id was returned; now a precondition fails.