Skip to content

fix distinct operator #5827

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
Feb 15, 2021
Merged

fix distinct operator #5827

merged 1 commit into from
Feb 15, 2021

Conversation

polgreen
Copy link
Contributor

@polgreen polgreen commented Feb 15, 2021

  • 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.
  • [n/a] 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).
  • [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.

Adds support for distinct operator in smt2 parser (which is syntactic sugar for a pairwise "not equal" predicate). Previously operator was parsed but later ignored

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.

Sorry but I don't think this is the correct semantics. I think that distinct is pair-wise inequality.

else
{
std::vector<exprt> pairwise_constraints;
for(std::size_t i = 0; i < op.size();)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is the semantics of SMT-LIB distinct. It is described here ( http://smtlib.cs.uiowa.edu/theories-Core.shtml ) as:

(par (A) (distinct A A Bool :pairwise))

And :pairwise is described on Page 34 of http://smtlib.cs.uiowa.edu/papers/smt-lib-reference-v2.6-r2017-07-18.pdf as:

[pairwise annotation] (f   t1···tn) is syntactic sugar (recursively) for (and (f   t1 t2) ··· (f   t1 tn) (f   t2···tn)).

This adds support for the distinct operator in SMTlib, by directly translating it into "not equal"

Prior to this change, the distinct operator was ignored, giving incorrect results
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.

Much better -- thank you!

{
for(std::size_t j = i; j < op.size(); j++)
{
if(i != j)
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could start at j = i + 1 and skip this check if you wanted.

@codecov
Copy link

codecov bot commented Feb 15, 2021

Codecov Report

Merging #5827 (9778868) into develop (23c11e8) will increase coverage by 3.76%.
The diff coverage is 95.28%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #5827      +/-   ##
===========================================
+ Coverage    69.08%   72.85%   +3.76%     
===========================================
  Files         1242     1423     +181     
  Lines       100842   154072   +53230     
===========================================
+ Hits         69667   112244   +42577     
- Misses       31175    41828   +10653     
Flag Coverage Δ
regression ?
unit ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ses/variable-sensitivity/interval_abstract_value.h 100.00% <ø> (ø)
...s/variable-sensitivity/value_set_abstract_object.h 100.00% <ø> (ø)
...-sensitivity/variable_sensitivity_object_factory.h 100.00% <ø> (ø)
...riable-sensitivity/full_struct_abstract_object.cpp 81.81% <33.33%> (-0.09%) ⬇️
.../analyses/variable-sensitivity/abstract_object.cpp 69.15% <50.00%> (-4.80%) ⬇️
...yses/variable-sensitivity/abstract_environment.cpp 87.42% <87.50%> (+2.02%) ⬆️
...s/variable-sensitivity/interval_abstract_value.cpp 78.77% <94.44%> (-0.88%) ⬇️
...variable-sensitivity/value_set_abstract_object.cpp 87.01% <97.14%> (+15.58%) ⬆️
...alyses/variable-sensitivity/abstract_environment.h 100.00% <100.00%> (ø)
...s/variable-sensitivity/constant_abstract_value.cpp 94.31% <100.00%> (+2.31%) ⬆️
... and 1381 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 268f886...9778868. Read the comment docs.

@martin-cs martin-cs merged commit 4018bbc into diffblue:develop Feb 15, 2021
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