Changes to sharing-map needed to merge VSD #5202
Conversation
martin-cs
requested review from
kroening,
peterschrammel,
pkesseli,
smowton and
tautschnig
as code owners
|
Some of these changes might now be redundant and I think it might be best to squash the commits as the messages aren't particularly meaningful now de-contextualised but I figured looking at the code might be the place to start. |
|
Note that the linter failures are unavoidable. |
| typename equalT> \ | ||
| type sharing_mapt<keyT, valueT, fail_if_equal, hashT, equalT> | ||
|
|
||
| #define SHARING_MAPTV(return_type, V) \ |
There was a problem hiding this comment.
Given that this is used in exactly one place I’d like to voice a strong preference for just inlining this macro.
There was a problem hiding this comment.
The already existing macro SHARING_MAPT4 does just the same, so I'd vote for using that one instead.
| void get_view(viewt &view) const; | ||
| template <class V> | ||
| void get_view(V &) const; | ||
| viewt get_view() const |
There was a problem hiding this comment.
I suppose technically this should be a template as well now.
| } | ||
| }; | ||
|
|
||
| namespace std |
There was a problem hiding this comment.
This is probably harmless but I think it’d nevertheless be preferable if we could find a way to do without this.
(I.e. remove it going forward after VSD is merged, not necessarily here)
|
Linter failures should be addressed with judicious use of |
| typename equalT> \ | ||
| type sharing_mapt<keyT, valueT, fail_if_equal, hashT, equalT> | ||
|
|
||
| #define SHARING_MAPTV(return_type, V) \ |
There was a problem hiding this comment.
The already existing macro SHARING_MAPT4 does just the same, so I'd vote for using that one instead.
| static void insert_view_item(sorted_viewt &v, view_itemt &&vi) | ||
| { | ||
| v.insert(vi); | ||
| } |
There was a problem hiding this comment.
Can we use forwarding references for these two functions? I.e.:
template <typename itemT>
static void insert_view_item(viewt &v, itemT &&vi)
{
v.push_back(std::forward<itemT>(vi));
}
| #endif | ||
|
|
||
| SHARING_MAPT(void)::get_view(viewt &view) const | ||
| SHARING_MAPTV(void, view_type)::get_view(view_type &view) const |
| SECTION("View") | ||
| { | ||
| typedef std::pair<std::string, std::string> pt; | ||
| typedef std::pair<irep_idt, std::string> pt; |
There was a problem hiding this comment.
Suggest changing this back to std::string. The intention was to sort the vector in lexicographic order. However, < for irep_idt sorts according to their sequence number AFAIK.
martin-cs
force-pushed
the
VSD/sharing-map
branch
2 times, most recently
from
b5818f4 to
951cb61
Compare
martin-cs
force-pushed
the
VSD/sharing-map
branch
3 times, most recently
from
3b13122 to
df7cea1
Compare
martin-cs
force-pushed
the
VSD/sharing-map
branch
from
df7cea1 to
ba46eb4
Compare
martin-cs
force-pushed
the
VSD/sharing-map
branch
from
83dd61e to
a56f389
Compare
martin-cs
force-pushed
the
VSD/sharing-map
branch
from
a56f389 to
96101fd
Compare
martin-cs
force-pushed
the
VSD/sharing-map
branch
from
96101fd to
ddb7e63
Compare
martin-cs
force-pushed
the
VSD/sharing-map
branch
from
ddb7e63 to
228b9dd
Compare
3 participants
This PR consists of a series of commits taken from the variable-sensitivity-domain branch originally by @hannes-steffenhagen-diffblue and @chrisr-diffblue . I have separated out the commits (and parts of commits) that just affect the sharing map so these can be reviewed and merged separately.