`TokenStream` improvements#56737

nnethercote · 2018-12-12T04:48:34Z

Some TokenStream improvements: shrinking TokenStream and some other types, and some other code clean-ups.

rust-highfive · 2018-12-12T04:48:37Z

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

nnethercote · 2018-12-12T04:49:44Z

The first commit just replicates #56699, which is a prerequisite. Once that lands I will rebase to remove that commit from this PR.

rust-highfive · 2018-12-12T06:18:21Z

The job mingw-check 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:0499a949:start=1544590199028490396,finish=1544590201310673674,duration=2282183278
$ 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=mingw-check
---
[00:03:58]     Checking rustc_data_structures v0.0.0 (/checkout/src/librustc_data_structures)
[00:04:00]     Checking arena v0.0.0 (/checkout/src/libarena)
[00:04:00]     Checking syntax_pos v0.0.0 (/checkout/src/libsyntax_pos)
[00:04:01]     Checking rustc_errors v0.0.0 (/checkout/src/librustc_errors)
[00:04:09] error[E0277]: `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>` cannot be sent between threads safely
[00:04:09]    --> src/libsyntax/ext/tt/macro_rules.rs:396:13
[00:04:09] 396 |             expander,
[00:04:09] 396 |             expander,
[00:04:09]     |             ^^^^^^^^ `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>` cannot be sent between threads safely
[00:04:09]     |
[00:04:09]     = help: within `(parse::token::Nonterminal, parse::token::LazyTokenStream)`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>`
[00:04:09]     = note: required because it appears within the type `std::option::Option<std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>>`
[00:04:09]     = note: required because it appears within the type `tokenstream::ThinTokenStream`
[00:04:09]     = note: required because it appears within the type `tokenstream::TokenTree`
[00:04:09]     = note: required because it appears within the type `parse::token::Nonterminal`
[00:04:09]     = note: required because it appears within the type `(parse::token::Nonterminal, parse::token::LazyTokenStream)`
[00:04:09]     = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<(parse::token::Nonterminal, parse::token::LazyTokenStream)>`
[00:04:09]     = note: required because it appears within the type `parse::token::Token`
[00:04:09]     = note: required because it appears within the type `ext::tt::quoted::TokenTree`
[00:04:09]     = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `alloc::raw_vec::RawVec<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `std::vec::Vec<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `ext::tt::macro_rules::MacroRulesMacroExpander`
[00:04:09]     = note: required for the cast to the object type `dyn ext::base::TTMacroExpander + std::marker::Send + std::marker::Sync`
[00:04:09] 
[00:04:09] error[E0277]: `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>` cannot be shared between threads safely
[00:04:09]    --> src/libsyntax/ext/tt/macro_rules.rs:396:13
[00:04:09] 396 |             expander,
[00:04:09] 396 |             expander,
[00:04:09]     |             ^^^^^^^^ `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>` cannot be shared between threads safely
[00:04:09]     |
[00:04:09]     = help: within `(parse::token::Nonterminal, parse::token::LazyTokenStream)`, the trait `std::marker::Sync` is not implemented for `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>`
[00:04:09]     = note: required because it appears within the type `std::option::Option<std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>>`
[00:04:09]     = note: required because it appears within the type `tokenstream::ThinTokenStream`
[00:04:09]     = note: required because it appears within the type `tokenstream::TokenTree`
[00:04:09]     = note: required because it appears within the type `parse::token::Nonterminal`
[00:04:09]     = note: required because it appears within the type `(parse::token::Nonterminal, parse::token::LazyTokenStream)`
[00:04:09]     = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<(parse::token::Nonterminal, parse::token::LazyTokenStream)>`
[00:04:09]     = note: required because it appears within the type `parse::token::Token`
[00:04:09]     = note: required because it appears within the type `ext::tt::quoted::TokenTree`
[00:04:09]     = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `alloc::raw_vec::RawVec<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `std::vec::Vec<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `ext::tt::macro_rules::MacroRulesMacroExpander`
[00:04:09]     = note: required for the cast to the object type `dyn ext::base::TTMacroExpander + std::marker::Send + std::marker::Sync`
[00:04:09] 
[00:04:09] error[E0277]: `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>` cannot be sent between threads safely
[00:04:09]    --> src/libsyntax/ext/tt/macro_rules.rs:408:13
[00:04:09] 408 |             expander,
[00:04:09] 408 |             expander,
[00:04:09]     |             ^^^^^^^^ `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>` cannot be sent between threads safely
[00:04:09]     |
[00:04:09]     = help: within `(parse::token::Nonterminal, parse::token::LazyTokenStream)`, the trait `std::marker::Send` is not implemented for `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>`
[00:04:09]     = note: required because it appears within the type `std::option::Option<std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>>`
[00:04:09]     = note: required because it appears within the type `tokenstream::ThinTokenStream`
[00:04:09]     = note: required because it appears within the type `tokenstream::TokenTree`
[00:04:09]     = note: required because it appears within the type `parse::token::Nonterminal`
[00:04:09]     = note: required because it appears within the type `(parse::token::Nonterminal, parse::token::LazyTokenStream)`
[00:04:09]     = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<(parse::token::Nonterminal, parse::token::LazyTokenStream)>`
[00:04:09]     = note: required because it appears within the type `parse::token::Token`
[00:04:09]     = note: required because it appears within the type `ext::tt::quoted::TokenTree`
[00:04:09]     = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `alloc::raw_vec::RawVec<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `std::vec::Vec<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `ext::tt::macro_rules::MacroRulesMacroExpander`
[00:04:09]     = note: required for the cast to the object type `dyn ext::base::TTMacroExpander + std::marker::Send + std::marker::Sync`
[00:04:09] 
[00:04:09] error[E0277]: `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>` cannot be shared between threads safely
[00:04:09]    --> src/libsyntax/ext/tt/macro_rules.rs:408:13
[00:04:09] 408 |             expander,
[00:04:09] 408 |             expander,
[00:04:09]     |             ^^^^^^^^ `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>` cannot be shared between threads safely
[00:04:09]     |
[00:04:09]     = help: within `(parse::token::Nonterminal, parse::token::LazyTokenStream)`, the trait `std::marker::Sync` is not implemented for `std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>`
[00:04:09]     = note: required because it appears within the type `std::option::Option<std::rc::Rc<std::vec::Vec<tokenstream::TokenStream>>>`
[00:04:09]     = note: required because it appears within the type `tokenstream::ThinTokenStream`
[00:04:09]     = note: required because it appears within the type `tokenstream::TokenTree`
[00:04:09]     = note: required because it appears within the type `parse::token::Nonterminal`
[00:04:09]     = note: required because it appears within the type `(parse::token::Nonterminal, parse::token::LazyTokenStream)`
[00:04:09]     = note: required because of the requirements on the impl of `std::marker::Send` for `std::sync::Arc<(parse::token::Nonterminal, parse::token::LazyTokenStream)>`
[00:04:09]     = note: required because it appears within the type `parse::token::Token`
[00:04:09]     = note: required because it appears within the type `ext::tt::quoted::TokenTree`
[00:04:09]     = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `alloc::raw_vec::RawVec<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `std::vec::Vec<ext::tt::quoted::TokenTree>`
[00:04:09]     = note: required because it appears within the type `ext::tt::macro_rules::MacroRulesMacroExpander`
[00:04:09]     = note: required for the cast to the object type `dyn ext::base::TTMacroExpander + std::marker::Send + std::marker::Sync`
[00:04:09] error: aborting due to 4 previous errors
[00:04:09] 
[00:04:09] For more information about this error, try `rustc --explain E0277`.
[00:04:09] error: Could not compile `syntax`.
[00:04:09] error: Could not compile `syntax`.
[00:04:09] 
[00:04:09] To learn more, run the command again with --verbose.
[00:04:09] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "check" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--color" "always" "--features" "" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "--message-format" "json"
[00:04:09] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap check
[00:04:09] Build completed unsuccessfully in 0:02:41
travis_time:end:10a468be:start=1544590210572672518,finish=1544590460214439220,duration=249641766702
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 1.
---
travis_time:end:01dcf37c:start=1544590460631927324,finish=1544590460638167393,duration=6240069
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:11e0bde9
$ 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:05d4745c
travis_time:start:05d4745c
$ 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:0544ccda
$ 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)

nnethercote · 2018-12-12T06:44:01Z

Oh, I need to use Lrc instead of Rc.

This shrinks: - ThinTokenStream: 16 to 8 bytes - TokenTree: 32 to 24 bytes - TokenStream: 40 to 32 bytes The only downside is that in a couple of places this requires using `to_vec()` (which allocates) instead of `sub_slice()`. But those places are rarely executed, so it doesn't hurt perf. Overall, this reduces instruction counts on numerous benchmarks by up to 3%.

It's a better choice in a few places.

They're both unused now.

Because the distinction provides little value, and removing it cleans up the code quite a bit.

`TokenStream::new` is a better name for the former, and the latter is now just equivalent to `TokenStream::Stream`.

nnethercote · 2018-12-12T09:38:40Z

I switched to Lrc.

petrochenkov · 2018-12-12T16:52:09Z

@bors r+
@bors delegate+

bors · 2018-12-12T16:52:13Z

✌️ @nnethercote can now approve this pull request

bors · 2018-12-12T16:52:14Z

📌 Commit e80c7dd has been approved by petrochenkov

bors · 2018-12-17T01:48:35Z

⌛ Testing commit e80c7dd with merge c6fb01d...

…henkov `TokenStream` improvements Some `TokenStream` improvements: shrinking `TokenStream` and some other types, and some other code clean-ups.

bors · 2018-12-17T04:18:10Z

☀️ Test successful - status-appveyor, status-travis
Approved by: petrochenkov
Pushing c6fb01d to master...

XXX: SymbolIndex

08857c1

rust-highfive assigned petrochenkov Dec 12, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 12, 2018

nnethercote added 5 commits December 12, 2018 20:36

Use TokenStream::concat more.

3c9aef1

It's a better choice in a few places.

Remove RcVec and RcSlice.

43c6ece

They're both unused now.

Merge TokenStreamKind into TokenStream.

07c12fa

Because the distinction provides little value, and removing it cleans up the code quite a bit.

Rename TokenStream::concat and remove TokenStream::concat_rc_vec.

e80c7dd

`TokenStream::new` is a better name for the former, and the latter is now just equivalent to `TokenStream::Stream`.

nnethercote force-pushed the TokenStream-improvements branch from fa14050 to e80c7dd Compare December 12, 2018 09:38

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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 12, 2018

Centril mentioned this pull request Dec 16, 2018

Rollup of 22 pull requests #56873

Closed

This was referenced Dec 16, 2018

Rework treatment of $crate in procedural macros #56647

Merged

Rollup of 20 pull requests #56875

Merged

bors merged commit e80c7dd into rust-lang:master Dec 17, 2018

nnethercote deleted the TokenStream-improvements branch December 17, 2018 20:53

petrochenkov mentioned this pull request Jan 8, 2019

Make TokenStream less recursive. #57004

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`TokenStream` improvements#56737