-
Notifications
You must be signed in to change notification settings - Fork 277
Split parse_list_types into two functions #3293
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
Split parse_list_types into two functions #3293
Conversation
7a669a9
to
ccd22e8
Compare
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.
It'd be a great time to add a unit test for these methods
{ | ||
// Loop over the types in the given string, parsing each one in turn | ||
// and adding to the type_list | ||
std::vector<typet> type_list; |
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.
⛏️
const std::vector<std::string> type_strings = parse_raw_list_types(src, opening_bracket, closing_bracket);
std::vector<typet> type_list;
std::transform(
type_strings.begin(),
type_strings.end(),
std::back_inserter(type_list),
[&](const std::string &raw_type) {
const typet new_type = java_type_from_string(raw_type, class_name_prefix);
INVARIANT(new_type != nil_typet(), "Failed to parse type");
return new_type;
});
return type_list;
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.
Does this actually give any real benefit when you're simply looping through an entire list with no bells or whistles?
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.
That's exactly when you should use it, so the reader can tell that you are looping through an entire list with no bells and whistles! It wouldn't work if you had some whistles.
FYI the ⛏️ here means a non-blocking comment that you could fix if you're in the area but mostly there as something to try and do in future.
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.
Cheers, was mostly just curious about the recommendation (wasn't meant as a refutal).
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.
🚫
This PR failed Diffblue compatibility checks (cbmc commit: ccd22e8).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/90702246
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.
Joel bot is feeling a bit sensitive at the moment as previously merged CBMC PRs have broken TG - since this touches Java code can you wait until it comes back online? (hopefully shortly). |
Please rebase to re-trigger the TG check. |
ccd22e8
to
0d8ceaf
Compare
In some situations you might want to know what the bytecode type is and then manually perform some operation afterwards, in this case it's useful to have a method that returns the string representation of the bytecode type.
0d8ceaf
to
4898928
Compare
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.
✔️
Passed Diffblue compatibility checks (cbmc commit: 4898928).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/90880596
In some situations you might want to know what the bytecode type is and then manually perform some operation afterwards, in this case it's useful to have a method that returns the string representation of the bytecode type.