Skip to content

VSD - Further cross-representation expression evaluation #5887

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 9 commits into from
Mar 8, 2021

Conversation

jezhiggins
Copy link
Contributor

The PR follows on from #5880, completing the implementation work for value-sets and intervals.

The bulk of the PR is additional unit tests, confirming behaviour for the various combinations of constants, intervals, value-sets of constants, value-sets of intervals, and value-sets with mixed content.

The functional changes are quite small - a fix to value-set evaluation to properly handle intervals and a similar change to value-set index ranges (used when indexing into arrays) to remove the assumption the contents are all constants.

As we look to do cross-representation evaluation, it's no longer meanful
to have separate tests for "expressions involving constants", etc
rewrite_expression has, incorrectly, expected every term in the
rewritten expression to be a constant. When there are intervals in play,
this is clearly not the case.

When we encounter an interval, don't attempt to coerce it to a constant
expression, simply use the existing operand.
A constant of, say, 5 has a index_range of 5. A value_set of { 5, 6,
9 } therefore has an index_range of 5,6,9.

An interval of [1, 3] has an index_range of 1,2,3. A value_set
containing that interval should have the same index_range.

A value_set of multiple constants and intervals should have an
index_range which is the union of its contents index_range.
@codecov
Copy link

codecov bot commented Mar 7, 2021

Codecov Report

Merging #5887 (b86dbcf) into develop (45b6d72) will increase coverage by 0.13%.
The diff coverage is 98.45%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5887      +/-   ##
===========================================
+ Coverage    72.95%   73.09%   +0.13%     
===========================================
  Files         1425     1428       +3     
  Lines       154281   155058     +777     
===========================================
+ Hits        112561   113341     +780     
+ Misses       41720    41717       -3     
Impacted Files Coverage Δ
...ses/variable-sensitivity/constant_abstract_value.h 100.00% <ø> (ø)
...s/variable-sensitivity/value_set_abstract_object.h 100.00% <ø> (ø)
unit/analyses/variable-sensitivity/eval.cpp 100.00% <ø> (ø)
...lyses/variable-sensitivity/abstract_value_object.h 87.71% <86.79%> (-12.29%) ⬇️
...ses/variable-sensitivity/abstract_value_object.cpp 95.43% <96.00%> (+7.20%) ⬆️
...variable-sensitivity/value_set_abstract_object.cpp 87.50% <98.18%> (-4.22%) ⬇️
...-sensitivity/variable_sensitivity_test_helpers.cpp 98.24% <98.24%> (ø)
...lyses/variable-sensitivity/abstract_object_set.cpp 100.00% <100.00%> (ø)
...nalyses/variable-sensitivity/abstract_object_set.h 100.00% <100.00%> (ø)
...s/variable-sensitivity/constant_abstract_value.cpp 88.88% <100.00%> (-5.43%) ⬇️
... and 18 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 fa12ca7...b86dbcf. Read the comment docs.

Copy link
Collaborator

@martin-cs martin-cs left a comment

Choose a reason for hiding this comment

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

Thanks for the clean ups and the unit tests. I think any changes are best done with the upcoming review of VSD.

@martin-cs martin-cs merged commit 104ba11 into diffblue:develop Mar 8, 2021
@jezhiggins jezhiggins deleted the vsd-value-sets-and-intervals branch April 1, 2021 09:29
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.

2 participants