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

Implement DoubleEndedIterator for CaseMappingIter #60112

Conversation

KamilaBorowska
Copy link
Contributor

No description provided.

@rust-highfive
Copy link
Collaborator

r? @cramertj

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 19, 2019
@Centril Centril added relnotes Marks issues that should be documented in the release notes of the next release. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 19, 2019
@Centril Centril added this to the 1.36 milestone Apr 19, 2019
@Centril
Copy link
Contributor

Centril commented Apr 19, 2019

r? @Amanieu

@rust-highfive rust-highfive assigned Amanieu and unassigned cramertj Apr 19, 2019
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:04a321ac:start=1555679515669706898,finish=1555679609158484179,duration=93488777281
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
travis_time:start:test_assembly
Check compiletest suite=assembly mode=assembly (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:15:40] 
[01:15:40] running 9 tests
[01:15:40] iiiiiiiii
[01:15:40] 
[01:15:40]  finished in 0.156
[01:15:40] travis_fold:end:test_assembly

---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-both (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:15:57] 
[01:15:57] running 121 tests
[01:16:23] .iiiii...i.....i..i...i..i.i.i..i.ii...i.....i..i....i..........iiii..........i...ii...i.......ii.i. 100/121
[01:16:28] i.i......iii.i.....ii
[01:16:28] 
[01:16:28]  finished in 31.094
[01:16:28] travis_fold:end:test_debuginfo

---
[01:26:47]    Compiling core v0.0.0 (/checkout/src/libcore)
[01:26:52] error[E0277]: `std::string::String` is not an iterator
[01:26:52]   --> src/libcore/../libcore/tests/char.rs:85:36
[01:26:52]    |
[01:26:52] 85 |         assert!(iter.chars().rev().eq(rev_iter));
[01:26:52]    |                                    ^^ `std::string::String` is not an iterator; try calling `.chars()` or `.bytes()`
[01:26:52]    = help: the trait `std::iter::Iterator` is not implemented for `std::string::String`
[01:26:52]    = note: required because of the requirements on the impl of `std::iter::IntoIterator` for `std::string::String`
[01:26:52] 
[01:26:52] error[E0277]: `std::string::String` is not an iterator
[01:26:52] error[E0277]: `std::string::String` is not an iterator
[01:26:52]    --> src/libcore/../libcore/tests/char.rs:114:36
[01:26:52]     |
[01:26:52] 114 |         assert!(iter.chars().rev().eq(rev_iter));
[01:26:52]     |                                    ^^ `std::string::String` is not an iterator; try calling `.chars()` or `.bytes()`
[01:26:52]     = help: the trait `std::iter::Iterator` is not implemented for `std::string::String`
[01:26:52]     = note: required because of the requirements on the impl of `std::iter::IntoIterator` for `std::string::String`
[01:26:52] 
[01:27:01] error: aborting due to 2 previous errors
[01:27:01] error: aborting due to 2 previous errors
[01:27:01] 
[01:27:01] For more information about this error, try `rustc --explain E0277`.
[01:27:01] error: Could not compile `core`.
[01:27:01] 
[01:27:01] To learn more, run the command again with --verbose.
[01:27:01] 
[01:27:01] 
[01:27:01] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "-p" "core" "--" "--quiet"
[01:27:01] 
[01:27:01] 
[01:27:01] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:27:01] Build completed unsuccessfully in 0:23:25
[01:27:01] Build completed unsuccessfully in 0:23:25
[01:27:01] make: *** [check] Error 1
[01:27:01] Makefile:48: recipe for target 'check' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0efa8fa0
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Fri Apr 19 14:40:40 UTC 2019
---
travis_time:end:250152c3:start=1555684842286782040,finish=1555684842361534126,duration=74752086
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:073ddaf0
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:3c707b55
$ dmesg | grep -i kill

I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact @TimNN. (Feature Requests)

@Amanieu
Copy link
Member

Amanieu commented Apr 20, 2019

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Apr 20, 2019

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Apr 20, 2019
@sfackler
Copy link
Member

Could you give an example of when you'd want to use this?

@clarfonthey
Copy link
Contributor

This was denied before in #38968.

@KamilaBorowska
Copy link
Contributor Author

KamilaBorowska commented Apr 21, 2019

There is no real point, it's mostly for consistency, I cannot see the CaseMappingIter ever changing in a way which would cause issues with having a DoubleEndedIterator implementation.

About ExactSizeIterator, it was implemented in #58778 (although ExactSizeIterator is somewhat more useful), which I did implement while considering a potential Clippy lint requiring size_hint for Iterator implementations, and noticing that one didn't have implementation of it while having a reasonable implementation of it. Often it's possible to do better than (0, None) - even something like flat_map could be checking size lower bound hints of iterators it's already processing, and check whether the main iterator is non-empty. Of course, there are exceptions like from_fn which have no useful information for size_hint, but those should be rare.

@sfackler
Copy link
Member

Our policy is generally to not take code that no one wants to use. Every new API imposes some cost, and we should at least be getting some value out of it.

@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label May 1, 2019
@rfcbot
Copy link

rfcbot commented May 1, 2019

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label May 1, 2019
@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels May 11, 2019
@rfcbot
Copy link

rfcbot commented May 11, 2019

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@Centril
Copy link
Contributor

Centril commented May 18, 2019

r? @sfackler

@Centril
Copy link
Contributor

Centril commented May 27, 2019

cc @rust-lang/libs Can someone please review this? :)

@alexcrichton
Copy link
Member

Looks like a review was provided but since there aren't compelling use cases for it and it's otherwise seen as perhaps a bit too much code for libstd for now, I'm going to close this.

@Centril Centril removed the relnotes Marks issues that should be documented in the release notes of the next release. label May 31, 2019
@Centril Centril removed this from the 1.36 milestone May 31, 2019
@KamilaBorowska
Copy link
Contributor Author

KamilaBorowska commented Mar 15, 2022

For future reference (in case somebody found this pull request), this was implemented in #88858.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants