-
Notifications
You must be signed in to change notification settings - Fork 277
Factor out computation of maximum union member #5251
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
Merged
Merged
Changes from all commits
Commits
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 hidden or 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 |
---|---|---|
|
@@ -21,6 +21,41 @@ Author: Daniel Kroening, [email protected] | |
#include <util/simplify_expr.h> | ||
#include <util/string_constant.h> | ||
|
||
/// Determine the member of maximum fixed bit width in a union type. If no | ||
/// member, or no member of fixed and non-zero width can be found, return | ||
/// nullopt. | ||
/// \param union_type: Type to determine the member of. | ||
/// \param ns: Namespace to resolve tag types. | ||
/// \return Pair of a componentt pointing to the maximum fixed bit-width | ||
/// member of \p union_type and the bit width of that member. | ||
static optionalt<std::pair<struct_union_typet::componentt, mp_integer>> | ||
find_widest_union_component(const union_typet &union_type, const namespacet &ns) | ||
{ | ||
const union_typet::componentst &components = union_type.components(); | ||
|
||
mp_integer max_width = 0; | ||
typet max_comp_type; | ||
irep_idt max_comp_name; | ||
|
||
for(const auto &comp : components) | ||
{ | ||
auto element_width = pointer_offset_bits(comp.type(), ns); | ||
|
||
if(!element_width.has_value() || *element_width <= max_width) | ||
continue; | ||
|
||
max_width = *element_width; | ||
max_comp_type = comp.type(); | ||
max_comp_name = comp.get_name(); | ||
} | ||
|
||
if(max_width == 0) | ||
return {}; | ||
else | ||
return std::make_pair( | ||
struct_union_typet::componentt{max_comp_name, max_comp_type}, max_width); | ||
} | ||
|
||
static exprt bv_to_expr( | ||
const exprt &bitvector_expr, | ||
const typet &target_type, | ||
|
@@ -1124,31 +1159,18 @@ exprt lower_byte_extract(const byte_extract_exprt &src, const namespacet &ns) | |
else if(src.type().id() == ID_union || src.type().id() == ID_union_tag) | ||
{ | ||
const union_typet &union_type = to_union_type(ns.follow(src.type())); | ||
const union_typet::componentst &components = union_type.components(); | ||
|
||
mp_integer max_width = 0; | ||
typet max_comp_type; | ||
irep_idt max_comp_name; | ||
|
||
for(const auto &comp : components) | ||
{ | ||
auto element_width = pointer_offset_bits(comp.type(), ns); | ||
|
||
if(!element_width.has_value() || *element_width <= max_width) | ||
continue; | ||
const auto widest_member = find_widest_union_component(union_type, ns); | ||
|
||
max_width = *element_width; | ||
max_comp_type = comp.type(); | ||
max_comp_name = comp.get_name(); | ||
} | ||
|
||
if(max_width > 0) | ||
if(widest_member.has_value()) | ||
{ | ||
byte_extract_exprt tmp(unpacked); | ||
tmp.type() = max_comp_type; | ||
tmp.type() = widest_member->first.type(); | ||
|
||
return union_exprt( | ||
max_comp_name, lower_byte_extract(tmp, ns), src.type()); | ||
widest_member->first.get_name(), | ||
lower_byte_extract(tmp, ns), | ||
src.type()); | ||
} | ||
} | ||
|
||
|
@@ -1945,34 +1967,19 @@ static exprt lower_byte_update_union( | |
const optionalt<exprt> &non_const_update_bound, | ||
const namespacet &ns) | ||
{ | ||
const union_typet::componentst &components = union_type.components(); | ||
|
||
mp_integer max_width = 0; | ||
typet max_comp_type; | ||
irep_idt max_comp_name; | ||
|
||
for(const auto &comp : components) | ||
{ | ||
auto element_width = pointer_offset_bits(comp.type(), ns); | ||
|
||
if(!element_width.has_value() || *element_width <= max_width) | ||
continue; | ||
|
||
max_width = *element_width; | ||
max_comp_type = comp.type(); | ||
max_comp_name = comp.get_name(); | ||
} | ||
const auto widest_member = find_widest_union_component(union_type, ns); | ||
|
||
PRECONDITION_WITH_DIAGNOSTICS( | ||
max_width > 0, | ||
widest_member.has_value(), | ||
"lower_byte_update of union of unknown size is not supported"); | ||
|
||
byte_update_exprt bu = src; | ||
bu.set_op(member_exprt{src.op(), max_comp_name, max_comp_type}); | ||
bu.type() = max_comp_type; | ||
bu.set_op(member_exprt{ | ||
src.op(), widest_member->first.get_name(), widest_member->first.type()}); | ||
bu.type() = widest_member->first.type(); | ||
|
||
return union_exprt{ | ||
max_comp_name, | ||
widest_member->first.get_name(), | ||
lower_byte_update(bu, value_as_byte_array, non_const_update_bound, ns), | ||
src.type()}; | ||
} | ||
|
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.
Is there any particular reason this returns an optional pair, rather than just an optional
componentt
? Given that the componentt has the the appropriate type in it and the caller could just dopointer_offset_bits
themselves. If it's just to simplify future clients, then fine :-) But just wondering as I don't see any current clients using the second half of the pair, and having the extra->first
or->second
slightly reduces readability at the call sites.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.
Yes, that second part is only used in #5252. I hope that's ok - I'll go ahead and merge.
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.
FWIW, you're of course right that the caller could compute the size themselves, but I thought duplicating work was not a good idea.
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.
Fine by me. I was reviewing in PR order so hadn't seen the followup PR at that point.