Skip to content

Log to file the SMT formula run by the solver #7161

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

Conversation

esteffin
Copy link
Contributor

@esteffin esteffin commented Sep 28, 2022

This PR introduces the new --dump-smt-formula argument that logs to a file the SMT formula given to the incremental SMT solver.
This is different from the old --outfile argument as it does not run the solver, and, because of that, stops the generated file to the first (check-sat) call.

This PR requires the work to support --outfile that will be added by PR 7126, so it is dependant to its merge.

  • 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 Sep 28, 2022

Codecov Report

Base: 78.36% // Head: 78.38% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (481c43e) compared to base (f025c91).
Patch coverage: 79.92% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #7161      +/-   ##
===========================================
+ Coverage    78.36%   78.38%   +0.02%     
===========================================
  Files         1651     1651              
  Lines       190009   190058      +49     
===========================================
+ Hits        148896   148973      +77     
+ Misses       41113    41085      -28     
Impacted Files Coverage Δ
src/analyses/constant_propagator.h 82.35% <ø> (ø)
src/analyses/invariant_set.cpp 0.00% <0.00%> (ø)
src/analyses/invariant_set.h 0.00% <0.00%> (ø)
src/ansi-c/c_expr.h 100.00% <ø> (ø)
src/ansi-c/c_typecheck_base.h 100.00% <ø> (ø)
src/ansi-c/scanner.l 63.39% <ø> (ø)
src/ansi-c/ansi_c_convert_type.cpp 79.48% <8.16%> (ø)
.../src/java_bytecode/character_refine_preprocess.cpp 54.27% <28.57%> (ø)
src/analyses/local_may_alias.cpp 63.80% <33.33%> (ø)
src/analyses/custom_bitvector_analysis.cpp 55.96% <50.00%> (ø)
... and 135 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@thomasspriggs thomasspriggs force-pushed the esteffin/add-dump-smt-formula-arg branch from dcb3aa7 to 2bdda65 Compare October 10, 2022 17:11
@esteffin esteffin force-pushed the esteffin/add-dump-smt-formula-arg branch from 2bdda65 to 2236012 Compare December 7, 2022 16:22
@esteffin esteffin marked this pull request as ready for review December 7, 2022 16:25
Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

Ok with the changes so far, but you'll need to update the man page to make CI pass.

Comment on lines +334 to +335
message_handlert &message_handler,
const std::string &arg_name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Random place for comment: the commit message needs to be reviewed (spurious "---------------------", missing words after "that is being").

--dump-smt-formula logs the full SMT formula that is being passed to the solver to a file while also running the solver.
@esteffin esteffin force-pushed the esteffin/add-dump-smt-formula-arg branch from 2236012 to 9b160e4 Compare December 8, 2022 16:47
@esteffin esteffin requested a review from jimgrundy as a code owner December 8, 2022 16:47
@esteffin esteffin force-pushed the esteffin/add-dump-smt-formula-arg branch from 9b160e4 to 6174266 Compare December 8, 2022 17:16
@esteffin esteffin force-pushed the esteffin/add-dump-smt-formula-arg branch from 6174266 to 481c43e Compare December 8, 2022 17:26
Copy link
Contributor

@thomasspriggs thomasspriggs left a comment

Choose a reason for hiding this comment

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

I am not particularly satisfied with the code duplication this PR introduces between the smt_piped_solver_processt class and the smt_incremental_dry_run_solvert class. However I'd prefer to get this feature merged and worry about clean-up afterwards. I have a potential refactor to resolve this, which I will raise as a follow-up myself.

@thomasspriggs thomasspriggs merged commit 5d603e4 into diffblue:develop Dec 12, 2022
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