Make simplify_expr take copy instead of reference #4301
Conversation
romainbrenguier
requested review from
chrisr-diffblue,
danpoe,
hannes-steffenhagen-diffblue,
kroening,
martin-cs,
owen-mc-diffblue,
peterschrammel,
pkesseli,
smowton,
tautschnig and
thk123
as code owners
| @@ -1120,13 +1120,11 @@ bool acceleration_utilst::assign_array( | |||
| { | |||
| replace_expr( | |||
| loop_counter, from_integer(0, loop_counter.type()), lower_bound); | |||
| simplify_expr(lower_bound, ns); | |||
There was a problem hiding this comment.
Good catch - but I'd suggest to instead make those have an effect, e.g., by using simplify instead of simplify_expr
The implementation will make a copy of the argument anyway. This makes it explicit in the type and allow the use of move by the caller which avoids a copy.
The return value was ignored and thus there was no use in making these calls, which was probably a mistake.
We use std::move in a few places where this is made possible by the change in interface.
romainbrenguier
force-pushed
the
refactor/simplify_expr_copy
branch
from
075d05e to
94b2145
Compare
There was a problem hiding this comment.
This PR failed Diffblue compatibility checks (cbmc commit: 075d05e).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/102674819
Status will be re-evaluated on next push.
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).
-
the compatibility was already broken by an earlier merge.
There was a problem hiding this comment.
This PR failed Diffblue compatibility checks (cbmc commit: 94b2145).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/102679219
Status will be re-evaluated on next push.
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).
-
the compatibility was already broken by an earlier merge.
The implementation would make a copy of the argument anyway.
This makes it explicit in the type and allow the use of move by the
caller which avoids a copy.