Set tokens on AST node in `collect_tokens` #80993

Aaron1011 · 2021-01-13T21:33:41Z

A new HasTokens trait is introduced, which is used to move logic from
the callers of collect_tokens into the body of collect_tokens.

In addition to reducing duplication, this paves the way for PR #80689,
which needs to perform additional logic during token collection.

rust-highfive · 2021-01-13T21:33:45Z

r? @lcnr

(rust-highfive has picked a reviewer for you, use r? to override)

Aaron1011 · 2021-01-13T21:34:01Z

r? @petrochenkov

Aaron1011 · 2021-01-13T21:34:08Z

@bors try @rust-timer queue

rust-timer · 2021-01-13T21:34:09Z

Awaiting bors try build completion.

bors · 2021-01-13T21:34:23Z

⌛ Trying commit 3d2458d6e89f30fe223f6b522e447f23111db04a with merge 735a2bd051c45fc592af35d8b243b1a7f1a97052...

bors · 2021-01-13T22:24:37Z

☀️ Try build successful - checks-actions
Build commit: 735a2bd051c45fc592af35d8b243b1a7f1a97052 (735a2bd051c45fc592af35d8b243b1a7f1a97052)

rust-timer · 2021-01-13T22:24:38Z

Queued 735a2bd051c45fc592af35d8b243b1a7f1a97052 with parent 9bc8b00, future comparison URL.

@rustbot label: +S-waiting-on-perf

compiler/rustc_parse/src/parser/nonterminal.rs

compiler/rustc_parse/src/parser/mod.rs

compiler/rustc_parse/src/parser/item.rs

compiler/rustc_parse/src/parser/expr.rs

compiler/rustc_parse/src/parser/attr.rs

rust-timer · 2021-01-14T01:04:36Z

Finished benchmarking try commit (735a2bd051c45fc592af35d8b243b1a7f1a97052): comparison url.

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. Please note that if the perf results are neutral, you should likely undo the rollup=never given below by specifying rollup- to bors.

Importantly, though, if the results of this run are non-neutral do not roll this PR up -- it will mask other regressions or improvements in the roll up.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf

A new `HasTokens` trait is introduced, which is used to move logic from the callers of `collect_tokens` into the body of `collect_tokens`. In addition to reducing duplication, this paves the way for PR rust-lang#80689, which needs to perform additional logic during token collection.

Aaron1011 · 2021-01-14T03:16:19Z

@petrochenkov I've addressed your comments

petrochenkov · 2021-01-14T07:39:38Z

@Aaron1011
Why did you run perf?
The PR is a pure refactoring and doesn't seem to do any extra work.
That makes it surprising that there's indeed a slight increase in instruction counts. I can't explain it by anything except for different inlining perhaps.

petrochenkov · 2021-01-14T07:40:01Z

Code LGTM.

Aaron1011 · 2021-01-14T15:09:13Z

I ran perf to see if this was responsible for part of the performance impact of #80689 - it looks like it's responsible for a small part.

Aaron1011 · 2021-01-14T15:53:53Z

@petrochenkov: It looks like the regression is only for very short benchmarks (e.g. helloworld) - for larger benchmarks, the impact is less than 0.1%. I think this is ready to merge if the perf impact is acceptable to you

petrochenkov · 2021-01-14T18:00:34Z

@bors r+

bors · 2021-01-14T18:00:35Z

📌 Commit a961e67 has been approved by petrochenkov

bors · 2021-01-14T23:07:39Z

⌛ Testing commit a961e67 with merge d5ce9be8014b33b0078bca9b7c666ce3a874b22c...

rust-log-analyzer · 2021-01-14T23:37:58Z

The job x86_64-gnu-nopt failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- [rustdoc] rustdoc/issue-80893.rs stdout ----

error: rustdoc failed!
status: signal: 13
command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-80893/auxiliary" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc/issue-80893" "/checkout/src/test/rustdoc/issue-80893.rs" "--test" "-Z" "unstable-options" "--test-builder" "true"
------------------------------------------

running 1 test

---
test result: FAILED. 410 passed; 1 failed; 1 ignored; 0 measured; 0 filtered out; finished in 30.04s



