-
Notifications
You must be signed in to change notification settings - Fork 273
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
Partial cleanup in solver code #1810
Closed
romainbrenguier
wants to merge
10
commits into
diffblue:develop
from
romainbrenguier:refactor/prop_conv
Closed
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
79cb900
Make literal return an optional
romainbrenguier fd7af72
Make get_bool return a optional
romainbrenguier 16ae7dd
Refactoring set_equality_to_true
romainbrenguier eedf20e
Rename SUB in baset
romainbrenguier 444bf14
Simplify is_unbounded_array
romainbrenguier 4c9364b
Refactoring in boolbvt::convert_add_sub
romainbrenguier 2333436
Refactoring in boolbvt::convert_array
romainbrenguier c41e26d
Refactoring in boolbvt::convert_array_of
romainbrenguier 1cd9a85
Refactoring in boolbvt::convert_index
romainbrenguier 1e4e071
Refactoring in boolbvt::convert_mult
romainbrenguier File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,15 +29,13 @@ Author: Daniel Kroening, [email protected] | |
#include <solvers/floatbv/float_utils.h> | ||
#include <solvers/lowering/expr_lowering.h> | ||
|
||
bool boolbvt::literal( | ||
const exprt &expr, | ||
const std::size_t bit, | ||
literalt &dest) const | ||
optionalt<literalt> | ||
boolbvt::literal(const exprt &expr, const std::size_t bit) const | ||
{ | ||
if(expr.type().id()==ID_bool) | ||
{ | ||
assert(bit==0); | ||
return prop_conv_solvert::literal(expr, dest); | ||
return prop_conv_solvert::literal(expr); | ||
} | ||
else | ||
{ | ||
|
@@ -50,16 +48,15 @@ bool boolbvt::literal( | |
map.mapping.find(identifier); | ||
|
||
if(it_m==map.mapping.end()) | ||
return true; | ||
return {}; | ||
|
||
const boolbv_mapt::map_entryt &map_entry=it_m->second; | ||
|
||
assert(bit<map_entry.literal_map.size()); | ||
if(!map_entry.literal_map[bit].is_set) | ||
return true; | ||
return {}; | ||
|
||
dest=map_entry.literal_map[bit].l; | ||
return false; | ||
return map_entry.literal_map[bit].l; | ||
} | ||
else if(expr.id()==ID_index) | ||
{ | ||
|
@@ -76,7 +73,7 @@ bool boolbvt::literal( | |
|
||
std::size_t offset=integer2unsigned(index*element_width); | ||
|
||
return literal(index_expr.array(), bit+offset, dest); | ||
return literal(index_expr.array(), bit + offset); | ||
} | ||
else if(expr.id()==ID_member) | ||
{ | ||
|
@@ -96,7 +93,7 @@ bool boolbvt::literal( | |
const typet &subtype=it->type(); | ||
|
||
if(it->get_name()==component_name) | ||
return literal(expr.op0(), bit+offset, dest); | ||
return literal(expr.op0(), bit + offset); | ||
|
||
std::size_t element_width=boolbv_width(subtype); | ||
|
||
|
@@ -564,7 +561,7 @@ literalt boolbvt::convert_rest(const exprt &expr) | |
return const_literal(true); | ||
} | ||
|
||
return SUB::convert_rest(expr); | ||
return baset::convert_rest(expr); | ||
} | ||
|
||
bool boolbvt::boolbv_set_equality_to_true(const equal_exprt &expr) | ||
|
@@ -605,7 +602,7 @@ void boolbvt::set_to(const exprt &expr, bool value) | |
const auto equal_expr = expr_try_dynamic_cast<equal_exprt>(expr); | ||
if(value && equal_expr && !boolbv_set_equality_to_true(*equal_expr)) | ||
return; | ||
SUB::set_to(expr, value); | ||
baset::set_to(expr, value); | ||
} | ||
|
||
exprt boolbvt::make_bv_expr(const typet &type, const bvt &bv) | ||
|
@@ -640,17 +637,9 @@ bool boolbvt::is_unbounded_array(const typet &type) const | |
if(unbounded_array==unbounded_arrayt::U_ALL) | ||
return true; | ||
|
||
const exprt &size=to_array_type(type).size(); | ||
|
||
mp_integer s; | ||
if(to_integer(size, s)) | ||
return true; | ||
|
||
if(unbounded_array==unbounded_arrayt::U_AUTO) | ||
if(s>MAX_FLATTENED_ARRAY_SIZE) | ||
return true; | ||
|
||
return false; | ||
const auto s = numeric_cast<mp_integer>(to_array_type(type).size()); | ||
return !s.has_value() || (unbounded_array == unbounded_arrayt::U_AUTO && | ||
*s > MAX_FLATTENED_ARRAY_SIZE); | ||
} | ||
|
||
void boolbvt::print_assignment(std::ostream &out) const | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,39 +6,30 @@ Author: Daniel Kroening, [email protected] | |
|
||
\*******************************************************************/ | ||
|
||
|
||
#include "boolbv.h" | ||
|
||
bvt boolbvt::convert_array(const exprt &expr) | ||
{ | ||
std::size_t width=boolbv_width(expr.type()); | ||
|
||
const std::size_t width = boolbv_width(expr.type()); | ||
if(width==0) | ||
return conversion_failed(expr); | ||
|
||
if(expr.type().id()==ID_array) | ||
{ | ||
assert(expr.has_operands()); | ||
INVARIANT(expr.has_operands(), "array should have operands"); | ||
const exprt::operandst &operands=expr.operands(); | ||
assert(!operands.empty()); | ||
std::size_t op_width=width/operands.size(); | ||
|
||
const std::size_t op_width = width / operands.size(); | ||
bvt bv; | ||
bv.reserve(width); | ||
|
||
forall_expr(it, operands) | ||
for(const exprt &op : operands) | ||
{ | ||
const bvt &tmp=convert_bv(*it); | ||
|
||
if(tmp.size()!=op_width) | ||
throw "convert_array: unexpected operand width"; | ||
|
||
forall_literals(it2, tmp) | ||
bv.push_back(*it2); | ||
const bvt &tmp = convert_bv(op); | ||
CHECK_RETURN(tmp.size() == op_width); | ||
bv.insert(bv.end(), tmp.begin(), tmp.end()); | ||
} | ||
|
||
return bv; | ||
} | ||
|
||
return conversion_failed(expr); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,16 +13,11 @@ Author: Daniel Kroening, [email protected] | |
|
||
bvt boolbvt::convert_array_of(const array_of_exprt &expr) | ||
{ | ||
if(expr.type().id()!=ID_array) | ||
throw "array_of must be array-typed"; | ||
|
||
const array_typet &array_type=to_array_type(expr.type()); | ||
|
||
if(is_unbounded_array(array_type)) | ||
return conversion_failed(expr); | ||
|
||
std::size_t width=boolbv_width(array_type); | ||
|
||
const std::size_t width = boolbv_width(array_type); | ||
if(width==0) | ||
{ | ||
// A zero-length array is acceptable; | ||
|
@@ -34,29 +29,25 @@ bvt boolbvt::convert_array_of(const array_of_exprt &expr) | |
} | ||
|
||
const exprt &array_size=array_type.size(); | ||
const auto size = numeric_cast<mp_integer>(array_size); | ||
|
||
mp_integer size; | ||
|
||
if(to_integer(array_size, size)) | ||
if(!size) | ||
return conversion_failed(expr); | ||
|
||
const bvt &tmp=convert_bv(expr.op0()); | ||
|
||
bvt bv; | ||
bv.resize(width); | ||
|
||
if(size*tmp.size()!=width) | ||
throw "convert_array_of: unexpected operand width"; | ||
CHECK_RETURN(*size * tmp.size() == width); | ||
|
||
std::size_t offset=0; | ||
|
||
for(mp_integer i=0; i<size; i=i+1) | ||
{ | ||
for(std::size_t j=0; j<tmp.size(); j++, offset++) | ||
bv[offset]=tmp[j]; | ||
} | ||
|
||
assert(offset==bv.size()); | ||
INVARIANT(offset == bv.size(), "final offset should correspond to size"); | ||
|
||
return bv; | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 don't see why this change is needed. If there is a real error then an exception should be thrown. If not then returning constant literals is acceptable.
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.
The function is not returning a constant literal when there is an error, it is just leaving
dest
untouched. So this is not changing the behavior of the function, just making the interface clearer.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.
May I suggest a different route: please just
#ifdef 0
this code. Whether you change the interface to anoptionalt
I don't care as much then. (Background: I wanted to figure out whether throwing an exception would be fine. I couldn't tell, because the code is never used.)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.
Not sure I get your comment. If I
#ifdef 0
the function when cbmc will not build becauseget_bool
is called from somewhere.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.
Sorry, this is not about
get_bool
, but aboutliteral
(bothboolbvt::literal
andprop_convt::literal
).