Skip to content

Deprecate make_and in favour of conjunction(expr, expr) #8450

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jun 25, 2025

Conversation

tautschnig
Copy link
Collaborator

make_and does not necessarily produce an and_exprt, so its name is misleading. We already have conjunction(exprt::operandst), and will now have a variant thereof that takes exactly two operands.

  • 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.
  • 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.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@kroening
Copy link
Member

conjunction(a, b) behaves differently from conjunction({a, b}), which is confusing.
How about adding simplified_conjunction(a, b)?

@tautschnig tautschnig self-assigned this Sep 10, 2024
@kroening
Copy link
Member

Related to this, I have been considering to allow and_exprt{} (to mean true) and or_exprt{} (to mean false). Any opinions?

@tautschnig
Copy link
Collaborator Author

conjunction(a, b) behaves differently from conjunction({a, b}), which is confusing. How about adding simplified_conjunction(a, b)?

Addressed by invoking conjunction(a, b) when calling conjunction on a 2-element collection of operands.

@tautschnig
Copy link
Collaborator Author

Related to this, I have been considering to allow and_exprt{} (to mean true) and or_exprt{} (to mean false). Any opinions?

Wouldn't that create the weird case that a constructor creates an object of a different type?

@tautschnig tautschnig force-pushed the no-make_and branch 2 times, most recently from a40d66d to 65235db Compare June 25, 2025 08:46
`make_and` does not necessarily produce an `and_exprt`, so its name is
misleading. We already have `conjunction(exprt::operandst)`, and will
now have a variant thereof that takes exactly two operands.
Copy link

codecov bot commented Jun 25, 2025

Codecov Report

Attention: Patch coverage is 90.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.38%. Comparing base (5cd085b) to head (18761f9).
Report is 2 commits behind head on develop.

Files with missing lines Patch % Lines
src/util/expr_util.cpp 0.00% 1 Missing ⚠️
src/util/std_expr.cpp 93.75% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8450      +/-   ##
===========================================
- Coverage    80.38%   80.38%   -0.01%     
===========================================
  Files         1688     1688              
  Lines       207183   207188       +5     
  Branches        73       73              
===========================================
+ Hits        166549   166550       +1     
- Misses       40634    40638       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@tautschnig tautschnig merged commit c524281 into diffblue:develop Jun 25, 2025
41 checks passed
@tautschnig tautschnig deleted the no-make_and branch June 25, 2025 18:21
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants