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 Iterator::max_size_hint #57360

Open
wants to merge 18 commits into
base: master
from

Conversation

Projects
None yet
6 participants
@clarcharr
Copy link
Contributor

clarcharr commented Jan 5, 2019

This is an alternative to #48544 (since closed) which adds a max_size_hint method to Iterator. It should be merged after #56932, and the actual change is in commit 2134889.

The idea of a "minimum size" is confusing for iterators and iterables, because it's unclear where this should exactly be described. However, the maximum size is unambiguous and will improve the upper bound hints for flatten and flat_map.

I figured that I'd create a PR here so that people can discuss the change, even though it probably won't be added immediately.

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 5, 2019

r? @alexcrichton

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

@clarcharr clarcharr force-pushed the clarcharr:max_size_hint branch from c8dc788 to c0d0a75 Jan 5, 2019

@rust-highfive

This comment was marked as outdated.

Copy link
Collaborator

rust-highfive commented Jan 5, 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:202bf90c:start=1546730289006703010,finish=1546730291405854971,duration=2399151961
$ 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:54] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:54] tidy error: /checkout/src/libcore/iter/adapters/flatten.rs:231: line longer than 100 chars
[00:03:54] tidy error: /checkout/src/libcore/iter/adapters/flatten.rs:234: line longer than 100 chars
40496 ./src/llvm-emscripten/lib/Target
37784 ./src/tools/lldb/www
36020 ./src/tools/clang/lib
34948 ./src/llvm/test/tools

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)

@Centril

This comment has been minimized.

Copy link
Contributor

Centril commented Jan 6, 2019

How does this square with boat's comment about object safety?
(i.e. will you be able to make Travis go green?)

@scottmcm

This comment has been minimized.

Copy link
Member

scottmcm commented Jan 9, 2019

I'm pretty sure the upper bounds are (sadly) completely unused: https://internals.rust-lang.org/t/is-size-hint-1-ever-used/8187?u=scottmcm

So I'm not sure how much good this will do in practice, much as I like it in theory.

@clarcharr clarcharr force-pushed the clarcharr:max_size_hint branch 2 times, most recently from a4d8e11 to 8132f5a Jan 9, 2019

@clarcharr clarcharr changed the title Add Iterator::MAX_SIZE_HINT Add Iterator::max_size_hint Jan 9, 2019

@clarcharr

This comment has been minimized.

Copy link
Contributor

clarcharr commented Jan 9, 2019

How does this square with boat's comment about object safety?
(i.e. will you be able to make Travis go green?)

You're right-- I've changed from a constant to a method instead. This limits its use in constants until some version of const Trait comes around, but it should do fine for all uses in size_hint.

