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

Refactoring use common code between option, result and accum #62883

Merged
merged 1 commit into from
Jul 28, 2019
Merged

Refactoring use common code between option, result and accum #62883

merged 1 commit into from
Jul 28, 2019

Conversation

Stargateur
Copy link
Contributor

Option and Result have almost exactly the same code that in accum.rs that implement Sum and Product. This PR just move some code to use the same code for all of them. I believe is better to not implement this Iterator feature twice.

I'm not very familiar with pub visibility hope I didn't make then public. However, maybe these adapters could be useful and we could think to make then pub.

#59605
#11084

r? @pnkfelix

@Stargateur Stargateur changed the title Refactoring use commun code between option, result and accum Refactoring use common code between option, result and accum Jul 22, 2019
@Stargateur
Copy link
Contributor Author

Stargateur commented Jul 23, 2019

I get an idea from #58975 (comment)

/// An iterator adapter that produces output as long as the underlying
/// iterator produces `Option::Some` values.
pub(crate) struct OptionShunt;

impl OptionShunt
{
    /// Process the given iterator as if it yielded a `T` instead of a
    /// `Option<T>`. Any `None` value will stop the inner iterator and
    /// the overall result will be a `None`.
    pub fn process<I, T, F, U, O>(iter: I, f: F) -> Option<U>
    where
        I: Iterator<Item = Option<T>>,
        O: Iterator<Item = Result<T, ()>>,
        F: FnMut(&mut ResultShunt<O, ()>) -> U,
    {
        ResultShunt::process(iter.map(|x| x.ok_or(())), f).ok()
    }
}

but I don't make it to compile, I don't think it's possible:

error[E0282]: type annotations needed for `&mut iter::adapters::ResultShunt<O, ()>`
   --> src/libcore/iter/traits/accum.rs:183:37
    |
183 |         OptionShunt::process(iter, |i| i.sum())
    |                                     ^ consider giving this closure parameter the explicit type `&mut iter::adapters::ResultShunt<O, ()>`, where the type parameter `O` is specified

error[E0277]: expected a `ops::function::FnMut<(&mut iter::adapters::ResultShunt<iter::adapters::Map<I, [closure@src/libcore/iter/adapters/mod.rs:2081:39: 2081:54]>, ()>,)>` closure, found `F`
    --> src/libcore/iter/adapters/mod.rs:2081:9
     |
2081 |         ResultShunt::process(iter.map(|x| x.ok_or(())), f).ok()
     |         ^^^^^^^^^^^^^^^^^^^^ expected an `FnMut<(&mut iter::adapters::ResultShunt<iter::adapters::Map<I, [closure@src/libcore/iter/adapters/mod.rs:2081:39: 2081:54]>, ()>,)>` closure, found `F`
     |
     = help: the trait `for<'r> ops::function::FnMut<(&'r mut iter::adapters::ResultShunt<iter::adapters::Map<I, [closure@src/libcore/iter/adapters/mod.rs:2081:39: 2081:54]>, ()>,)>` is not implemented for `F`
     = help: consider adding a `where for<'r> F: ops::function::FnMut<(&'r mut iter::adapters::ResultShunt<iter::adapters::Map<I, [closure@src/libcore/iter/adapters/mod.rs:2081:39: 2081:54]>, ()>,)>` bound
note: required by `iter::adapters::ResultShunt::<I, E>::process`
    --> src/libcore/iter/adapters/mod.rs:2102:5
     |
2102 | /     pub fn process<F, U>(iter: I, mut f: F) -> Result<U, E>
2103 | |         where F: FnMut(&mut Self) -> U
2104 | |     {
2105 | |         let mut shunt = ResultShunt::new(iter);
2106 | |         let value = f(shunt.by_ref());
2107 | |         shunt.reconstruct(value)
2108 | |     }
     | |_____^

error[E0277]: expected a `ops::function::FnOnce<(&mut iter::adapters::ResultShunt<iter::adapters::Map<I, [closure@src/libcore/iter/adapters/mod.rs:2081:39: 2081:54]>, ()>,)>` closure, found `F`
    --> src/libcore/iter/adapters/mod.rs:2081:9
     |
