Case Study: Borrow checker prevents a re-writing of #3337 to use Iterator#3344
Closed
gregorycoppola wants to merge 20 commits intodevelopfrom
Closed
Case Study: Borrow checker prevents a re-writing of #3337 to use Iterator#3344gregorycoppola wants to merge 20 commits intodevelopfrom
Iterator#3344gregorycoppola wants to merge 20 commits intodevelopfrom
Conversation
added 20 commits
October 6, 2022 12:59
* page through mempool results * process fee estimate transactions in sorted order, null estimates in random order * read in a "minimal" form, to avoid unnecessary deserialization * check the nonce in RAM * deserialize and process matching transactions
error[E0277]: `dyn MempoolIterator<'_, Item = MemPoolTxMinimalInfo>` is not an iterator
--> src/core/mempool.rs:1328:32
|
1328 | for tx_reduced_info in db_txs {
| ^^^^^^ `dyn MempoolIterator<'_, Item = MemPoolTxMinimalInfo>` is not an iterator
|
= help: the trait `Iterator` is not implemented for `dyn MempoolIterator<'_, Item = MemPoolTxMinimalInfo>`
= note: required because of the requirements on the impl of `Iterator` for `Box<dyn MempoolIterator<'_, Item = MemPoolTxMinimalInfo>>`
= note: required because of the requirements on the impl of `IntoIterator` for `Box<dyn MempoolIterator<'_, Item = MemPoolTxMinimalInfo>>`
--> src/core/mempool.rs:1192:9
|
1180 | fn sorted_fee_rate_transactions<'connection>(
| ----------- lifetime `'connection` defined here
...
1192 | / Box::new(TransactionPageCursor::<'connection> {
1193 | | connection,
1194 | | base_query: sql.to_string(),
1195 | | current_offset: 0,
1196 | | page_size: 10_000,
1197 | | current_page_remaining: vec![],
1198 | | })
| |__________^ returning this value requires that `'connection` must outlive `'static`
|
help: to declare that the trait object captures data from argument `connection`, you can add an explicit `'connection` lifetime bound
|
1182 | ) -> Box<dyn MempoolIterator<'connection, Item = MemPoolTxMinimalInfo> + 'connection> {
| +++++++++++++
error: lifetime may not live long enough
--> src/core/mempool.rs:1216:9
|
1204 | fn null_fee_rate_transactions<'connection>(
| ----------- lifetime `'connection` defined here
...
1216 | / Box::new(TransactionPageCursor {
1217 | | connection,
1218 | | base_query: sql.to_string(),
1219 | | current_offset: 0,
1220 | | page_size: 10_000,
1221 | | current_page_remaining: vec![],
1222 | | })
| |__________^ returning this value requires that `'connection` must outlive `'static`
|
help: to declare that the trait object captures data from argument `connection`, you can add an explicit `'connection` lifetime bound
|
1206 | ) -> Box<dyn MempoolIterator<'connection, Item = MemPoolTxMinimalInfo> + 'connection> {
| +++++++++++++
warning: variable does not need to be mutable
--> src/core/mempool.rs:1238:13
|
1238 | let mut fee_rate_transactions = Self::sorted_fee_rate_transactions(conn);
| ----^^^^^^^^^^^^^^^^^^^^^
| |
| help: remove this `mut`
warning: variable does not need to be mutable
--> src/core/mempool.rs:1239:13
|
1239 | let mut null_rate_transactions = Self::null_fee_rate_transactions(conn);
| ----^^^^^^^^^^^^^^^^^^^^^^
| |
| help: remove this `mut`
error: lifetime may not live long enough
--> src/core/mempool.rs:1241:9
|
1234 | fn get_transaction_list_to_process<'connection>(
| ----------- lifetime `'connection` defined here
...
1241 | / Box::new(IteratorMixer::create_from(
1242 | | fee_rate_transactions,
1243 | | null_rate_transactions,
1244 | | null_estimate_fraction,
1245 | | ))
| |__________^ returning this value requires that `'connection` must outlive `'static`
|
help: to declare that the trait object captures data from argument `conn`, you can add an explicit `'connection` lifetime bound
|
1237 | ) -> Box<dyn MempoolIterator<'connection, Item = MemPoolTxMinimalInfo> + 'connection> {
…t a time
--> src/core/mempool.rs:1256:38
|
1218 | clarity_tx,
| ---------- first mutable borrow occurs here
...
1225 | for tx_reduced_info in db_txs {
| ------ a temporary with access to the first borrow is created here ...
...
1256 | let inside_result = todo(clarity_tx, &consider, self.cost_estimator.as_mut());
| ^^^^^^^^^^ second mutable borrow occurs here
...
1276 | }
| - ... and the first borrow might be used here, when that temporary is dropped and runs the destructor for type `Box<dyn Iterator<Item = MemPoolTxMinimalInfo>>`
Contributor
Author
|
Just made this PR to record the branch for research purposes. |
Contributor
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is just for research purposes.
It illustrates a case in which the borrow-checker will prevent us from re-writing #3337 into a modular framework based on
Iterator.The pattern that the borrow checker disagrees with is: