Skip to content

Remove redundant lines in ID_dynamic_object branch in value_set_dereferencet #4291

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
Feb 28, 2019

Conversation

owen-mc-diffblue
Copy link
Contributor

This branch is not taken any more, at least in symex. The code in it
doesn't make much sense, which makes me think it isn't taken at all.

  • 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.
  • 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).
  • 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.

@owen-mc-diffblue owen-mc-diffblue changed the title Remove redundant ID_dynamic_object branch Remove redundant ID_dynamic_object branch in value_set_dereferencet Feb 27, 2019
@tautschnig
Copy link
Collaborator

This reminds me of f1ff824 from #2646 (and #3769, where the actual merge of that work is being prepared). I don't feel sufficiently comfortable removing this at the moment (given the mess that f1ff824 tries to clean up). It's probably ok to be removed, and it's perfectly possible that ID_dynamic_object expressions aren't generated at all. But I'm not really sure...

@tautschnig
Copy link
Collaborator

Actually the regression tests suggest that this isn't dead code at all:

rw_range_sett: address_of `dynamic_object' not handled

This branch is not taken by symex but it is taken by the slicer. The code
I've removed has no effect, as far as I can tell, and may have been left
over from an old implementation from before the beginning of the git
history.

Note, there is a commit that splits ID_dynamic_object into two different
ids. See diffblue#2646 and diffblue#3769 for more details.
@owen-mc-diffblue
Copy link
Contributor Author

Thanks for pointing that commit out to me, @tautschnig . I hope it can be merged soon, as it seems to fix some bugs in the slicer.

Indeed, this code does turn out to be used. I've updated my commit to just remove the lines from it that seem to have no effect at all. If that doesn't pass CI, or someone points out an effect that they have, then I will close this PR.

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: d57e21b).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/102634292

@smowton smowton merged commit 863d199 into diffblue:develop Feb 28, 2019
@owen-mc-diffblue owen-mc-diffblue deleted the remove-unused-code branch February 28, 2019 11:24
@owen-mc-diffblue owen-mc-diffblue changed the title Remove redundant ID_dynamic_object branch in value_set_dereferencet Remove redundant lines in ID_dynamic_object branch in value_set_dereferencet Mar 1, 2019
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.

4 participants