Skip to content

Make set_indices a template #4333

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

romainbrenguier
Copy link
Contributor

@romainbrenguier romainbrenguier commented Mar 6, 2019

This makes the interface of goto_symex_statet simpler and can simplify
code of function templates using set_indices

This is based on #3986 (merged)

  • 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.
  • [na] 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).
  • [na] My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • [na] White-space or formatting changes outside the feature-related changed lines are in commits of their own.

Copy link
Contributor

@smowton smowton left a comment

Choose a reason for hiding this comment

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

Perhaps TODO isn't the right formulation, but the idea that we're supposed to be using the type system to track which expressions are renamed, and that reaching those TODOs means that doesn't fully happen yet (but we don't want to abruptly make that fatal) seems good to me.

@romainbrenguier
Copy link
Contributor Author

@smowton

Perhaps TODO isn't the right formulation, but the idea that we're supposed to be using the type system to track which expressions are renamed, and that reaching those TODOs means that doesn't fully happen yet (but we don't want to abruptly make that fatal) seems good to me.

This comment should have been on #3986 (and @tautschnig already commented on that point)

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: f2e7514).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/103336929
Status will be re-evaluated on next push.
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 compatibility was already broken by an earlier merge.

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: eade946).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/103336871
Status will be re-evaluated on next push.
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 compatibility was already broken by an earlier merge.

@romainbrenguier romainbrenguier force-pushed the refactor/symex-set-indices-template branch from f2e7514 to c7b92fc Compare March 6, 2019 12:36
romainbrenguier added a commit that referenced this pull request Mar 6, 2019
Make return type of set_l*_indices be a renamedt [blocks: #4333]
This will be necessary to make them templates
This makes the interface of goto_symex_statet simpler and can simplify
code of function templates using set_indices
Now tha set_indices is a template we don't need the case disjunction on
the level parameter.
@romainbrenguier romainbrenguier force-pushed the refactor/symex-set-indices-template branch from c7b92fc to 78cf428 Compare March 6, 2019 13:12
@romainbrenguier romainbrenguier changed the title Make set_indices a template [depends on #3986] Make set_indices a template Mar 6, 2019
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: 78cf428).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/103368256
Status will be re-evaluated on next push.
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 compatibility was already broken by an earlier merge.

@romainbrenguier romainbrenguier merged commit 611b910 into diffblue:develop Mar 6, 2019
@romainbrenguier romainbrenguier deleted the refactor/symex-set-indices-template branch March 6, 2019 15:00
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