Skip to content
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

Independent solver rework #85

Merged
merged 5 commits into from
Sep 14, 2023

Conversation

dim8art
Copy link

@dim8art dim8art commented Apr 29, 2023

Summary:

Checklist:

  • The PR addresses a single issue. If it can be divided into multiple independent PRs, please do so.
  • The PR is divided into a logical sequence of commits OR a single commit is sufficient.
  • There are no unnecessary commits (e.g. commits fixing issues in a previous commit in the same PR).
  • Each commit has a meaningful message documenting what it does.
  • All messages added to the codebase, all comments, as well as commit messages are spellchecked.
  • The code is commented OR not applicable/necessary.
  • The patch is formatted via clang-format OR not applicable (if explicitly overridden leave unchecked and explain).
  • There are test cases for the code you added or modified OR no such test cases are required.

@dim8art dim8art marked this pull request as draft April 29, 2023 01:48
@dim8art dim8art force-pushed the IndependentSolverRework branch 2 times, most recently from 38fc182 to 5c1c074 Compare May 15, 2023 11:41
@dim8art dim8art marked this pull request as ready for review May 15, 2023 11:50
@dim8art dim8art force-pushed the IndependentSolverRework branch 5 times, most recently from cdb3b3b to 0d0fcc1 Compare August 4, 2023 08:02
@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2023

Codecov Report

Merging #85 (5555b3e) into main (9406351) will increase coverage by 0.22%.
The diff coverage is 83.20%.

❗ Current head 5555b3e differs from pull request most recent head 5937f8b. Consider uploading reports for the commit 5937f8b to get more accurate results

@@            Coverage Diff             @@
##             main      #85      +/-   ##
==========================================
+ Coverage   65.37%   65.60%   +0.22%     
==========================================
  Files         211      213       +2     
  Lines       28676    29018     +342     
  Branches     6374     6437      +63     
==========================================
+ Hits        18746    19036     +290     
- Misses       7442     7519      +77     
+ Partials     2488     2463      -25     
Files Changed Coverage Δ
include/klee/Expr/Constraints.h 50.00% <ø> (ø)
include/klee/Expr/Expr.h 82.30% <0.00%> (-1.24%) ⬇️
lib/Core/SeedInfo.cpp 9.78% <0.00%> (-0.45%) ⬇️
lib/Solver/CachingSolver.cpp 66.31% <ø> (+1.91%) ⬆️
lib/Solver/FastCexSolver.cpp 30.39% <ø> (+0.05%) ⬆️
lib/Solver/Solver.cpp 41.04% <0.00%> (ø)
lib/Solver/Z3Solver.cpp 69.52% <ø> (+0.57%) ⬆️
lib/Solver/QueryLoggingSolver.cpp 62.80% <37.50%> (+0.38%) ⬆️
lib/Expr/Assignment.cpp 46.55% <41.17%> (+1.09%) ⬆️
lib/Solver/AssignmentValidatingSolver.cpp 30.23% <50.00%> (+1.01%) ⬆️
... and 17 more

... and 9 files with indirect coverage changes

@dim8art dim8art force-pushed the IndependentSolverRework branch 2 times, most recently from eaf1337 to a26c72d Compare August 7, 2023 09:41
@dim8art dim8art force-pushed the IndependentSolverRework branch 5 times, most recently from 0e8b5e3 to 1d4b9bd Compare August 19, 2023 12:21
bool allowFreeValues) {
AssignmentEvaluator v(*this, allowFreeValues);
for (; begin != end; ++begin) {
if ((*begin)->getWidth() == Expr::Bool && !v.visit(*begin)->isTrue())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please separate the logic for processing non-boolean expressions into a separate function


#undef ITEMSIZE
#undef ALPHA
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put a new line

if (*it1 != *it2)
return false;
}
return size() == b.size();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Swap the size equality condition with the element equality conditions for better performance

if (*it1 > *it2)
return false;
}
return size() < b.size();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above

disjointSets_ty ds() const { return disjointSets; }
};
} // namespace klee
#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put a new line

@@ -27,45 +28,44 @@ using symcretes_ty = SymcreteOrderedSet;

class Assignment {
public:
typedef std::map<const Array *, SparseStorage<unsigned char>> bindings_ty;
using bindings_ty = ImmutableMap<const Array *, SparseStorage<unsigned char>>;
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use PersistentMap instead of ImmutableMap, since the persistent version is a wrapper for convenience

@@ -328,6 +328,13 @@ class Expr {
// Given an array of new kids return a copy of the expression
// but using those children.
virtual ref<Expr> rebuild(ref<Expr> kids[/* getNumKids() */]) const = 0;
virtual ref<Expr> rebuild() const {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this the code used?

};
} // namespace klee

#endif
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put a new line

icsu.reEvaluateConcretization(newConcretization);
return icsu.getConcretizedVersion();
}
} // namespace klee
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put a new line

@@ -132,7 +132,8 @@ struct isValidOrSatisfyingResponse {
isValidOrSatisfyingResponse(KeyType &_key) : key(_key) {}

bool operator()(ref<SolverResponse> a) const {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Split keys in boolean keys and non-boolean keys and update the predicate accordingly

@misonijnik misonijnik merged commit f72486f into UnitTestBot:main Sep 14, 2023
20 checks passed
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