Skip to content

Conversation

@kroening
Copy link
Collaborator

  • Each commit message has a non-empty body, explaining why the change was made.
  • My contribution is formatted in line with CODING_STANDARD.md.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • 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).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

  1. Why?
  2. Missing tests.
  3. This piece of code will require further work once the function member is gone from instructiont as noted below.

for(const auto &disjunct : disjuncts)
{
std::string string_value =
from_expr(ns, s_it->source.pc->function, disjunct);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aren't we removing the function member?

@kroening
Copy link
Collaborator Author

  1. It avoids long lines, and matches with the usual definition of the Gentzen sequent.
  2. Ok, will add some.
  3. --show-vcc now uses format() #2977 takes care of this!

@kroening kroening force-pushed the show_vcc_disjunction branch from 871298a to 185c912 Compare October 14, 2018 18:30
Rationale: This avoids a long "{1}" line and matches the usual definition of
the Gentzen sequent.
@kroening kroening force-pushed the show_vcc_disjunction branch from 185c912 to 09a7989 Compare October 14, 2018 18:31
@kroening kroening assigned tautschnig and unassigned kroening Oct 14, 2018
Copy link
Contributor

@allredj allredj left a 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: 871298a).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/87887369
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.

Copy link
Contributor

@allredj allredj left a 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: 09a7989).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/87893119

@kroening kroening merged commit e859483 into develop Oct 14, 2018
@kroening kroening deleted the show_vcc_disjunction branch October 14, 2018 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants