Skip to content

C++: Handle switch statements in the guards library #15941

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 9 commits into from
Mar 18, 2024

Conversation

MathiasVP
Copy link
Contributor

@MathiasVP MathiasVP commented Mar 15, 2024

This PR introduces the minimal changes necessary to support switch statements in the guards library. After this PR, it's now possible to ask if a block is controlled by a particular case in a switch statement.

It's still possible to do more in this area. For example, GuardCondition.comparesEq still doesn't reason about switch statements since the interface on that predicate can't really support it (its columns include (Expr left, Expr right, int k) which holds if left == right + k, but the case label in a switch isn't an expression). So such changes are probably best done in a follow-up PR.

Commit-by-commit review recommended.

DCA looks good. The removed results are clear FPs that now disappear. And the new results is because the medium precision query now shows these results since they're no longer results in the high precision query (😭).

@github-actions github-actions bot added the C++ label Mar 15, 2024
@MathiasVP MathiasVP marked this pull request as ready for review March 18, 2024 09:51
@MathiasVP MathiasVP requested a review from a team as a code owner March 18, 2024 09:51
@jketema
Copy link
Contributor

jketema commented Mar 18, 2024

DCA looks good.

What about the Kamalio jolin order badness?

@MathiasVP
Copy link
Contributor Author

DCA looks good.

What about the Kamalio jolin order badness?

Sadly, that's a bug in the join order badness that we haven't been able to understand :( This one constantly shows up in DCA runs, and I haven't been able to figure out what's going on.

Copy link
Contributor

@jketema jketema left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants