[Do not merge] Allow generic parameters to be specified in expressions without `::` #53074

varkor · 2018-08-04T21:55:25Z

Since chained comparison operators are invalid, the :: disambiguation for generic parameters in expressions is no longer syntactically necessary. With the current parser infrastructure this is still a little awkward, requiring backtracking. However, it'd be nice to know just how bad backtracking is, because it might turn out only to be much worse in niche edge cases, in which case the usability benefit could be worthwhile.

This PR is intended solely to test the performance of these parser changes.

@rust-lang/infra: would it be possible to get a perf run for this change?

rust-highfive · 2018-08-04T21:55:35Z

r? @michaelwoerister

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

Mark-Simulacrum · 2018-08-04T21:57:29Z

@bors try

bors · 2018-08-04T21:57:40Z

⌛ Trying commit 23aac5c with merge 832690d...

[Do not merge] Allow generic parameters to be specified in expressions without `::` Since chained comparison operators are invalid, it's plausible that the `::` disambiguation for generic parameters in expressions is no longer necessary. With the current parser infrastructure this is still a little awkward, requiring backtracking. However, it'd be nice to know just how bad backtracking is, because it _might_ turn out only to be much worse in niche edge cases, in which case the usability benefit could be worthwhile. This PR is intended solely to test the performance of these parser changes. @rust-lang/infra: would it be possible to get a perf run for this change?

Mark-Simulacrum · 2018-08-04T21:59:00Z

A perf build should be queueable with @bors-timer build 832690dbad88a28adad03ff465fbdfaea48ba79a.

Mark-Simulacrum · 2018-08-04T22:00:17Z

I feel like this would need an RFC anyway, but good to get performance numbers so at least that's not a concern.

varkor · 2018-08-04T22:19:37Z

@Mark-Simulacrum: yes, if this turns out to look viable, I'd definitely want to put it through the RFC process before merging anything. It's a significant change. I thought it was better to check whether it was practical first.

A perf build should be queueable with @bors-timer build 832690dbad88a28adad03ff465fbdfaea48ba79a.

Thanks!

bors · 2018-08-04T23:45:57Z

☀️ Test successful - status-travis
State: approved= try=True

varkor · 2018-08-04T23:50:21Z

@bors-timer build 832690d

Mark-Simulacrum · 2018-08-04T23:52:11Z

@rust-timer build 832690d

Miswrote the instructions...

rust-timer · 2018-08-04T23:52:12Z

Success: Queued 832690d with parent 94c3c34, comparison URL.

kennytm · 2018-08-05T03:29:48Z

Perf is ready. 1 to 2% regression in half of the tests.

varkor · 2018-08-05T11:06:56Z

This is better than I expected, but still not ideal. I'll try to give some thoughts to how to speed it up.

rust-highfive · 2018-08-05T15:59:43Z

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:40:55] ....................................................................................................
[00:40:59] ....................................................................................................
[00:41:01] ....................................................................................................
[00:41:03] ...........i........................................................................................
[00:41:06] .....F.FFFFF........................................................................................
[00:41:10] ....................................................................................................
[00:41:13] ....................................................................................................
[00:41:16] ....................................................................................................
[00:41:19] ....................................................................................................
[00:41:19] ....................................................................................................
[00:41:22] ....................................................................................................
[00:41:26] .....i..............................................................................................
[00:41:28] ..............i.....................................................................................
[00:41:32] ....................................................................................................
[00:41:35] ....................................................................................................
kout/src/test/ui/issue-20616-2.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issue-20616-2/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issue-20616-2/auxiliary" "-A" "unused"
[00:41:40] ------------------------------------------
[00:41:40] 
[00:41:40] ------------------------------------------
[00:41:40] stderr:
[00:41:40] stderr:
[00:41:40] ------------------------------------------
[00:41:40] {"message":"expected type, found `'static`","code":null,"level":"error","spans":[{"file_name":"/checkout/src/test/ui/issue-20616-2.rs","byte_start":718,"byte_end":725,"line_start":22,"line_end":22,"column_start":23,"column_end":30,"is_primary":true,"text":[{"text":"type Type_2 = Type_1_<'static ()>; //~ error: expected one of `,` or `>`, found `(`","highlight_start":23,"highlight_end":30}],"label":null,"suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":"error: expected type, found `'static`\n  --> /checkout/src/test/ui/issue-20616-2.rs:22:23\n   |\nLL | type Type_2 = Type_1_<'static ()>; //~ error: expected one of `,` or `>`, found `(`\n   |                       ^^^^^^^\n\n"}
[00:41:40] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:41:40] ------------------------------------------
[00:41:40] 
[00:41:40] 
[00:41:40] thst/ui/issue-20616-3/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issue-20616-3/auxiliary" "-A" "unused"
[00:41:40] ------------------------------------------
[00:41:40] 
[00:41:40] ------------------------------------------
[00:41:40] stderr:
[00:41:40] stderr:
[00:41:40] ------------------------------------------
[00:41:40] {"message":"expected one of `>`, lifetime, or type, found `,`","code":null,"level":"error","spans":[{"file_name":"/checkout/src/test/ui/issue-20616-3.rs","byte_start":819,"byte_end":820,"line_start":25,"line_end":25,"column_start":24,"column_end":25,"is_primary":true,"text":[{"text":"type Type_3<T> = Box<T,,>; //~ error: expected one of `>`, identifier, lifetime, or type, found `,`","highlight_start":24,"highlight_end":25}],"label":"expected one of `>`, lifetime, or type here","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":"error: expected one of `>`, lifetime, or type, found `,`\n  --> /checkout/src/test/ui/issue-20616-3.rs:25:24\n   |\nLL | type Type_3<T> = Box<T,,>; //~ error: expected one of `>`, identifier, lifetime, or type, found `,`\n   |                        ^ expected one of `>`, lifetime, or type here\n\n"}
[00:41:40] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:41:40] ------------------------------------------
[00:41:40] 
[00:41:40] thread '[ui] ui/issue-20616-3.rs' pan,`
[00:41:40] thread '[ui] ui/issue-20616-3.rs' pan,`
[00:41:40] + error: expected one of `>`, lifetime, or type, found `,`
[00:41:40] 3    |
[00:41:40] 3    |
[00:41:40] 4 LL | type Type_4<T> = Type_1_<'static,, T>;
[00:41:40] 
[00:41:40] -    |                                  ^ expected one of `>`, identifier, lifetime, or type here
[00:41:40] +    |                                  ^ expected one of `>`, lifetime, or type here
[00:41:40] 7 error: aborting due to previous error
[00:41:40] 8 
[00:41:40] 
[00:41:40] 
[00:41:40] 
[00:41:40] The actual stderr differed from the expected stderr.
[00:41:40] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issue-20616-4/issue-20616-4.stderr
[00:41:40] To update references, rerun the tests and pass the `--bless` flag
[00:41:40] To only update this specific test, also pass `--test-args issue-20616-4.rs`
[00:41:40] error: 1 errors occurred comparing output.
[00:41:40] status: exit code: 1
[00:41:40] status: exit code: 1
[00:41:40] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/issue-20616-4.rs" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issue-20616-4/a" "-Crpath" "-O" "-Zunstable-options" "-Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/issue-20616-4/auxiliary" "-A" "unused"
[00:41:40] ------------------------------------------
[00:41:40] 
[00:41:40] ------------------------------------------
[00:41:40] ------------------------------------------
[00:41:40] s20616-7.rs","byte_start":1143,"byte_end":1144,"line_start":40,"line_end":40,"column_start":22,"column_end":23,"is_primary":true,"text":[{"text":"type Type_7 = Box<(),,>; //~ error: expected one of `>`, identifier, lifetime, or type, found `,`","highlight_start":22,"highlight_end":23}],"label":"expected one of `>`, lifetime, or type here","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[],"rendered":"error: expected one of `>`, lifetime, or type, found `,`\n  --> /checkout/src/test/ui/issue-20616-7.rs:40:22\n   |\nLL | type Type_7 = Box<(),,>; //~ error: expected one of `>`, identifier, lifetime, or type, found `,`\n   |                      ^ expected one of `>`, lifetime, or type here\n\n"}
[00:41:40] {"message":"aborting due to previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to previous error\n\n"}
[00:41:40] ------------------------------------------
[00:41:40] 
[00:41:40] thread '[ui] ui/issue-20616-7.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:3149:9
[00:41:40] 
[00:41:40] 
[00:41:40] ---- [ui] ui/issue-20616-6.rs stdout ----
[00:41:40] diff of stderr:
[00:41:40] 
[00:41:40] - error: expected one of `>`, identifier, lifetime, or type, found `,`
[00:41:40] + error: expected one of `>`, lifetime, or type, found `,`
[00:41:40] 3    |
[00:41:40] 3    |
[00:41:40] 4 LL | type Type_6 = Type_5_<'a,,>;
[00:41:40] 
[00:41:40] -    |                          ^ expected one of `>`, identifier, lifetime, or type here
[00:41:40] +    |                          ^ expected uration=59015694
The command "date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
" exited with 0.
travis_fold:start:after_failure.1
travis_time:start:0c4fa469
---
travis_time:end:2228b880:start=1533484780395236961,finish=1533484780405328730,duration=10091769
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:00942959
$ 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:0ffa5606
$ 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)

varkor · 2018-08-05T18:19:31Z

@bors try

bors · 2018-08-05T18:19:41Z

⌛ Trying commit 2066031 with merge 1d1139d2989104a9fd35a9193a27dfc2632cb981...

bors · 2018-08-05T20:08:35Z

☀️ Test successful - status-travis
State: approved= try=True

varkor · 2018-08-05T20:21:58Z

@rust-timer build 1d1139d2989104a9fd35a9193a27dfc2632cb981

rust-timer · 2018-08-05T20:21:59Z

Success: Queued 1d1139d2989104a9fd35a9193a27dfc2632cb981 with parent 9f9ac89, comparison URL.

varkor · 2018-08-06T00:18:21Z

Right now, this seems implausible without changing the parser significantly. 2% increase in compile times is probably not worth the benefits.

Allow generic parameters to be specified in expressions without ::

23aac5c

rust-highfive assigned michaelwoerister Aug 4, 2018

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

kennytm 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 Aug 5, 2018

Refactoring

ca699c8

Fix failing tests

2066031

varkor closed this Aug 6, 2018

varkor mentioned this pull request Aug 19, 2018

[Do not merge] Allow generic parameters to be specified in expressions without :: #53511

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Do not merge] Allow generic parameters to be specified in expressions without `::` #53074