command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/compiletest" "--compile-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib" "--run-lib-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib/rustlib/x86_64-unknown-linux-gnu/lib" "--rustc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "--rustdoc-path" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustdoc" "--src-base" "/checkout/src/test/rustdoc" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/rustdoc" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--suite" "rustdoc" "--mode" "rustdoc" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/checkout/obj/build/x86_64-unknown-linux-gnu/llvm/build/bin/FileCheck" "--host-rustcflags" "-Crpath -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python3" "--lldb-python" "/usr/bin/python3" "--gdb" "/usr/bin/gdb" "--llvm-version" "11.0.1-rust-1.51.0-nightly" "--llvm-components" "aarch64 aarch64asmparser aarch64codegen aarch64desc aarch64disassembler aarch64info aarch64utils aggressiveinstcombine all all-targets analysis arm armasmparser armcodegen armdesc armdisassembler arminfo armutils asmparser asmprinter avr avrasmparser avrcodegen avrdesc avrdisassembler avrinfo binaryformat bitreader bitstreamreader bitwriter cfguard codegen core coroutines coverage debuginfocodeview debuginfodwarf debuginfogsym debuginfomsf debuginfopdb demangle dlltooldriver dwarflinker engine executionengine extensions frontendopenmp fuzzmutate globalisel gtest gtest_main hexagon hexagonasmparser hexagoncodegen hexagondesc hexagondisassembler hexagoninfo instcombine instrumentation interpreter ipo irreader jitlink libdriver lineeditor linker lto mc mca mcdisassembler mcjit mcparser mips mipsasmparser mipscodegen mipsdesc mipsdisassembler mipsinfo mirparser msp430 msp430asmparser msp430codegen msp430desc msp430disassembler msp430info native nativecodegen nvptx nvptxcodegen nvptxdesc nvptxinfo objcarcopts object objectyaml option orcerror orcjit passes powerpc powerpcasmparser powerpccodegen powerpcdesc powerpcdisassembler powerpcinfo profiledata remarks riscv riscvasmparser riscvcodegen riscvdesc riscvdisassembler riscvinfo riscvutils runtimedyld scalaropts selectiondag sparc sparcasmparser sparccodegen sparcdesc sparcdisassembler sparcinfo support symbolize systemz systemzasmparser systemzcodegen systemzdesc systemzdisassembler systemzinfo tablegen target testingsupport textapi transformutils vectorize webassembly webassemblyasmparser webassemblycodegen webassemblydesc webassemblydisassembler webassemblyinfo windowsmanifest x86 x86asmparser x86codegen x86desc x86disassembler x86info xray" "--cc" "" "--cxx" "" "--cflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"


failed to run: /checkout/obj/build/bootstrap/debug/bootstrap --stage 2 test
Build completed unsuccessfully in 0:24:54

bors · 2021-01-14T23:40:18Z

💔 Test failed - checks-actions

petrochenkov · 2021-01-15T00:39:45Z

---- [rustdoc] rustdoc/issue-80893.rs stdout ----

error: rustdoc failed!
status: signal: 13

@bors retry

bors · 2021-01-15T05:36:53Z

⌛ Testing commit a961e67 with merge dcf622e...

bors · 2021-01-15T08:32:00Z

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing dcf622e to master...

rust-highfive assigned lcnr Jan 13, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 13, 2021

rust-highfive assigned petrochenkov and unassigned lcnr Jan 13, 2021

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jan 13, 2021

petrochenkov reviewed Jan 13, 2021

View reviewed changes

petrochenkov 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 Jan 13, 2021

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jan 14, 2021

Aaron1011 force-pushed the collect-set-tokens branch from 3d2458d to a961e67 Compare January 14, 2021 03:10

petrochenkov removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 14, 2021

Aaron1011 mentioned this pull request Jan 14, 2021

Refactor token collection to capture trailing token immediately #81017

Merged

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 Jan 14, 2021

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 14, 2021

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 Jan 15, 2021

bors added the merged-by-bors This PR was explicitly merged by bors. label Jan 15, 2021

bors merged commit dcf622e into rust-lang:master Jan 15, 2021

rustbot added this to the 1.51.0 milestone Jan 15, 2021

bors mentioned this pull request Jan 15, 2021

Always collect tokens when we are already inside a collect_tokens call #81029

Closed

Set tokens on AST node in collect_tokens #80993

Set tokens on AST node in collect_tokens #80993

Uh oh!

Conversation

Aaron1011 commented Jan 13, 2021

Uh oh!

rust-highfive commented Jan 13, 2021

Uh oh!

Aaron1011 commented Jan 13, 2021

Uh oh!

Aaron1011 commented Jan 13, 2021

Uh oh!

rust-timer commented Jan 13, 2021

Uh oh!

bors commented Jan 13, 2021

Uh oh!

bors commented Jan 13, 2021

Uh oh!

rust-timer commented Jan 13, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rust-timer commented Jan 14, 2021

Uh oh!

Aaron1011 commented Jan 14, 2021

Uh oh!

petrochenkov commented Jan 14, 2021

Uh oh!

petrochenkov commented Jan 14, 2021

Uh oh!

Aaron1011 commented Jan 14, 2021

Uh oh!

Aaron1011 commented Jan 14, 2021

Uh oh!

petrochenkov commented Jan 14, 2021

Uh oh!

bors commented Jan 14, 2021

Uh oh!

bors commented Jan 14, 2021

Uh oh!

rust-log-analyzer commented Jan 14, 2021

Uh oh!

bors commented Jan 14, 2021

Uh oh!

petrochenkov commented Jan 15, 2021

Uh oh!

bors commented Jan 15, 2021

Uh oh!

bors commented Jan 15, 2021

Uh oh!

Uh oh!

Set tokens on AST node in `collect_tokens` #80993

Set tokens on AST node in `collect_tokens` #80993