Make split_string check&throw instead of assert return value #3471
Conversation
hannes-steffenhagen-diffblue
requested review from
kroening,
peterschrammel,
pkesseli,
smowton and
tautschnig
as code owners
| throw deserialization_exceptiont{"expected string `" + s + | ||
| "' to contain two substrings " | ||
| "delimited by " + | ||
| delim + " but has " + |
There was a problem hiding this comment.
"but has" -> "but there are" (suggestion)
There was a problem hiding this comment.
🚫
This PR failed Diffblue compatibility checks (cbmc commit: b68d590).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/92679458
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
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 incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.
|
(Unrelated request, for another PR: This variant should return a std::pair<std::string, std::string>. One day, this will enable these to become std::string_view. |
hannes-steffenhagen-diffblue
force-pushed
the
string-split-throw-instead-of-assert
branch
from
b68d590 to
4538eaf
Compare
There was a problem hiding this comment.
🚫
This PR failed Diffblue compatibility checks (cbmc commit: 4538eaf).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/92726576
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.
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 incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.
The 2-string version of split_string previously asserted that the split would result in the right number of substrings. That made it not very useful for parsing user input, as using it would require parsing the string before passing it to split_string.
| "' to contain two substrings " | ||
| "delimited by " + | ||
| delim + " but has " + | ||
| std::to_string(result.size())}; |
There was a problem hiding this comment.
Nit: " but has " + std::to_string(result.size()) + " substrings"
hannes-steffenhagen-diffblue
force-pushed
the
string-split-throw-instead-of-assert
branch
from
4538eaf to
19babdc
Compare
There was a problem hiding this comment.
✔️
Passed Diffblue compatibility checks (cbmc commit: 19babdc).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/92836392
The 2-string version of split_string previously asserted that the split would
result in the right number of substrings. That made it not very useful for
parsing user input, as using it would require parsing the string before passing
it to split_string.