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

Add Rev delegate methods for the impl of Iterator. #57245

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
5 participants
@czipperz
Copy link
Contributor

czipperz commented Jan 1, 2019

No description provided.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 1, 2019

r? @TimNN

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

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 1, 2019

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:006642a6:start=1546315616515119080,finish=1546315617518097538,duration=1002978458
$ 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
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:03:16]    Compiling rustc_lsan v0.0.0 (/checkout/src/librustc_lsan)
[00:03:17]    Compiling rustc_msan v0.0.0 (/checkout/src/librustc_msan)
[00:03:17]    Compiling rustc_asan v0.0.0 (/checkout/src/librustc_asan)
[00:03:17]    Compiling rustc_tsan v0.0.0 (/checkout/src/librustc_tsan)
[00:03:19] error[E0276]: impl has stricter requirements than trait
[00:03:19]      |
[00:03:19]      |
[00:03:19] 458  | /     fn rposition<P>(&mut self, predicate: P) -> Option<usize> where
[00:03:19] 459  | |         P: FnMut(<I as Iterator>::Item) -> bool
[00:03:19] 460  | |     {
[00:03:19] 461  | |         self.iter.position(predicate)
[00:03:19] 462  | |     }
[00:03:19]      | |_____^ impl has extra requirement `P: ops::function::FnMut<(<I as iter::iterator::Iterator>::Item,)>`
[00:03:19]     ::: src/libcore/iter/iterator.rs:1978:5
[00:03:19]      |
[00:03:19]      |
[00:03:19] 1978 | /     fn rposition<P>(&mut self, mut predicate: P) -> Option<usize> where
[00:03:19] 1979 | |         P: FnMut(Self::Item) -> bool,
[00:03:19] 1980 | |         Self: Sized + ExactSizeIterator + DoubleEndedIterator
[00:03:19] ...    |
[00:03:19] ...    |
[00:03:19] 1989 | |         }).break_value()
[00:03:19] 1990 | |     }
[00:03:19]      | |_____- definition of `rposition` from trait
[00:03:19] error: aborting due to previous error
[00:03:19] 
[00:03:19] For more information about this error, try `rustc --explain E0276`.
[00:03:19] error: Could not compile `core`.
[00:03:19] error: Could not compile `core`.
[00:03:19] warning: build failed, waiting for other jobs to finish...
[00:03:21] error: build failed
[00:03:21] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json"
[00:03:21] expected success, got: exit code: 101
[00:03:21] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:03:21] Build completed unsuccessfully in 0:00:15
[00:03:21] make: *** [all] Error 1
[00:03:21] Makefile:18: recipe for target 'all' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:2ae5a54a
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Tue Jan  1 04:10:27 UTC 2019
---
travis_time:end:02638dbe:start=1546315827766340379,finish=1546315827771644586,duration=5304207
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:00963399
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:1d965cc8
travis_time:start:1d965cc8
$ 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:02ee4b99
$ 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)

@czipperz

This comment has been minimized.

Copy link
Contributor

czipperz commented Jan 1, 2019

This does three things:

  1. Implement the following methods trivially: count, last, and position.
  2. Mark all delegates as #[inline].
  3. Replace <I as Iterator>::Item with Self::Item.

@czipperz czipperz force-pushed the czipperz:add_rev_iterator_methods branch from f637ab5 to d752f81 Jan 1, 2019

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 1, 2019

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:13b2a450:start=1546318715630896839,finish=1546318716697156347,duration=1066259508
$ 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
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:03:10]    Compiling rustc_lsan v0.0.0 (/checkout/src/librustc_lsan)
[00:03:10]    Compiling rustc_tsan v0.0.0 (/checkout/src/librustc_tsan)
[00:03:11]    Compiling rustc_msan v0.0.0 (/checkout/src/librustc_msan)
[00:03:11]    Compiling rustc_asan v0.0.0 (/checkout/src/librustc_asan)
[00:03:12] error[E0276]: impl has stricter requirements than trait
[00:03:12]      |
[00:03:12]      |
[00:03:12] 451  | /     fn rposition<P>(&mut self, predicate: P) -> Option<usize> where
[00:03:12] 452  | |         P: FnMut(<I as Iterator>::Item) -> bool
[00:03:12] 453  | |     {
[00:03:12] 454  | |         self.iter.position(predicate)
[00:03:12] 455  | |     }
[00:03:12]      | |_____^ impl has extra requirement `P: ops::function::FnMut<(<I as iter::iterator::Iterator>::Item,)>`
[00:03:12]     ::: src/libcore/iter/iterator.rs:1978:5
[00:03:12]      |
[00:03:12]      |
[00:03:12] 1978 | /     fn rposition<P>(&mut self, mut predicate: P) -> Option<usize> where
[00:03:12] 1979 | |         P: FnMut(Self::Item) -> bool,
[00:03:12] 1980 | |         Self: Sized + ExactSizeIterator + DoubleEndedIterator
[00:03:12] ...    |
[00:03:12] ...    |
[00:03:12] 1989 | |         }).break_value()
[00:03:12] 1990 | |     }
[00:03:12]      | |_____- definition of `rposition` from trait
[00:03:13] error: aborting due to previous error
[00:03:13] 
[00:03:13] For more information about this error, try `rustc --explain E0276`.
[00:03:13] error: Could not compile `core`.
[00:03:13] error: Could not compile `core`.
[00:03:13] warning: build failed, waiting for other jobs to finish...
[00:03:14] error: build failed
[00:03:14] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" "panic-unwind backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json"
[00:03:14] expected success, got: exit code: 101
[00:03:14] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:03:14] Build completed unsuccessfully in 0:00:14
[00:03:14] Makefile:18: recipe for target 'all' failed
[00:03:14] make: *** [all] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:026aed32
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Tue Jan  1 05:01:59 UTC 2019
---
travis_time:end:06b33ad0:start=1546318920239129385,finish=1546318920244338508,duration=5209123
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:15d876e6
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0daebf3d
travis_time:start:0daebf3d
$ 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:17d5c800
$ 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)

@czipperz

This comment has been minimized.

Copy link
Contributor

czipperz commented Jan 1, 2019

The third is negotiable. It breaks multiple methods to use <I as Iterator>::Item whereas Self::Item works great and is more consistent.

@clarcharr

This comment has been minimized.

Copy link
Contributor

clarcharr commented Jan 2, 2019

These change the execution order and cannot be merged as-is, unfortunately:

let iter = || (1..=10).map(|x| println!("{}", x));
iter().count(); // prints 1-10
iter().rev().count(); // prints 10-1
iter().next(); // prints 1
iter().rev().last(); // prints 10-1
@czipperz

This comment has been minimized.

Copy link
Contributor

czipperz commented Jan 2, 2019

@clarcharr That's a great point. I'll make a PR that makes tests for these.

@czipperz

This comment has been minimized.

Copy link
Contributor

czipperz commented Jan 2, 2019

I'm keeping the PR open for right now so I remember to work on this.

@Dylan-DPC

This comment has been minimized.

Copy link
Member

Dylan-DPC commented Jan 21, 2019

ping from triage @czipperz any updates on this?

@czipperz

This comment has been minimized.

Copy link
Contributor

czipperz commented Jan 21, 2019

@Dylan-DPC I'm working on other things right now. Do you think I should add tests ensuring this functionality or it is fine as is?

@czipperz

This comment has been minimized.

Copy link
Contributor

czipperz commented Jan 21, 2019

The only method addition that would improve performance without breaking functionality was position, but I couldn't get that to compile because it would add new generic bounds.

@czipperz czipperz closed this Jan 21, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment