[wip] Rework how type params are parsed #61254

estebank · 2019-05-28T05:53:32Z

Separate lifetimes, type arguments, const params and type bindings
as early as possible in the parser. When encountering bad ordering,
suggest the correct order in a homogeneous way for all possible
incorrect ordering.

rust-highfive · 2019-05-28T05:53:43Z

r? @michaelwoerister

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

estebank · 2019-05-28T05:58:54Z

I need to improve the wording, and it'd be a good idea to keep the separation in the hir as well, but the bulk of work is done.

estebank · 2019-05-28T06:01:11Z

src/test/ui/traits/trait-object-vs-lifetime.stderr

   |
 LL |     let _: S<'static +, 'static>;
-   |                         ^^^^^^^
+   |              ^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `'static, 'static`


This requires fixing before merging.

Separate lifetimes, type arguments, const params and type bindings as early as possible in the parser. When encountering bad ordering, suggest the correct order in a homogeneous way for all possible incorrect ordering.

rust-highfive · 2019-05-28T07:40:06Z

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

travis_time:end:1e2f9d9c:start=1559024616460807741,finish=1559024617302691999,duration=841884258
$ 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
$ export GCP_CACHE_BUCKET=rust-lang-ci-cache
$ export AWS_ACCESS_KEY_ID=AKIA46X5W6CZEJZ6XT55
---
[01:15:31] .................................................................................................... 4700/5590
[01:15:38] .................................................................................................... 4800/5590
[01:15:45] .................................................................................................... 4900/5590
[01:15:48] .................................................................................................... 5000/5590
[01:15:53] ...........................................................................F........................ 5100/5590
[01:16:02] .................................................................................................... 5300/5590
[01:16:05] .................................................................................................... 5400/5590
[01:16:08] .................................................................................................... 5500/5590
[01:16:11] ............................i.............................................................
---
[01:16:11] 1 error: incorrect parameter order
[01:16:11] -   --> $DIR/suggest-move-types.rs:28:20
[01:16:11] +   --> $DIR/suggest-move-types.rs:26:20
[01:16:11] 3    |
[01:16:11] 4 LL | struct A<T, M: One<A=(), T>> {
[01:16:11] 5    |                    ^^^^^^^ help: reorder the arguments: `T, A = ()`
[01:16:11] 6 
[01:16:11] 7 error: incorrect parameter order
[01:16:11] -   --> $DIR/suggest-move-types.rs:34:37
[01:16:11] +   --> $DIR/suggest-move-types.rs:32:37
[01:16:11] +   --> $DIR/suggest-move-types.rs:32:37
[01:16:11] 9    |
[01:16:11] 10 LL | struct Al<'a, T, M: OneWithLifetime<A=(), T, 'a>> {
[01:16:11] 11    |                                     ^^^^^^^^^^^ help: reorder the arguments: `'a, T, A = ()`
[01:16:11] 12 
[01:16:11] 13 error: incorrect parameter order
[01:16:11] -   --> $DIR/suggest-move-types.rs:40:28
[01:16:11] +   --> $DIR/suggest-move-types.rs:38:28
[01:16:11] +   --> $DIR/suggest-move-types.rs:38:28
[01:16:11] 15    |
[01:16:11] 16 LL | struct B<T, U, V, M: Three<A=(), B=(), C=(), T, U, V>> {
[01:16:11] 17    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `T, U, V, A = (), B = (), C = ()`
[01:16:11] 18 
[01:16:11] 19 error: incorrect parameter order
[01:16:11] -   --> $DIR/suggest-move-types.rs:47:53
[01:16:11] +   --> $DIR/suggest-move-types.rs:45:53
[01:16:11] +   --> $DIR/suggest-move-types.rs:45:53
[01:16:11] 21    |
[01:16:11] 22 LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<A=(), B=(), C=(), T, U, V, 'a, 'b, 'c>> {
[01:16:11] 23    |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `'a, 'b, 'c, T, U, V, A = (), B = (), C = ()`
[01:16:11] 24 
[01:16:11] 25 error: incorrect parameter order
[01:16:11] -   --> $DIR/suggest-move-types.rs:55:28
[01:16:11] +   --> $DIR/suggest-move-types.rs:53:28
[01:16:11] +   --> $DIR/suggest-move-types.rs:53:28
[01:16:11] 27    |
[01:16:11] 28 LL | struct C<T, U, V, M: Three<T, A=(), B=(), C=(), U, V>> {
[01:16:11] 29    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `T, U, V, A = (), B = (), C = ()`
[01:16:11] 30 
[01:16:11] 31 error: incorrect parameter order
[01:16:11] -   --> $DIR/suggest-move-types.rs:62:53
[01:16:11] +   --> $DIR/suggest-move-types.rs:60:53
[01:16:11] +   --> $DIR/suggest-move-types.rs:60:53
[01:16:11] 33    |
[01:16:11] 34 LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<T, 'a, A=(), B=(), C=(), U, 'b, V, 'c>> {
[01:16:11] 35    |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `'a, 'b, 'c, T, U, V, A = (), B = (), C = ()`
[01:16:11] 36 
[01:16:11] 37 error: incorrect parameter order
[01:16:11] -   --> $DIR/suggest-move-types.rs:70:28
[01:16:11] +   --> $DIR/suggest-move-types.rs:68:28
[01:16:11] +   --> $DIR/suggest-move-types.rs:68:28
[01:16:11] 39    |
[01:16:11] 40 LL | struct D<T, U, V, M: Three<T, A=(), B=(), U, C=(), V>> {
[01:16:11] 41    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `T, U, V, A = (), B = (), C = ()`
[01:16:11] 42 
[01:16:11] 43 error: incorrect parameter order
[01:16:11] -   --> $DIR/suggest-move-types.rs:77:53
[01:16:11] +   --> $DIR/suggest-move-types.rs:75:53
[01:16:11] +   --> $DIR/suggest-move-types.rs:75:53
[01:16:11] 45    |
[01:16:11] 46 LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<T, 'a, A=(), B=(), U, 'b, C=(), V, 'c>> {
[01:16:11] 47    |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `'a, 'b, 'c, T, U, V, A = (), B = (), C = ()`
[01:16:11] 
[01:16:11] The actual stderr differed from the expected stderr.
[01:16:11] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/suggestions/suggest-move-types/suggest-move-types.stderr
[01:16:11] Actual stderr saved to /checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/suggestions/suggest-move-types/suggest-move-types.stderr
[01:16:11] To update references, rerun the tests and pass the `--bless` flag
[01:16:11] To only update this specific test, also pass `--test-args suggestions/suggest-move-types.rs`
[01:16:11] error: 1 errors occurred comparing output.
[01:16:11] status: exit code: 1
[01:16:11] status: exit code: 1
[01:16:11] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/ui/suggestions/suggest-move-types.rs" "-Zthreads=1" "--target=x86_64-unknown-linux-gnu" "--error-format" "json" "-Zui-testing" "-C" "prefer-dynamic" "--out-dir" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui/suggestions/suggest-move-types" "-Crpath" "-O" "-Cdebuginfo=0" "-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/suggestions/suggest-move-types/auxiliary" "-A" "unused"
[01:16:11] ------------------------------------------
[01:16:11] 
[01:16:11] ------------------------------------------
[01:16:11] stderr:
[01:16:11] stderr:
[01:16:11] ------------------------------------------
[01:16:11] error: incorrect parameter order
[01:16:11]   --> /checkout/src/test/ui/suggestions/suggest-move-types.rs:26:20
[01:16:11]    |
[01:16:11] LL | struct A<T, M: One<A=(), T>> { //~ ERROR incorrect parameter order
[01:16:11]    |                    ^^^^^^^ help: reorder the arguments: `T, A = ()`
[01:16:11] error: incorrect parameter order
[01:16:11]   --> /checkout/src/test/ui/suggestions/suggest-move-types.rs:32:37
[01:16:11]    |
[01:16:11]    |
[01:16:11] LL | struct Al<'a, T, M: OneWithLifetime<A=(), T, 'a>> {
[01:16:11]    |                                     ^^^^^^^^^^^ help: reorder the arguments: `'a, T, A = ()`
[01:16:11] error: incorrect parameter order
[01:16:11]   --> /checkout/src/test/ui/suggestions/suggest-move-types.rs:38:28
[01:16:11]    |
[01:16:11]    |
[01:16:11] LL | struct B<T, U, V, M: Three<A=(), B=(), C=(), T, U, V>> { //~ ERROR incorrect parameter order
[01:16:11]    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `T, U, V, A = (), B = (), C = ()`
[01:16:11] error: incorrect parameter order
[01:16:11]   --> /checkout/src/test/ui/suggestions/suggest-move-types.rs:45:53
[01:16:11]    |
[01:16:11]    |
[01:16:11] LL | struct Bl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<A=(), B=(), C=(), T, U, V, 'a, 'b, 'c>> {
[01:16:11]    |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `'a, 'b, 'c, T, U, V, A = (), B = (), C = ()`
[01:16:11] error: incorrect parameter order
[01:16:11]   --> /checkout/src/test/ui/suggestions/suggest-move-types.rs:53:28
[01:16:11]    |
[01:16:11]    |
[01:16:11] LL | struct C<T, U, V, M: Three<T, A=(), B=(), C=(), U, V>> { //~ ERROR incorrect parameter order
[01:16:11]    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `T, U, V, A = (), B = (), C = ()`
[01:16:11] error: incorrect parameter order
[01:16:11]   --> /checkout/src/test/ui/suggestions/suggest-move-types.rs:60:53
[01:16:11]    |
[01:16:11]    |
[01:16:11] LL | struct Cl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<T, 'a, A=(), B=(), C=(), U, 'b, V, 'c>> {
[01:16:11]    |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `'a, 'b, 'c, T, U, V, A = (), B = (), C = ()`
[01:16:11] error: incorrect parameter order
[01:16:11]   --> /checkout/src/test/ui/suggestions/suggest-move-types.rs:68:28
[01:16:11]    |
[01:16:11]    |
[01:16:11] LL | struct D<T, U, V, M: Three<T, A=(), B=(), U, C=(), V>> { //~ ERROR incorrect parameter order
[01:16:11]    |                            ^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `T, U, V, A = (), B = (), C = ()`
[01:16:11] error: incorrect parameter order
[01:16:11]   --> /checkout/src/test/ui/suggestions/suggest-move-types.rs:75:53
[01:16:11]    |
[01:16:11]    |
[01:16:11] LL | struct Dl<'a, 'b, 'c, T, U, V, M: ThreeWithLifetime<T, 'a, A=(), B=(), U, 'b, C=(), V, 'c>> {
[01:16:11]    |                                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: reorder the arguments: `'a, 'b, 'c, T, U, V, A = (), B = (), C = ()`
[01:16:11] error: aborting due to 8 previous errors
[01:16:11] 
[01:16:11] 
[01:16:11] ------------------------------------------
---
[01:16:11] thread 'main' panicked at 'Some tests failed', src/tools/compiletest/src/main.rs:521:22
[01:16:11] note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
[01:16:11] 
[01:16:11] 
[01:16:11] 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" "--src-base" "/checkout/src/test/ui" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/ui" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "ui" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-6.0/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--target-rustcflags" "-Crpath -O -Cdebuginfo=0 -Zunstable-options  -Lnative=/checkout/obj/build/x86_64-unknown-linux-gnu/native/rust-test-helpers" "--docck-python" "/usr/bin/python2.7" "--lldb-python" "/usr/bin/python2.7" "--gdb" "/usr/bin/gdb" "--quiet" "--llvm-version" "6.0.0\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[01:16:11] 
[01:16:11] 
[01:16:11] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:16:11] Build completed unsuccessfully in 0:05:15
[01:16:11] Build completed unsuccessfully in 0:05:15
[01:16:11] make: *** [check] Error 1
[01:16:11] Makefile:48: recipe for target 'check' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:0a6d5b96
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Tue May 28 07:40:00 UTC 2019

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)

petrochenkov · 2019-05-28T08:38:28Z

Looks like this is a revert of #45930 basically.
cc @eddyb @varkor

eddyb · 2019-05-28T15:14:14Z

src/libsyntax/ast.rs

-    pub args: Vec<GenericArg>,
+    pub lifetimes: Vec<Lifetime>,
+    pub types: Vec<P<Ty>>,
+    pub const_args: Vec<AnonConst>,


How are you going to preserve the correct order? Types and constants can be interleaved, last I checked.

(They're currently blocked from being so, but this restriction was not necessarily intended to be permanent.)

eddyb · 2019-05-28T15:17:10Z

I don't see the point of this revert - how does it help diagnostics at all?
Also, it goes in the wrong direction for the AST - it's replacing a representation of syntax with a "baked down" view (which we're not using even lower down in the compiler, ironically, because it's not worthwhile).

varkor · 2019-05-28T15:29:40Z

It looks like this regresses diagnostics too?

estebank · 2019-05-29T17:54:58Z

I'll rework the approach. I'm still not sure I understand the benefit of having lifetimes share the same vec as type args and const args, while we do separate bindings, but evidently it is something I'm not seeing.

It looks like this regresses diagnostics too?

This was a first pass to try and unify the "incorrectly ordered args" in one place, the second pass would have been wording/polish. I first started only doing the diagnostic part and realized that there are multiple places where we didn't care about most of the arg vec, we either only cared about lifetimes or type params. It ballooned into this PR.

How are you going to preserve the correct order? Types and constants can be interleaved, last I checked.

(They're currently blocked from being so, but this restriction was not necessarily intended to be permanent.)

Point taken, although that doesn't preclude separating lifetimes away :)

Looks like this is a revert of #45930 basically.

I wasn't aware of that PR, but the spirit behind this is the same as the one expressed in #45930 (comment).

@eddyb advised making the parser agnostic and making these checks only later in typeck. I'll look into it in a separate PR.

rust-highfive assigned michaelwoerister May 28, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 28, 2019

estebank force-pushed the type-params branch from 4487191 to f05c4d7 Compare May 28, 2019 05:58

estebank force-pushed the type-params branch from f05c4d7 to 435e20e Compare May 28, 2019 05:59

estebank commented May 28, 2019

View reviewed changes

This comment has been minimized.

Sign in to view

Rework how type params are parsed

4f2d183

Separate lifetimes, type arguments, const params and type bindings as early as possible in the parser. When encountering bad ordering, suggest the correct order in a homogeneous way for all possible incorrect ordering.

estebank force-pushed the type-params branch from 435e20e to 4f2d183 Compare May 28, 2019 06:22

eddyb reviewed May 28, 2019

View reviewed changes

estebank closed this May 29, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[wip] Rework how type params are parsed #61254

[wip] Rework how type params are parsed #61254

estebank commented May 28, 2019

rust-highfive commented May 28, 2019

estebank commented May 28, 2019

estebank May 28, 2019

This comment has been minimized.

rust-highfive commented May 28, 2019

petrochenkov commented May 28, 2019

eddyb May 28, 2019

varkor May 28, 2019

eddyb commented May 28, 2019 •

edited

Loading

varkor commented May 28, 2019

estebank commented May 29, 2019

[wip] Rework how type params are parsed #61254

[wip] Rework how type params are parsed #61254

Conversation

estebank commented May 28, 2019

rust-highfive commented May 28, 2019

estebank commented May 28, 2019

estebank May 28, 2019

Choose a reason for hiding this comment

This comment has been minimized.

rust-highfive commented May 28, 2019

petrochenkov commented May 28, 2019

eddyb May 28, 2019

Choose a reason for hiding this comment

varkor May 28, 2019

Choose a reason for hiding this comment

eddyb commented May 28, 2019 • edited Loading

varkor commented May 28, 2019

estebank commented May 29, 2019

eddyb commented May 28, 2019 •

edited

Loading