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

Remove ExactSizeIterator constraint from SimpleSmt::with_leaves() #228

Merged
merged 7 commits into from
Nov 28, 2023

Conversation

plafer
Copy link
Contributor

@plafer plafer commented Nov 27, 2023

The ExactSizeIterator constraint forces the caller to instantiate iterators into, say, a Vec, which leads to useless memory allocations. The signature of the function is now cleaner as well.

The one downside of this approach is that in the unlikely case that the collection indeed has more than 2^63 elements, then we will do a lot of processing before figuring that out. However, this will almost never happen, and we should optimize for the case where the iterator doesn't have too many elements.

Copy link
Contributor

@bobbinth bobbinth left a comment

Choose a reason for hiding this comment

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

Looks good! Thank you! Let's resolve the merge conflict (which appeared after I merged #224), squash the commits, and merge.

}
// compute the max number of entries. We use an upper bound of depth 63 because we consider
// passing in a vector of size 2^64 infeasible.
let max_num_entries = 2usize.pow(tree.depth.min(63).into());
Copy link
Contributor

Choose a reason for hiding this comment

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

minor nit: we usually write 2_usize instead of 2usize.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@plafer plafer merged commit a7791b4 into next Nov 28, 2023
11 checks passed
@plafer plafer deleted the plafer-remove-exactsizeiterator branch November 28, 2023 20:34
bobbinth pushed a commit that referenced this pull request Dec 21, 2023
…#228)

* Change InvalidNumEntries error

* max computation

* remove length check

* remove ExactSizeIterator constraint

* fix InvalidNumEntries error condition

* 2_usize
bobbinth pushed a commit that referenced this pull request Feb 14, 2024
…#228)

* Change InvalidNumEntries error

* max computation

* remove length check

* remove ExactSizeIterator constraint

* fix InvalidNumEntries error condition

* 2_usize
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.

2 participants