Skip to content

Function to get a goto model from a C program for use in unit tests #5265

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

Merged
merged 1 commit into from
Mar 11, 2020

Conversation

danpoe
Copy link
Contributor

@danpoe danpoe commented Mar 10, 2020

The function takes a C program (as a string or an input stream) and converts it
to a goto model.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@danpoe danpoe force-pushed the refactor/get-goto-model branch from 9d3adc0 to 5e0e30b Compare March 11, 2020 11:11
Comment on lines 45 to 46
optionst options;
cbmc_parse_optionst::set_default_options(options);

Choose a reason for hiding this comment

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

This doesn’t actually do anything though? set_default_options just sets default values on the options, which you then discard.

Copy link
Contributor Author

@danpoe danpoe Mar 11, 2020

Choose a reason for hiding this comment

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

Removed

const bool error = language.parse(in, "");

if(error)
return {};

Choose a reason for hiding this comment

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

I thinki it’d be better to just throw an exception rather than return optional, because presumably you’d expect for unit tests that their input programs do compile (ditto for all the other error cases)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


language_filet &language_file = language_files.add_file("");

language_file.language = get_default_language();

Choose a reason for hiding this comment

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

This function is called get_goto_model_from_c, so I think this should be something like get_language_from_mode(ID_C).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


cmdlinet cmdline;

config.main = std::string("main");

Choose a reason for hiding this comment

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

Actually, I think this shouldn’t be called only once... because config is a global variable and could be messed with by other tests, ideally this would start with something like config = configt{}; so we can be sure we’re starting from a “clean” config and run every time (it’s not like this is going to cost us much test performance).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I've now gotten rid of the initialize function altogether.

}
} // namespace

optionalt<goto_modelt> get_goto_model_from_c(std::istream &in)

Choose a reason for hiding this comment

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

Not sure if it’s needed right now, but we’ll probably eventually want to set options for config and such in here as well (e.g. by passing in std::function<void(configt&)> which gets to set some options between resetting everything to default and parsing the C.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave this for a future PR.

@danpoe danpoe force-pushed the refactor/get-goto-model branch 2 times, most recently from 99c113e to 1210f72 Compare March 11, 2020 13:46
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM

@danpoe danpoe force-pushed the refactor/get-goto-model branch 3 times, most recently from e92d1a6 to 174f8f3 Compare March 11, 2020 16:04
The function takes a C program (as a string or an input stream) and converts it
to a goto model.
@danpoe danpoe merged commit c7bb936 into diffblue:develop Mar 11, 2020
@danpoe danpoe deleted the refactor/get-goto-model branch June 2, 2020 17:06
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