Skip to content

Conversation

thomasspriggs
Copy link
Contributor

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

@codecov
Copy link

codecov bot commented Jul 8, 2022

Codecov Report

Merging #7004 (4d5d529) into develop (82439fd) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #7004   +/-   ##
========================================
  Coverage    77.84%   77.85%           
========================================
  Files         1569     1569           
  Lines       180590   180656   +66     
========================================
+ Hits        140582   140648   +66     
  Misses       40008    40008           
Impacted Files Coverage Δ
..._incremental/smt2_incremental_decision_procedure.h 75.00% <ø> (ø)
...ncremental/smt2_incremental_decision_procedure.cpp 95.47% <100.00%> (+0.73%) ⬆️
...ncremental/smt2_incremental_decision_procedure.cpp 97.58% <100.00%> (+0.33%) ⬆️

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 9a0dec1...4d5d529. Read the comment docs.

Copy link
Contributor

@esteffin esteffin left a comment

Choose a reason for hiding this comment

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

All good to me.
Just a minor non-blocking comment

solver_process->send(smt_declare_function_commandt{array_identifier, {}});
const std::vector<exprt> &elements = array.operands();
const std::size_t index_width =
array_sort->index_sort().cast<smt_bit_vector_sortt>()->bit_width();
Copy link
Contributor

Choose a reason for hiding this comment

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

⛏️
array_sort->index_sort() is never enforced to be of smt_bit_vector_sortt type (although we don't use other types).
I suggest adding and INVARIANT to ensure the cast is safe

This means that there will be fewer changes required if we ever change
the encoding of arrays such that they do not have a bit
vector sorted index. It also means that there is no need to add an
invariant for this previously implicitly assumed property.
@thomasspriggs thomasspriggs merged commit 22fec4c into diffblue:develop Jul 11, 2022
@thomasspriggs thomasspriggs deleted the tas/smt_array_exprt branch July 11, 2022 14:28
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