@clarcharr clarcharr force-pushed the clarcharr:max_size_hint branch from 8132f5a to 3d1194c Jan 9, 2019

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 9, 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:09b3d33c:start=1547001510636797919,finish=1547001580405810453,duration=69769012534
$ 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:25]    Compiling cc v1.0.25
[00:03:25]    Compiling libc v0.2.46
[00:03:25]    Compiling core v0.0.0 (/checkout/src/libcore)
[00:03:25]    Compiling unwind v0.0.0 (/checkout/src/libunwind)
[00:03:25] error: expected one of `,` or `}`, found `.`
[00:03:25]    --> src/libcore/iter/adapters/flatten.rs:236:18
[00:03:25]     |
[00:03:25] 236 |                 f.checked_add(b)?.checked_add(m.checked_mul(max)?)?
[00:03:25]     |                  ^ expected one of `,` or `}` here
[00:03:26]    Compiling build_helper v0.1.0 (/checkout/src/build_helper)
[00:03:31] error[E0422]: cannot find struct, variant or union type `catch` in this scope
[00:03:31]    --> src/libcore/iter/adapters/flatten.rs:235:55
[00:03:31]     |
[00:03:31]     |
[00:03:31] 235 |             (Some(f), Some(b), Some(m), Some(max)) => catch {
[00:03:31] 
[00:03:31]    Compiling compiler_builtins v0.1.4
[00:03:31]    Compiling cmake v0.1.33
[00:03:31]    Compiling backtrace-sys v0.1.27
---
[00:03:36]    Compiling rustc_asan v0.0.0 (/checkout/src/librustc_asan)
[00:03:41] error[E0308]: mismatched types
[00:03:41]     --> src/libcore/option.rs:1178:43
[00:03:41]      |
[00:03:41] 1178 |     fn max_size_hint() -> Option<usize> { 1 }
[00:03:41]      |                           -------------   ^ expected enum `option::Option`, found integral variable
[00:03:41]      |                           |
[00:03:41]      |                           expected `option::Option<usize>` because of return type
[00:03:41]      |
[00:03:41]      = note: expected type `option::Option<usize>`
[00:03:41] 
[00:03:41] error[E0308]: mismatched types
[00:03:41]     --> src/libcore/option.rs:1226:43
[00:03:41]      |
[00:03:41]      |
[00:03:41] 1226 |     fn max_size_hint() -> Option<usize> { 1 }
[00:03:41]      |                           -------------   ^ expected enum `option::Option`, found integral variable
[00:03:41]      |                           |
[00:03:41]      |                           expected `option::Option<usize>` because of return type
[00:03:41]      |
[00:03:41]      = note: expected type `option::Option<usize>`
[00:03:41] 
[00:03:41] error[E0308]: mismatched types
[00:03:41]     --> src/libcore/option.rs:1265:43
[00:03:41]      |
[00:03:41]      |
[00:03:41] 1265 |     fn max_size_hint() -> Option<usize> { 1 }
[00:03:41]      |                           -------------   ^ expected enum `option::Option`, found integral variable
[00:03:41]      |                           |
[00:03:41]      |                           expected `option::Option<usize>` because of return type
[00:03:41]      |
[00:03:41]      = note: expected type `option::Option<usize>`
[00:03:41] 
[00:03:42] error[E0308]: mismatched types
[00:03:42]     --> src/libcore/result.rs:1083:43
[00:03:42]      |
[00:03:42]      |
[00:03:42] 1083 |     fn max_size_hint() -> Option<usize> { 1 }
[00:03:42]      |                           -------------   ^ expected enum `option::Option`, found integral variable
[00:03:42]      |                           |
[00:03:42]      |                           expected `option::Option<usize>` because of return type
[00:03:42]      |
[00:03:42]      = note: expected type `option::Option<usize>`
[00:03:42] 
[00:03:42] error[E0308]: mismatched types
[00:03:42]     --> src/libcore/result.rs:1130:43
[00:03:42]      |
[00:03:42]      |
[00:03:42] 1130 |     fn max_size_hint() -> Option<usize> { 1 }
[00:03:42]      |                           -------------   ^ expected enum `option::Option`, found integral variable
[00:03:42]      |                           |
[00:03:42]      |                           expected `option::Option<usize>` because of return type
[00:03:42]      |
[00:03:42]      = note: expected type `option::Option<usize>`
[00:03:42] 
[00:03:42] error[E0308]: mismatched types
[00:03:42]     --> src/libcore/result.rs:1175:43
[00:03:42]      |
[00:03:42]      |
[00:03:42] 1175 |     fn max_size_hint() -> Option<usize> { 1 }
[00:03:42]      |                           -------------   ^ expected enum `option::Option`, found integral variable
[00:03:42]      |                           |
[00:03:42]      |                           expected `option::Option<usize>` because of return type
[00:03:42]      |
[00:03:42]      = note: expected type `option::Option<usize>`
[00:03:42] 
[00:03:44] error: aborting due to 8 previous errors
[00:03:44] 
[00:03:44] Some errors occurred: E0308, E0422.
---
[00:03:44] 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:44] expected success, got: exit code: 101
[00:03:44] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:03:44] Build completed unsuccessfully in 0:00:21
[00:03:44] make: *** [all] Error 1
[00:03:44] Makefile:18: recipe for target 'all' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:07ab2fea
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Wed Jan  9 02:43:34 UTC 2019
---
travis_time:end:1a5b739e:start=1547001815085521184,finish=1547001815090210210,duration=4689026
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:11504506
$ 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:0180b712
travis_time:start:0180b712
$ 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:2a765984
$ 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)

@clarcharr clarcharr force-pushed the clarcharr:max_size_hint branch 5 times, most recently from 43ea988 to bc01591 Jan 10, 2019

@bors

This comment has been minimized.

Copy link
Contributor

bors commented Jan 13, 2019

☔️ The latest upstream changes (presumably #57577) made this pull request unmergeable. Please resolve the merge conflicts.

@clarcharr clarcharr force-pushed the clarcharr:max_size_hint branch from bc01591 to eb18ed3 Jan 15, 2019

@clarcharr

This comment has been minimized.

Copy link
Contributor

clarcharr commented Jan 15, 2019

Rebased. Is there something wrong with the current implementation of try right now? I thought that the issues wrt macro resolution were solved, but apparently not…

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 15, 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:08356510:start=1547589672919180181,finish=1547589675227109024,duration=2307928843
$ 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:04:18]    Compiling cc v1.0.28
[00:04:18]    Compiling libc v0.2.46
[00:04:18]    Compiling core v0.0.0 (/checkout/src/libcore)
[00:04:18]    Compiling build_helper v0.1.0 (/checkout/src/build_helper)
[00:04:18] error: expected one of `,` or `}`, found `.`
[00:04:18]    --> src/libcore/iter/adapters/flatten.rs:236:18
[00:04:18]     |
[00:04:18] 236 |                 f.checked_add(b)?.checked_add(m.checked_mul(max)?)?
[00:04:18]     |                  ^ expected one of `,` or `}` here
[00:04:19]    Compiling unwind v0.0.0 (/checkout/src/libunwind)
[00:04:23] error[E0574]: expected struct, variant or union type, found macro `try`
[00:04:23]    --> src/libcore/iter/adapters/flatten.rs:235:55
[00:04:23]     |
[00:04:23]     |
[00:04:23] 235 |             (Some(f), Some(b), Some(m), Some(max)) => try {
[00:04:23]     |                                                       ^^^ did you mean `try!(...)`?
[00:04:24]    Compiling compiler_builtins v0.1.4
[00:04:24]    Compiling cmake v0.1.33
[00:04:24]    Compiling backtrace-sys v0.1.27
[00:04:26]    Compiling std v0.0.0 (/checkout/src/libstd)
---
[00:04:37] 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:04:37] expected success, got: exit code: 101
[00:04:37] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:04:37] Build completed unsuccessfully in 0:00:20
[00:04:37] make: *** [all] Error 1
[00:04:37] Makefile:18: recipe for target 'all' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:301c870a
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Tue Jan 15 22:06:03 UTC 2019

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)

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