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

WIP: Allocator- and fallibility-polymorphic collections #52420

Closed
wants to merge 8 commits into from

Conversation

Ericson2314
Copy link
Contributor

@Ericson2314 Ericson2314 commented Jul 16, 2018

This picks up where #50882 will leave off, adding allocator parameters to the other collections. It also adds and associated error type for allocation, so as to enable fallibility-polymorphic code and support the few impls and other functions which require "infallible" allocation.

When done, should convert all the collections for #42774


Unfortunately, as documented in #52396, I had to make an AllocHelper trait in order to get my default impl to work.

Also, OK(_): Result<T, !> is no longer deemed and infallible pattern so my let Ok(..) = ..; doesn't work. I can switch it to something else, but I'd like to make sure that's intentional first. edit fixed with exhaustive_patterns.

@rust-highfive
Copy link
Collaborator

r? @withoutboats

(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 Jul 16, 2018
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.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.
[00:03:52]    Compiling tidy v0.1.0 (file:///checkout/src/tools/tidy)
[00:04:00] some tidy checks failed
[00:04:00] 
[00:04:00] 
[00:04:00] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:00] 
[00:04:00] 
[00:04:00] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:00] Build completed unsuccessfully in 0:00:49
[00:04:00] Build completed unsuccessfully in 0:00:49
[00:04:00] Makefile:79: recipe for target 'tidy' failed
[00:04:00] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:09a4a148
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.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.

[00:03:54] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:54] tidy error: /checkout/src/liballoc/string.rs:994: line longer than 100 chars
[00:03:54] tidy error: /checkout/src/liballoc/collections/linked_list.rs:1320: line longer than 100 chars
[00:03:54] tidy error: /checkout/src/liballoc/collections/vec_deque.rs:606: line longer than 100 chars
[00:03:54] tidy error: /checkout/src/liballoc/vec.rs:566: line longer than 100 chars
[00:03:55] some tidy checks failed
[00:03:55] 
[00:03:55] 
[00:03:55] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:55] 
[00:03:55] 
[00:03:55] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:55] Build completed unsuccessfully in 0:00:48
[00:03:55] Build completed unsuccessfully in 0:00:48
[00:03:55] Makefile:79: recipe for target 'tidy' failed
[00:03:55] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
 ./obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/lib
177640 ./obj/build/bootstrap/debug/deps
177640 ./obj/build/bootstrap/debug/deps
170388 ./obj/build/cache
170384 ./obj/build/cache/2018-07-13
158296 ./.git/modules
158292 ./.git/modules/src
149120 ./src/llvm-emscripten/test
145036 ./obj/build/bootstrap/debug/incremental
130516 ./obj/build/bootstrap/debug/incremental/bootstrap-3kaq1kqcanyi4
130512 ./obj/build/bootstrap/debug/incremental/bootstrap-3kaq1kqcanyi4/s-f2yn7t2c53-1dpl6b5-ee4a52izcif7
97528 ./obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/codegen-backends
76824 ./.git/modules/src/tools
71508 ./src/llvm/lib
65420 ./src/llvm-emscripten/test/CodeGen
---
32212 ./src/libcompiler_builtins/compiler-rt/test
31048 ./.git/modules/src/tools/lld
31024 ./src/llvm/test/tools
30672 ./.git/modules/src/tools/lld/objects
30664 ./.git/modules/src/t9820 ./src/llvm/test/CodeGen/AMDGPU
9176 ./obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps
8992 ./src/doc/book
8904 ./src/llvm/lib/CodeGen
8572 ./.git/modules/src/tools/rustfmt
---
travis_time:end:076ea8a5:start=1531756599015138620,finish=1531756599022816824,duration=7678204
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:1b87ee8c
$ 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 -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:07ce538e
travis_time:start:07ce538e
$ 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:01e2c38b
$ 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)


/// The type of any errors thrown by the allocator, customarily
/// either `AllocErr`, for when error recovery is allowed, or `!`
/// to signify that all errors will result in .
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The last word of this sentence is missing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks :)

