Skip to content

Initial set of unit tests for static and local lamdbas #2

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

Conversation

thk123
Copy link
Collaborator

@thk123 thk123 commented Feb 19, 2018

Currently in a misnamed file java_bytecode_parser_rclass_attribute.cpp

@thk123 thk123 requested a review from mgudemann as a code owner February 19, 2018 17:07
/// Represents the argument of an instruction that uses a CONSTANT_Fieldref
/// This is used for example as an argument to a getstatic and putstatic
/// instruction
class fieldref_exprt : public exprt
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this is pulled into its own PR: diffblue#1856

@mgudemann mgudemann force-pushed the feature/parse_bootstrapmethods_attribute branch from bf53c54 to a8f0116 Compare February 21, 2018 09:33
@thk123 thk123 force-pushed the feature/parse_bootstrapmethods_unittests branch 2 times, most recently from 10d4391 to e7e0e22 Compare February 23, 2018 18:11
@thk123
Copy link
Collaborator Author

thk123 commented Feb 23, 2018

@mgudemann This is now ready for review and inclusion into the PR - I wonder if some of the tests are kind of redundant (e.g. we have the exhaustive static set, but then for member and local variable we just care about the ones that capture variables since they are the only ones that behave differently, what do you think?

Copy link
Owner

@mgudemann mgudemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, just some minor comments

require_parse_tree::lambda_method_handlet
require_parse_tree::require_lambda_entry_for_descriptor(
const java_bytecode_parse_treet::classt &parsed_class,
std::string descriptor,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not const ?

typedef java_bytecode_parse_treet::classt::lambda_method_handle_mapt::
value_type lambda_method_entryt;

int matches_found = 0;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

size_t or unsigned


int matches_found = 0;

const auto matching_lambda_entry = std::find_if(
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might also count to detect several matching entries

@thk123 thk123 force-pushed the feature/parse_bootstrapmethods_unittests branch from e7e0e22 to 406fc00 Compare February 26, 2018 14:49
@mgudemann mgudemann merged this pull request into mgudemann:feature/parse_bootstrapmethods_attribute Feb 26, 2018
@thk123 thk123 deleted the feature/parse_bootstrapmethods_unittests branch February 26, 2018 17:32
mgudemann pushed a commit that referenced this pull request Aug 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants