Skip to content

Do not use exprt::move_to_operands as it is marked as deprecated #3253

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
Nov 1, 2018

Conversation

tautschnig
Copy link
Collaborator

In particular, the use in std_code.h triggers a series of warnings every time
this file is included.

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a 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).
  • n/a 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
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: 141b5e5).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/89882763

@smowton
Copy link
Contributor

smowton commented Nov 1, 2018

In #3067 the solution arrived at was to have a rvalue-ref version of add. Rather oddly this was implemented by creating rvalue-ref overloads of copy_to_operands (https://github.com/diffblue/cbmc/blob/develop/src/util/expr.h#L126), so we end up writing some_expr.copy_to_operands(std::move(other_expr)).

I'd suggest simply renaming copy_to_operands(exprt &&) as move_to_operands(exprt &&) or add_to_operands(exprt &&) and use that function to implement this PR.

@smowton
Copy link
Contributor

smowton commented Nov 1, 2018

PR here: #3254

expr.cpp should not be using methods marked as deprecated in its own header
file. This change furthermore increases type safety as the constructor of
`not_exprt` will check that the type of the operand is ID_bool.
@tautschnig
Copy link
Collaborator Author

I have removed the changes to codet from this commit/PR, I will do them separately based on the work that @smowton has done in the meantime. Hopefully this is now no longer controversial.

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

@smowton smowton merged commit 7a4545c into diffblue:develop Nov 1, 2018
@tautschnig tautschnig deleted the no-move_to_operands branch November 1, 2018 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants