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

Handle quantifiers with statement expressions #8605

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

qinheping
Copy link
Collaborator

This pull request addresses the issue of handling quantifiers that contain statement expressions in CBMC. This fix ensures that CBMC can correctly process and verify programs that use quantifiers with statement expressions.

Changes

  • goto_clean_expr.cpp: Added logic to handle quantifiers with statement expressions. This includes:
    • A new function find_base_symbol to identify the base symbol in an expression.
    • Modifications to clean_expr to handle quantifiers with statement expressions by converting them pure expressions.
  • c_typecheck_expr.cpp: Updated to allow quantifiers with statement expressions.
  • Tests: Introduced new tests to verify the correct handling of quantifiers with statement expressions. These tests ensure that CBMC can process and verify programs that use such quantifiers without errors.

Motivation and Context

Statement expressions may not contain side effects, and should be accepted as quantifier bodies when they don't.
Supporting quantifiers with statement expressions enhances CBMC's capability to handle a broader range of programs, e.g., GOTO program compiled from Rust MIR model-checking/kani#3737.

  • 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.

@qinheping qinheping force-pushed the quantifiers-with-statement-expressions branch 2 times, most recently from 5fe1c65 to f9f9581 Compare March 8, 2025 07:30
@qinheping qinheping changed the title Fix: Handle quantifiers with statement expressions in CBMC Handle quantifiers with statement expressions Mar 8, 2025
@qinheping qinheping force-pushed the quantifiers-with-statement-expressions branch 2 times, most recently from 2c11ad1 to 7e30d2c Compare March 10, 2025 06:05
@kroening
Copy link
Member

I am sure it makes sense to extend what's allowed in quantifiers. I am surprised by the choice of a (fragment) of statement expressions, which are a gcc extension at the C language level. In particular, note that this means you can't actually use these in Kani.

The examples suggest that you are perhaps looking for let expressions? These are unproblematic to use in quantifiers, and Kani can use these right away.

@qinheping
Copy link
Collaborator Author

In particular, note that this means you can't actually use these in Kani.

Kani is actually the motivation of support statement expressions in quantifiers. In Kani, we generate GOTO programs from Rust MIR. However, all Rust expressions will be compiled into a sequence of statements (including decl, assign, and conditional goto) in Rust MIR. We typically wrap all such statements into a closure function (in Rust) so that the evaluation of the Rust expressions becomes a single function call in GOTO. This approach doesn't work here because function calls are also side-effect expressions in GOTO. Therefore, we are considering inlining all function calls in MIR and generating a statement expression in GOTO from the inlined expression.

I don't think let-expressions are enough here because there will still be function calls or statement expressions in the where clauses of the let-expressions.

@kroening
Copy link
Member

Kani is actually the motivation of support statement expressions in quantifiers. In Kani, we generate GOTO programs from Rust MIR. However, all Rust expressions will be compiled into a sequence of statements (including decl, assign, and conditional goto) in Rust MIR. We typically wrap all such statements into a closure function (in Rust) so that the evaluation of the Rust expressions becomes a single function call in GOTO. This approach doesn't work here because function calls are also side-effect expressions in GOTO. Therefore, we are considering inlining all function calls in MIR and generating a statement expression in GOTO from the inlined expression.

I don't think let-expressions are enough here because there will still be function calls or statement expressions in the where clauses of the let-expressions.

Ok, then how about defining the concept of a "pure" function, and then allowing those in quantifiers?

We need that in the front-end anyway, for C++'s constexpr.

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.

A few initial comments, still need to work through the core piece of the implementation (see my "Note to self").

@tautschnig
Copy link
Collaborator

We need that in the front-end anyway, for C++'s constexpr.

But then constexpr has C++-standards-version-specific peculiarities, so a single definition might not work? I had pointed @qinheping to my constexpr implemention in #8386 as an initial guidance, though.

@qinheping
Copy link
Collaborator Author

Ok, then how about defining the concept of a "pure" function, and then allowing those in quantifiers?

In the design of this PR, no function calls are allowed in quantifiers.

@qinheping qinheping force-pushed the quantifiers-with-statement-expressions branch from 35a0665 to 449b771 Compare March 11, 2025 20:32
@qinheping qinheping force-pushed the quantifiers-with-statement-expressions branch from 449b771 to 7969993 Compare March 12, 2025 05:59
Copy link

codecov bot commented Mar 12, 2025

Codecov Report

Attention: Patch coverage is 65.33333% with 52 lines in your changes missing coverage. Please review.

Project coverage is 79.17%. Comparing base (f55f636) to head (7969993).
Report is 4 commits behind head on develop.

Files with missing lines Patch % Lines
src/ansi-c/goto-conversion/goto_clean_expr.cpp 64.58% 51 Missing ⚠️
src/ansi-c/c_typecheck_expr.cpp 83.33% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8605      +/-   ##
===========================================
- Coverage    79.63%   79.17%   -0.46%     
===========================================
  Files         1733     1733              
  Lines       197704   198973    +1269     
  Branches     17963    17967       +4     
===========================================
+ Hits        157438   157536      +98     
- Misses       40266    41437    +1171     

☔ 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.

@qinheping qinheping force-pushed the quantifiers-with-statement-expressions branch from 68f15c9 to d8d55ec Compare March 12, 2025 06:57
This commit addresses the issue of handling quantifiers that contain statement expressions in CBMC. Previously, quantifiers without side effects were not supported.

Changes include:
- Added logic to handle quantifiers with statement expressions in `goto_clean_expr.cpp`.
- Updated `c_typecheck_expr.cpp` to allow quantifiers with statement expressions.
- Introduced new tests to verify the correct handling of quantifiers with statement expressions.
@qinheping qinheping force-pushed the quantifiers-with-statement-expressions branch from d8d55ec to 5cdfb0b Compare March 12, 2025 07:08
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