[Do not merge] Allow generic parameters to be specified in expressions without `::` #53074

varkor commented Aug 4, 2018 •

edited

Loading

rust-highfive commented Aug 4, 2018

Mark-Simulacrum commented Aug 4, 2018

bors commented Aug 4, 2018

Mark-Simulacrum commented Aug 4, 2018

Mark-Simulacrum commented Aug 4, 2018

varkor commented Aug 4, 2018 •

edited

Loading

bors commented Aug 4, 2018

varkor commented Aug 4, 2018

Mark-Simulacrum commented Aug 4, 2018

rust-timer commented Aug 4, 2018

kennytm commented Aug 5, 2018

varkor commented Aug 5, 2018

rust-highfive commented Aug 5, 2018

varkor commented Aug 5, 2018

bors commented Aug 5, 2018

bors commented Aug 5, 2018

varkor commented Aug 5, 2018

rust-timer commented Aug 5, 2018

varkor commented Aug 6, 2018

[Do not merge] Allow generic parameters to be specified in expressions without :: #53074

[Do not merge] Allow generic parameters to be specified in expressions without :: #53074

Conversation

varkor commented Aug 4, 2018 • edited Loading

rust-highfive commented Aug 4, 2018

Mark-Simulacrum commented Aug 4, 2018

bors commented Aug 4, 2018

Mark-Simulacrum commented Aug 4, 2018

Mark-Simulacrum commented Aug 4, 2018

varkor commented Aug 4, 2018 • edited Loading

bors commented Aug 4, 2018

varkor commented Aug 4, 2018

Mark-Simulacrum commented Aug 4, 2018

rust-timer commented Aug 4, 2018

kennytm commented Aug 5, 2018

varkor commented Aug 5, 2018

rust-highfive commented Aug 5, 2018

varkor commented Aug 5, 2018

bors commented Aug 5, 2018

bors commented Aug 5, 2018

varkor commented Aug 5, 2018

rust-timer commented Aug 5, 2018

varkor commented Aug 6, 2018

[Do not merge] Allow generic parameters to be specified in expressions without `::` #53074

[Do not merge] Allow generic parameters to be specified in expressions without `::` #53074

varkor commented Aug 4, 2018 •

edited

Loading

varkor commented Aug 4, 2018 •

edited

Loading