@aturon aturon added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 17, 2018
glandium and others added 8 commits July 19, 2018 19:38
This turns `Box<T>` into `Box<T, A: Alloc = Global>`. This is a
minimalist change to achieve this, not touching anything that could have
backwards incompatible consequences like requiring type annotations in
places where they currently aren't required,
per rust-lang#50822 (comment)
We will use this for fallibility polymorphism
See its documentation for why its useful
@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-5.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.

[00:03:39] travis_fold:start:tidy
travis_time:start:tidy
tidy check
[00:03:39] tidy error: /checkout/src/liballoc/string.rs:994: line longer than 100 chars
[00:03:39] tidy error: /checkout/src/liballoc/collections/linked_list.rs:1320: line longer than 100 chars
[00:03:39] tidy error: /checkout/src/liballoc/collections/vec_deque.rs:606: line longer than 100 chars
[00:03:39] tidy error: /checkout/src/liballoc/vec.rs:566: line longer than 100 chars
[00:03:41] some tidy checks failed
[00:03:41] 
[00:03:41] 
[00:03:41] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:03:41] 
[00:03:41] 
[00:03:41] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:03:41] Build completed unsuccessfully in 0:00:45
[00:03:41] Build completed unsuccessfully in 0:00:45
[00:03:41] Makefile:79: recipe for target 'tidy' failed
[00:03:41] make: *** [tidy] Error 1

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:00dfee99
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
---
travis_time:end:005a7d82:start=1532011648424021661,finish=1532011648430689459,duration=6667798
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:26d0c12a
$ 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 -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:0b05c20b
travis_time:start:0b05c20b
$ 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:08d4dcf9
$ 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)

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

r? @alexcrichton

Thanks for the PR @Ericson2314! I think at this time though the libs team doesn't have the bandwidth to review and land this before the 2018 edition. This is a very large step for us to take which we want to be sure to take carefully. Would it be possible to postpone this until after the 2018 edition has released?

@Ericson2314
Copy link
Contributor Author

I suppose that's fine. When is that likely to be?

I'd just like the fact that there's work done towards this to affect decisions like Alloc stabilization. (I don't think I actually need to break any APIs, but let this show that many long-standing issues with alloc are in fact resolvable, so who knows what else a deep dive would turn up.)

@Ericson2314
Copy link
Contributor Author

If at least #50882 can not be blocked, that would help me keep this up-to-date in the meantime.

@alexcrichton
Copy link
Member

#50882 AFAIK is largely technically blocked on the regression in error messages, but it's basically in the same boat as this PR in that it requires some discussion and design which the libs team doesn't have the bandwidth for right now, but after the edition 2018 release we can revisit.

@glandium
Copy link
Contributor

#50882 is not blocked on regression in error messages anymore. It's blocked on one test failing in what looks like a race condition, and that only happens when building with LLVM 5.

@alexcrichton
Copy link
Member

@glandium er sorry but that PR is also large enough and contentious enough that we're not going to be able to land it before the edition, I'll comment over there though.

@SimonSapin
Copy link
Contributor

Making AllocErr generic a type parameter / associated type seems like a significant enough design change in public APIs to merit being discussed outside of a large PR.

@Ericson2314
Copy link
Contributor Author

Sure I do not expect this one to be merged quickly, only to provide a concrete example of what I am talking about.

@XAMPPRocky
Copy link
Member

Triage: Based on the comments above I'm closing this PR and marking it S-blocked-closed on the landing of the 2018 edition. Feel free to reopen once the 2018 edition lands!

@XAMPPRocky XAMPPRocky closed this Aug 10, 2018
@XAMPPRocky XAMPPRocky added S-blocked-closed and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 10, 2018
@tesuji
Copy link
Contributor

tesuji commented Feb 13, 2019

Now the 2018 edition is landed. The PR could be reopened.

@Ericson2314
Copy link
Contributor Author

@lzutao thanks for you interest, but this depends on #50882, which is still blocked on a bug in an older LLVM that we still try to support. Please go read that thread for details.

@jyn514 jyn514 added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-blocked-closed labels Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. 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.