2081 |         ResultShunt::process(iter.map(|x| x.ok_or(())), f).ok()
     |         ^^^^^^^^^^^^^^^^^^^^ expected an `FnOnce<(&mut iter::adapters::ResultShunt<iter::adapters::Map<I, [closure@src/libcore/iter/adapters/mod.rs:2081:39: 2081:54]>, ()>,)>` closure, found `F`
     |
     = help: the trait `ops::function::FnOnce<(&mut iter::adapters::ResultShunt<iter::adapters::Map<I, [closure@src/libcore/iter/adapters/mod.rs:2081:39: 2081:54]>, ()>,)>` is not implemented for `F`
     = help: consider adding a `where F: ops::function::FnOnce<(&mut iter::adapters::ResultShunt<iter::adapters::Map<I, [closure@src/libcore/iter/adapters/mod.rs:2081:39: 2081:54]>, ()>,)>` bound
note: required by `iter::adapters::ResultShunt::<I, E>::process`
    --> src/libcore/iter/adapters/mod.rs:2102:5
     |
2102 | /     pub fn process<F, U>(iter: I, mut f: F) -> Result<U, E>
2103 | |         where F: FnMut(&mut Self) -> U
2104 | |     {
2105 | |         let mut shunt = ResultShunt::new(iter);
2106 | |         let value = f(shunt.by_ref());
2107 | |         shunt.reconstruct(value)
2108 | |     }
     | |_____^

Also, I'm not sure it better to do this.

@bors

This comment has been minimized.

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 26, 2019
@scottmcm scottmcm added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 27, 2019
@scottmcm
Copy link
Member

r? @scottmcm

(Per #62459 (comment))

@rust-highfive rust-highfive assigned scottmcm and unassigned pnkfelix Jul 27, 2019
@scottmcm
Copy link
Member

This seems like a good simplification to me. And pub(crate) is fine -- tidy would have yelled at you if you'd accidentally exposed something without a stability marker.

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jul 27, 2019

📌 Commit 3334802 has been approved by scottmcm

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 27, 2019
Centril added a commit to Centril/rust that referenced this pull request Jul 28, 2019
…scottmcm

Refactoring use common code between option, result and accum

`Option` and `Result` have almost exactly the same code that in `accum.rs` that implement `Sum` and `Product`. This PR just move some code to use the same code for all of them. I believe is better to not implement this `Iterator` feature twice.

I'm not very familiar with pub visibility hope I didn't make then public. However, maybe these adapters could be useful and we could think to make then pub.

rust-lang#59605
rust-lang#11084

r? @pnkfelix
bors added a commit that referenced this pull request Jul 28, 2019
Rollup of 8 pull requests

Successful merges:

 - #61207 (Allow lifetime elision in `Pin<&(mut) Self>`)
 - #62074 (squash of all commits for nth_back on ChunksMut)
 - #62771 (Break dependencies between `syntax_ext` and other crates)
 - #62883 (Refactoring use common code between option, result and accum)
 - #62949 (Re-enable assertions in PPC dist builder)
 - #62996 (tidy: Add a check for inline unit tests)
 - #63038 (Make more informative error on outer attribute after inner)
 - #63050 (ci: download awscli from our mirror)

Failed merges:

r? @ghost
@bors bors merged commit 3334802 into rust-lang:master Jul 28, 2019
@Stargateur Stargateur deleted the refactoring-adapters branch July 28, 2019 05:07
hkBst added a commit to hkBst/rust that referenced this pull request Sep 28, 2022
Remove a FIXME whose code got moved away in rust-lang#62883.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 2, 2022
Remove a FIXME whose code got moved away in rust-lang#62883.

Remove a FIXME whose code got moved away in rust-lang#62883.
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 2, 2022
…iaskrgr

Rollup of 6 pull requests

Successful merges:

 - rust-lang#102195 (Improve the COPYRIGHT file)
 - rust-lang#102313 (Update docs so that deprecated method points to relevant method)
 - rust-lang#102353 (Allow passing rustix_use_libc cfg using RUSTFLAGS)
 - rust-lang#102405 (Remove a FIXME whose code got moved away in rust-lang#62883.)
 - rust-lang#102525 (rustdoc: remove orphaned link on array bracket)
 - rust-lang#102557 (fix issue with x.py setup running into explicit panic)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Feb 10, 2023
Remove a FIXME whose code got moved away in rust-lang/rust#62883.
thomcc pushed a commit to tcdi/postgrestd that referenced this pull request Feb 10, 2023
Remove a FIXME whose code got moved away in #62883.

Remove a FIXME whose code got moved away in rust-lang/rust#62883.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants