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

Rename InternedString to LocalInternedString and introduce a new thread-safe InternedString #49894

Merged
merged 1 commit into from
Apr 27, 2018

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Apr 12, 2018

This is an allocation-free alternative to #46972.

cc @rust-lang/compiler

r? @michaelwoerister

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Apr 12, 2018
@rust-highfive
Copy link
Collaborator

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:00:46] configure: rust.quiet-tests     := True
---
[00:05:53] tidy error: /checkout/src/libsyntax_pos/symbol.rs:399: line longer than 100 chars
[00:05:54] some tidy checks failed
[00:05:54]
[00:05:54]
[00:05:54] 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:05:54] expected success, got: exit code: 1
[00:05:54]
[00:05:54]
[00:05:54] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:05:54] Build completed unsuccessfully in 0:02:30
[00:05:54] Makefile:79: recipe for target 'tidy' failed
[00:05:54] make: *** [tidy] Error 1

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

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:00:46] configure: rust.quiet-tests     := True
---
[00:06:14] tidy error: /checkout/src/libsyntax_pos/symbol.rs:399: line longer than 100 chars
[00:06:15] some tidy checks failed
[00:06:15]
[00:06:15]
[00:06:15] 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:06:15] expected success, got: exit code: 1
[00:06:15]
[00:06:15]
[00:06:15] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:06:15] Build completed unsuccessfully in 0:02:36
[00:06:15] Makefile:79: recipe for target 'tidy' failed
[00:06:15] make: *** [tidy] Error 1
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:05f4146a:start=1523499577381273781,finish=1523499577389599607,duration=8325826
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:09b82c50
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:09b82c50:start=1523499577397309795,finish=1523499577406013046,duration=8703251
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:3324f878
$ dmesg | grep -i kill
[   11.571928] init: failsafe main process (1094) killed by TERM signal

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

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.
Resolving deltas: 100% (614309/614309), completed with 4928 local objects.
---
[00:00:56] configure: rust.quiet-tests     := True
---
[00:33:14] 1362 |         let path = external_path(cx, &cx.tcx.item_name(did),
[00:33:14]      |                                      ^^^^^^^^^^^^^^^^^^^^^^ expected str, found struct `syntax::symbol::InternedString`
---
[00:33:14] 1469 |         let path = external_path(cx, &cx.tcx.item_name(trait_ref.def_id),
[00:33:14]      |                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected str, found struct `syntax::symbol::InternedString`
---
[00:33:16] 2796 |                 let path = external_path(cx, &cx.tcx.item_name(did),
[00:33:16]      |                                              ^^^^^^^^^^^^^^^^^^^^^^ expected str, found struct `syntax::symbol::InternedString`
---
[00:33:16] 2807 |                 let path = external_path(cx, &cx.tcx.item_name(did),
[00:33:16]      |                                              ^^^^^^^^^^^^^^^^^^^^^^ expected str, found struct `syntax::symbol::InternedString`
---
[00:33:16] 2825 |                         let path = external_path(cx, &cx.tcx.item_name(did),
[00:33:16]      |                                                      ^^^^^^^^^^^^^^^^^^^^^^ expected str, found struct `syntax::symbol::InternedString`
---
[00:33:17] 2848 |                     let path = external_path(cx, &cx.tcx.item_name(did), Some(did),
[00:33:17]      |                                                  ^^^^^^^^^^^^^^^^^^^^^^ expected str, found struct `syntax::symbol::InternedString`
---
[00:33:19] 264 |                         hir::PathSegment::from_name(Symbol::intern(&param.name))
[00:33:19]     |                                                                    ^^^^^^^^^^^ expected str, found struct `syntax::symbol::InternedString`
---
[00:33:25] Makefile:28: recipe for target 'all' failed
[00:33:25] make: *** [all] Error 1
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:008b7d54:start=1523530164931732610,finish=1523530164944951894,duration=13219284
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:1122f171
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:1122f171:start=1523530164956619488,finish=1523530164969050438,duration=12430950
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:083dd5fa
$ dmesg | grep -i kill
[   10.852881] init: failsafe main process (1092) killed by TERM signal

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)

@Zoxc Zoxc force-pushed the sync-internedstring branch 2 times, most recently from 78792f6 to 1154d18 Compare April 12, 2018 12:28
@petrochenkov
Copy link
Contributor

Why this distinction is necessary? When one should use InternedString and when LocalInternedString? I can't infer it from this patch (I haven't followed other parallelization PRs though).

I planned to get rid of InternedString completely in favor of Symbol after #49300 is fixed (and StableInternedString -> StableSymbol for string-y Ord and stuff).

@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 12, 2018

Why this distinction is necessary?

LocalInternedString has a Deref<Target=str> impl which makes it more expressive to use, but also makes it not thread-safe (it doesn't implement Send). InternedString has no Deref<Target=str> impl and implements Send and Sync which makes it suitable to be shared between multiple threads.

When one should use InternedString and when LocalInternedString?

Pick InternedString if you are sharing or sending things between threads (the compiler will inform you). Otherwise you can pick either one.

I planned to get rid of InternedString completely in favor of Symbol

You can still get rid of the introduced InternedString in favor of Symbol. It still makes sense to keep around LocalInternedString because of its more expressive API, but it should probably be renamed to LocalSymbol at that point.

@petrochenkov
Copy link
Contributor

I've just noticed that string interner is no longer thread local and every access through Symbol's index into interner's vector goes through a lock.
So it looks like LocalInternedString in addition to being a convenience also serves as an optimization.
For convenience alone I'd personally wouldn't add an extra string type, but with locks I guess there's no way to avoid this string type proliferation. Sigh.

@petrochenkov petrochenkov removed their assignment Apr 12, 2018
@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 12, 2018

@petrochenkov We can replace everything with Symbol(&'tcx str) once querification reaches libsyntax =P

@petrochenkov
Copy link
Contributor

petrochenkov commented Apr 12, 2018

interner is no longer thread local and every access through Symbol's index into interner's vector goes through a lock

Actually, now I understand even less. If the interner is global under a lock, then string contents live effectively forever once interned and can be accessed from all threads, then why Deref to str is incompatible with Sync/Send. Please, ELI5 how this works.

@Mark-Simulacrum
Copy link
Member

I'm also somewhat confused in general about the design of Interner; it feels like we could relatively easily get away with something like what is implemented in this repo -- the API isn't one-to-one compatible, but the potential API should be effectively the same as today's Symbol/Interner API, if not easier to use. The approach is one where we basically just leak the data (which must be true for today's implementation of Interner if it's safe, since we have safe, public as_str on symbol that returns &'static str.

The negative there is that the size of Symbol increases from 4 bytes to 16 bytes as it goes from u32 to &'static str. I don't know what the size and performance impact of that would be.


This might be best to discuss elsewhere, but it's also confusing to me why we make InternedString not implement Send when it does implement Sync. Since it's a Copy type that seems to imply that not implementing Send means nothing -- I can share a reference between two threads and then copy it in both threads. (Maybe this means a lint on this situation would be good?)

@bors
Copy link
Contributor

bors commented Apr 13, 2018

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

@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 13, 2018

The approach is one where we basically just leak the data (which must be true for today's implementation of Interner if it's safe, since we have safe, public as_str on symbol that returns &'static str.

We do not want to leak strings. The idea of LocalInternedString is that it contains a &'thread str where thread is the lifetime of the current thread. So in order to ensure LocalInternedString is sound we just need to have the interner (which contains the str) outlive threads we create LocalInternedStrings on.

This might be best to discuss elsewhere, but it's also confusing to me why we make InternedString not implement Send when it does implement Sync. Since it's a Copy type that seems to imply that not implementing Send means nothing -- I can share a reference between two threads and then copy it in both threads. (Maybe this means a lint on this situation would be good?)

LocalInternedString can't implement both Sync and Copy, so I added a commit to remove the Sync impl.

@Mark-Simulacrum
Copy link
Member

We do not want to leak strings.

Yet isn't that basically a requirement for today's implementation?

"This accesses the thread-local interner in order to get to the string." is the comment on InternedString which can safely be retrieved from LocalInternedString. I also see "This is safe because the interner never frees strings" in the code; that also implies that we're effectively leaking strings today while still paying the overhead for keeping InternedString not just pointing to the string data, and instead having to go through Symbol to get to the data. Am I missing something? What is the difference between LocalInternedString and InternedString?

It seems to me that long-term Symbol should become &'static str (perhaps wrapped in a newtype) and that should be feasible and is almost what we have today anyway.

@Zoxc Zoxc force-pushed the sync-internedstring branch 2 times, most recently from e39e15a to 63e5b05 Compare April 15, 2018 15:05
@rust-highfive
Copy link
Collaborator

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:00:47] configure: rust.quiet-tests     := True
---
serialize-c04ded78717d5d67.rlib --extern syntax_pos=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax_pos-9f3518d56a01456f.so --extern fmt_macros=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libfmt_macros-3706e912fdb98df1.so --extern bitflags=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libbitflags-9866e194db82a141.rlib --extern graphviz=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libgraphviz-0bde40de32995f14.so --extern flate2=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libflate2-3c8223b0152f22a5.rlib -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/backtrace-sys-8b35e3c2ea935fab/out/.libs -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/miniz-sys-63734d0048644b22/out` (exit code: 101)
[00:07:49] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:07:49] expected success, got: exit code: 101
[00:07:49] thread 'main' panicked at 'cargo must succeed', bootstrap/compile.rs:1085:9
[00:07:49] note: Run with `RUST_BACKTRACE=1` for a backtrace.
[00:07:49] travis_fold:end:stage0-rustc
[00:07:49] travis_time:end:stage0-rustc:start=1523804678821502182,finish=1523804855296100416,duration=176474598234
[00:07:49] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:07:49] Build completed unsuccessfully in 0:03:08
[00:07:49] Makefile:28: recipe for target 'all' failed
[00:07:49] make: *** [all] Error 1
---
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:151da37b:start=1523804855738312853,finish=1523804855744662139,duration=6349286
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:173af718
$ dmesg | grep -i kill
[   10.535064] init: failsafe main process (1094) killed by TERM signal

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)

@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 15, 2018

Yet isn't that basically a requirement for today's implementation?

No. The requirement is that the interner outlives any threads using LocalInternedString. Since LocalInternedString is not Send nor Sync and the &'static str field is private, there is no way for the string reference to escape the thread using it.

I also see "This is safe because the interner never frees strings" in the code

It never frees strings until it is dropped, then all the strings are freed. I changed this comment to make it clearer.

"This accesses the thread-local interner in order to get to the string." is the comment on InternedString which can safely be retrieved from LocalInternedString

I'm not sure what your point is there, but the comment is incorrect since we the interner is no longer thread-local. I removed that.

@rust-highfive
Copy link
Collaborator

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:00:48] configure: rust.quiet-tests     := True
---
--extern rustc_errors=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_errors-609e2421d03f9c9a.so --extern rustc_apfloat=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_apfloat-1299638d641ea770.rlib --extern rustc_back=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_back-861dba9fe03aa669.so --extern graphviz=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libgraphviz-0bde40de32995f14.so --extern jobserver=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libjobserver-5bcc8c1ccd509892.rlib --extern lazy_static=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/liblazy_static-2803512e03c19cf7.rlib --extern proc_macro=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libproc_macro-6e4119b5ec8457a3.so --extern rustc_const_math=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_const_math-895f7ddc4467bb8d.so --extern tempdir=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libtempdir-65dde0349e75e965.rlib --extern rustc_data_structures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_data_structures-93cb1ddd29ab61a4.so --extern fmt_macros=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/releasestage0-rustc:start=1523805119657710455,finish=1523805319582979368,duration=199925268913
[00:08:16] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap build
[00:08:16] Build completed unsuccessfully in 0:03:33
[00:08:16] Makefile:28: recipe for target 'all' failed
[00:08:16] make: *** [all] Error 1

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)

@Mark-Simulacrum
Copy link
Member

Ah, I think I understand now. I believe the root of my confusion came from the fact that without parallel queries being enabled, neither InternedString nor LocalInternedString are Sync/Send, which makes the API this PR proposes fine.

In order for this to be fine with parallel queries, I believe the Interner must live globally (not per-thread); is that an accurate representation of the code today? Otherwise as far as I can tell a Symbol created on thread A can be transferred to other threads, which would lead to problems.

@Zoxc Zoxc force-pushed the sync-internedstring branch 2 times, most recently from 3c87be0 to 10b1de6 Compare April 15, 2018 17:04
@rust-highfive
Copy link
Collaborator

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.
Receiving objects: 100% (102/102), 11.39 KiB | 11.39 MiB/s, done.
---
Resolving deltas: 100% (97/97), completed with 89 local objects.
---
[00:00:45] configure: rust.quiet-tests     := True
---
[00:13:25] 115 |                     ty::BrNamed(id, name)))
[00:13:25]     |                                     ^^^^ expected struct `syntax::symbol::InternedString`, found struct `syntax::symbol::LocalInternedString`
---
[00:13:25] 127 |                     name,
[00:13:25]     |                     ^^^^ expected struct `syntax::symbol::InternedString`, found struct `syntax::symbol::LocalInternedString`
---
[00:13:25] 135 |                     bound_region: ty::BrNamed(id, name)
[00:13:25]     |                                                   ^^^^ expected struct `syntax::symbol::InternedString`, found struct `syntax::symbol::LocalInternedString`
---
[00:13:31] 889 |             name: l.lifetime.name.name().as_str(),
[00:13:31]     |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `syntax::symbol::InternedString`, found struct `syntax::symbol::LocalInternedString`
---
[00:13:31] 1430 |             name: param.lifetime.name.name().as_str(),
[00:13:31]      |                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected struct `syntax::symbol::InternedString`, found struct `syntax::symbol::LocalInternedString`
---
[00:13:31]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name rustc_typeck librustc_typeck/lib.rs --color always --error-format json --crate-type dylib --emit=dep-info,link -C prefer-dynamic -C opt-level=2 -C metadata=d7a9eeb5d1acb48b -C extra-filename=-d7a9eeb5d1acb48b --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/release/deps --extern fmt_macros=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libfmt_macros-3706e912fdb98df1.so --extern rustc_data_structures=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_data_structures-93cb1ddd29ab61a4.so --extern rustc_const_math=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_const_math-895f7ddc4467bb8d.so --extern arena=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libarena-f2778bd6cd1c5bdd.so --extern rustc_platform_intrinsics=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_platform_intrinsics-6f259a2a9f59267f.so --extern rustc=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc-23f5dcecdda8feb4.so --extern rustc_errors=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/librustc_errors-609e2421d03f9c9a.so --extern log=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/liblog-3e35efb9e5bc7a9a.rlib --extern syntax=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax-d3b6fcf798f7d22a.so --extern syntax_pos=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/deps/libsyntax_pos-9f3518d56a01456f.so -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/backtrace-sys-8b35e3c2ea935fab/out/.libs -L native=/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-rustc/x86_64-unknown-linux-gnu/release/build/miniz-sys-63734d0048644b22/out` (exit code: 101)
---
121760 ./obj/build/bootstrap/debug/incremental/bootstrap-351vorei3hhuv/s-f059e4u5y0-1ocolbt-3ssff2uugm5rt
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:1322ddb0:start=1523812471589806466,finish=1523812471597800232,duration=7993766
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:1346d13a
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name '

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)

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Apr 26, 2018
@bors
Copy link
Contributor

bors commented Apr 26, 2018

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

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Apr 26, 2018
@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 27, 2018

@bors r=michaelwoerister

@bors
Copy link
Contributor

bors commented Apr 27, 2018

📌 Commit ce387b8 has been approved by michaelwoerister

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 27, 2018
@Zoxc
Copy link
Contributor Author

Zoxc commented Apr 27, 2018

@bors r=michaelwoerister

@bors
Copy link
Contributor

bors commented Apr 27, 2018

📌 Commit 4d52751 has been approved by michaelwoerister

kennytm added a commit to kennytm/rust that referenced this pull request Apr 27, 2018
…woerister

Rename InternedString to LocalInternedString and introduce a new thread-safe InternedString

This is an allocation-free alternative to rust-lang#46972.
bors added a commit that referenced this pull request Apr 27, 2018
Rollup of 7 pull requests

Successful merges:

 - #49707 (Add "the Rustc book")
 - #50222 (Bump bootstrap compiler to 2018-04-24)
 - #50227 (Fix ICE with erroneous `impl Trait` in a trait impl)
 - #50229 (Add setting to go to item if there is only one result)
 - #50231 (Add more doc aliases)
 - #50246 (Make dump_{alloc,allocs,local}() no-ops when tracing is disabled.)
 - #49894 (Rename InternedString to LocalInternedString and introduce a new thread-safe InternedString)

Failed merges:
@bors bors merged commit 4d52751 into rust-lang:master Apr 27, 2018
/// Represents a string stored in the string interner
#[derive(Clone, Copy, Eq)]
pub struct InternedString {
symbol: Symbol,
Copy link
Member

Choose a reason for hiding this comment

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

I saw this PR too late, but wish this was named SymbolContents instead of InternedString, since AFAICT all it does is it changes the behavior of comparison / hashing / etc. to act on the string value inside.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd expect Symbol to contain SymbolContents and not the other way around =P

Copy link
Member

Choose a reason for hiding this comment

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

SymbolAsStr? I mean, I want the type to evoke "this is a Symbol made to look like the string contents thereof".

@Zoxc Zoxc deleted the sync-internedstring branch April 27, 2018 12:20
phansch added a commit to phansch/rust-clippy that referenced this pull request Apr 28, 2018
barzamin pushed a commit to rustodon/maud that referenced this pull request Apr 28, 2018
barzamin added a commit to rustodon/maud that referenced this pull request Apr 28, 2018
lambda-fairy pushed a commit to lambda-fairy/maud that referenced this pull request May 2, 2018
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.

7 participants