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. |
src/util/sharing_map.h
Outdated
| @@ -54,6 +54,16 @@ Author: Daniel Poetzl | |||
| 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.
src/util/sharing_map.h
Outdated
| 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.
src/util/std_expr.h
Outdated
| @@ -121,6 +121,18 @@ class symbol_exprt : public nullary_exprt | |||
| } | |||
| }; | |||
|
|
|||
| 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 |
src/util/sharing_map.h
Outdated
| @@ -54,6 +54,16 @@ Author: Daniel Poetzl | |||
| 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.
src/util/sharing_map.h
Outdated
| 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));
}
src/util/sharing_map.h
Outdated
| @@ -779,15 +824,15 @@ ::get_sharing_stats_map(Iterator begin, Iterator end) | |||
| } | |||
| #endif | |||
|
|
|||
| SHARING_MAPT(void)::get_view(viewt &view) const | |||
| SHARING_MAPTV(void, view_type)::get_view(view_type &view) const | |||
unit/util/sharing_map.cpp
Outdated
| @@ -492,7 +492,7 @@ TEST_CASE("Sharing map views and iteration", "[core][util]") | |||
|
|
|||
| 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
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.