Skip to content

Define an add_const utility function #4257

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 2 commits into from
Feb 25, 2019

Conversation

romainbrenguier
Copy link
Contributor

@romainbrenguier romainbrenguier commented Feb 22, 2019

This is useful in particular when we want to force the const version of
a method to be called.
For exprt there can be differences in performance between const and
non-const.

This is applied to goto_symex_statet::rename as an example, and trying of the example in jbmc/regression/jbmc-strings/StringConcat/test_buffer_nondet_loop5 it reduced the number of calls to detach from 744638 to 736844.

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

@@ -345,10 +347,12 @@ void goto_symex_statet::rename(

INVARIANT(
(expr.id() != ID_with ||
expr.type() == to_with_expr(expr).old().type()) &&
add_const(expr).type() == to_with_expr(add_const(expr)).old().type()) &&
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe declare a const exprt &c_expr = expr; before the INVARIANT and use that in here? There are a lot of add_consts here.

@kroening
Copy link
Member

Simply use std::add_const, it's in <type_traits>?

@romainbrenguier
Copy link
Contributor Author

@kroening

Simply use std::add_const, it's in <type_traits>?

std::add_const only acts on types

@tautschnig
Copy link
Collaborator

It seems you'd need std::as_const, but that's C++-17. Though it may be a reason to rename this function from add_const to as_const @romainbrenguier ?

This is useful in particular when we want to force the const version of
a method to be called.
For exprt there can be differences in performance between const and
non-const.
In some places the non const version of .type() and others would be
called.
We force the const version to be called by using as_const.

In the example in
 `jbmc/regression/jbmc-strings/StringConcat/test_buffer_nondet_loop5`
 this reduced the number of calls to detach from 744638 to 736844.
@romainbrenguier romainbrenguier removed their assignment Feb 25, 2019
@romainbrenguier
Copy link
Contributor Author

@tautschnig

It seems you'd need std::as_const, but that's C++-17. Though it may be a reason to rename this function from add_const to as_const @romainbrenguier ?

done

@tautschnig tautschnig merged commit 6589d99 into diffblue:develop Feb 25, 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.

3 participants