Skip to content

Conversation

@romainbrenguier
Copy link
Contributor

No functional changes.
This is a follow-up for #5035. Some code is now unreachable and can be removed, other parts can be moved to the string_insertion_builtin_function class to improve the structure of the code.

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

This is now totally handled by the string_dependencies class.
/// 4. forall i in [offset', s1.length). res[i + s2.length] = s1[i]
/// This is equivalent to
/// `res=concat(substring(s1, 0, offset'), concat(s2, substring(s1, offset')))`.
/// \param fresh_symbol: generator of fresh symbols
Copy link
Member

Choose a reason for hiding this comment

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

🚫 docs don't match signature

}

protected:
explicit string_insertion_builtin_functiont(
Copy link
Member

Choose a reason for hiding this comment

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

explicit not required

@romainbrenguier romainbrenguier force-pushed the clean-up/constraint-generation2 branch 3 times, most recently from 239ae21 to fc4baa4 Compare August 28, 2019 12:57
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: fc4baa4).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/124918171

@codecov-io
Copy link

codecov-io commented Aug 28, 2019

Codecov Report

Merging #5063 into develop will increase coverage by 0.01%.
The diff coverage is 60.22%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5063      +/-   ##
===========================================
+ Coverage    69.51%   69.53%   +0.01%     
===========================================
  Files         1314     1315       +1     
  Lines       108980   108967      -13     
===========================================
+ Hits         75760    75766       +6     
+ Misses       33220    33201      -19
Impacted Files Coverage Δ
...lvers/strings/string_constraint_generator_main.cpp 84.83% <ø> (+0.38%) ⬆️
src/solvers/strings/string_builtin_function.h 76.82% <ø> (-0.6%) ⬇️
...rs/strings/string_concatenation_builtin_function.h 33.33% <ø> (ø) ⬆️
src/solvers/strings/string_constraint_generator.h 100% <ø> (ø) ⬆️
src/solvers/strings/string_builtin_function.cpp 74.67% <ø> (+9.38%) ⬆️
...vers/strings/string_insertion_builtin_function.cpp 57.14% <57.14%> (ø)
...olvers/strings/string_insertion_builtin_function.h 81.81% <81.81%> (ø)
src/util/std_code.cpp 53.75% <0%> (-4.21%) ⬇️
src/ansi-c/expr2c.cpp 67.88% <0%> (-0.22%) ⬇️
src/analyses/ai.h 59.15% <0%> (ø) ⬆️
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9549238...3e1096d. Read the comment docs.

Put all the code related to the builtin string_insert function to a
string_insertion_builtin_function file.
The string preprocessing no longer produces calls to this builtin
function which would have 3 arguments (in addition to the two string
arguments) so the UNIMPLEMENTED case can be made into a precondition instead.
This is the only place where it is used, and the constraint method is
very short so it makes sense to inline it there.
This is only used in one place and the code of both function is very
short (trivial in one case) so it is better to merge them.
@romainbrenguier romainbrenguier force-pushed the clean-up/constraint-generation2 branch from fc4baa4 to 3e1096d Compare August 28, 2019 13:59
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: 3e1096d).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/124929261

@romainbrenguier romainbrenguier merged commit 1305f75 into diffblue:develop Aug 28, 2019
@romainbrenguier romainbrenguier deleted the clean-up/constraint-generation2 branch August 28, 2019 15: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.

6 participants