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

Eliminate redundant computation in VC experession translation #1009

Merged
merged 2 commits into from
Mar 12, 2025

Conversation

shazqadeer
Copy link
Contributor

@shazqadeer shazqadeer commented Mar 9, 2025

This PR is in response to issue #993 which uncovers unexpected long-running execution during the translation of a large Expr into a VCExpr. My intuition suggested that the expression translation should do linear work unless the expression being translated involves deeply nested inlined function calls. Linear complexity would not exhibit the behavior being seen since the example does not have deeply nested function calls.

The root cause appears to be a piece of code that examines the current term substitution whenever the translation scope crosses a binding (let-binding or quantifier). This examination is done to collect free variables in the substituted expressions to check whether they might collide with the bound variables in the binding about to be translated. In the example, the function ConcatVec is being invoked with increasingly larger expressions for its arguments which makes the substitution mapping formals to actuals increasingly larger. There is a let binding inside ConcatVec which triggers the examination of the increasingly larger substitutions. This ultimately leads to quadratic complexity in translation.

I concluded that the collision detection is unnecessary because it will never detect any collisions in the scenarios where it is being invoked. The substitutions should never refer to the bound variables of the binding being translated. To test my understanding, I ran the Boogie tests with a Debug.Assert capturing my hypothesis. All tests passed.

This PR removes the redundant collision detection code.

@shazqadeer shazqadeer requested a review from RustanLeino March 9, 2025 08:08
Copy link
Contributor

@RustanLeino RustanLeino left a comment

Choose a reason for hiding this comment

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

Your reasoning looks correct to me. That is, I agree that, for a well-formed Boogie program, no collision would be possible. So, it seems fine to remove that logic.

@shazqadeer shazqadeer merged commit 02781d0 into master Mar 12, 2025
5 checks passed
@shazqadeer
Copy link
Contributor Author

Fixes issue #993

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.

None yet

2 participants