Skip to content

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Mar 11, 2025

Sadly, we cannot put the cached annotation directly on the existing toString predicate, as that introduces spurious non-monotonicity errors (the non-monotonicity checker considers cached predicates to be black-boxes). Instead, we add an intermediate toStringImpl predicate, which acts like the previous toString predicate, and then cache the new toString predicate, which simply forwards to toStringImpl.

DCA shows a nice average 7% analysis time speedup.

@github-actions github-actions bot added Rust Pull requests that update Rust code Swift labels Mar 11, 2025
module Impl {
class Decl extends Generated::Decl {
override string toString() { result = super.toString() }
override string toStringImpl() { result = super.toStringImpl() }

Check warning

Code scanning / CodeQL

Redundant override Warning

Redundant override of
this predicate
.
@hvitved hvitved force-pushed the rust/cache-to-string branch 3 times, most recently from 5ab442f to 03a87de Compare March 13, 2025 12:07
@hvitved hvitved added the no-change-note-required This PR does not need a change note label Mar 13, 2025
@hvitved hvitved marked this pull request as ready for review March 13, 2025 19:00
@hvitved hvitved requested a review from a team as a code owner March 13, 2025 19:00
@hvitved hvitved changed the title Rust: Cache Element.toString Rust/Swift: Cache Element.toString Mar 13, 2025
@hvitved hvitved requested a review from redsun82 March 13, 2025 19:16
redsun82
redsun82 previously approved these changes Mar 14, 2025
Copy link
Contributor

@redsun82 redsun82 left a comment

Choose a reason for hiding this comment

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

This is nice, thanks!

@hvitved
Copy link
Contributor Author

hvitved commented Mar 14, 2025

Rebased to resolve merge conflict.

@hvitved hvitved requested a review from redsun82 March 14, 2025 11:38
@redsun82
Copy link
Contributor

Swift integration tests failure is a timeout flake, merging this now

@redsun82 redsun82 merged commit a2851f7 into github:main Mar 17, 2025
24 of 25 checks passed
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 Rust Pull requests that update Rust code Swift

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants