Skip to content

Conversation

@aschackmull
Copy link
Contributor

@aschackmull aschackmull commented Nov 11, 2025

Improves the asymptotic performance of wrapper guards by applying the usual rank-iteration tranformation to a recursive universal quantification.

An observed example of the form

int foo(string x) {
  switch(x) {
     case "1" : return 1;
     case "2" : return 2;
     ...
     case "1000" : return 1000;
     default: return 0;
  }
}

goes from O(n^3) to O(n^2) to evaluate the O(n) result (the set of implications from return value to parameter value seeded with return being zero or not-zero).

@aschackmull aschackmull marked this pull request as ready for review November 12, 2025 08:44
@aschackmull aschackmull requested a review from a team as a code owner November 12, 2025 08:44
Copilot AI review requested due to automatic review settings November 12, 2025 08:44
Copilot finished reviewing on behalf of aschackmull November 12, 2025 08:46
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the asymptotic performance of wrapper guards by applying a rank-iteration transformation to a recursive universal quantification. The optimization changes the complexity from O(n³) to O(n²) when evaluating O(n) results for methods with many return expressions (e.g., a switch statement with 1000 cases).

Key Changes

  • Introduces dense ranking for return expressions based on their location to enable incremental processing
  • Replaces a universal quantification (forex) with a rank-based recursion that iterates through return expressions in order
  • Adds helper predicates to track relevance of return expressions and their values

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Nov 12, 2025
Copy link
Contributor

@hvitved hvitved left a comment

Choose a reason for hiding this comment

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

LGTM, two comments.

validReturnInCustomGuard(_, _, m, ppos, retval, val) and rnk = 0
or
validReturnInCustomGuardToRank(rnk - 1, m, ppos, retval, val) and
rnk <= maxRank(m) and
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this line needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. The condition for iterating to the next rank is otherwise vacuously true, and then we'll get "infinite" tuples.

private predicate validReturnInCustomGuardToRank(
int rnk, NonOverridableMethod m, ParameterPosition ppos, GuardValue retval, GuardValue val
) {
validReturnInCustomGuard(_, _, m, ppos, retval, val) and rnk = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

It's the validReturnInCustomGuard(_, _, m, ppos, retval, val) that ensures that this becomes a forex and not a forall, right? If so, that might deserve a comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. And the fact that we've pushed the constraint

          not exists(GuardValue notRetval |
            exprHasValue(ret, notRetval) and
            disjointValues(notRetval, retval)
          )

into it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a commit with an elaborating comment.

@aschackmull aschackmull merged commit a28a718 into github:main Nov 13, 2025
24 of 36 checks passed
@aschackmull aschackmull deleted the guards/wrapper-perf branch November 13, 2025 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants