rustc: remove type & lifetime parameter names from the typesystem.

[eddyb]

Everything that needs a name, for whatever reason, must get it from the DefId.

Fixes #50125 by replacing special-casing of Self (and the gensym hack) with getting the type for the Self parameter from the trait, and comparing types with that, instead.

Also removes AST pretty-printing for input position impl Trait, it now prints like -> impl Trait.
That is, the relevant ty::Predicates are obtained on the fly from the definition and printed.

r? @nikomatsakis cc @michaelwoerister @varkor @oli-obk @GuillaumeGomez

[eddyb]

@bors try (for perf)

[bors]

⌛ Trying commit a4ede7fbf81ccf0e3685c4b1926c2376f90b81f2 with merge bc159c404dc86f12fe6e0315841c8694ed5fa2ca...

[rust-highfive]

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:55:01] ....................................................................................................
[00:55:04] ....................................................................................................
[00:55:07] .........i..........................................................................................
[00:55:10] ....................................................................................................
[00:55:12] ..........................................................iiiiiiiii.................................
[00:55:18] ....................................................................................................
[00:55:22] ....................................................................................................
[00:55:25] .......................................i............................................................
[00:55:27] .........................................................................................i.i..ii....
---
[01:20:18]    Compiling rustc_driver v0.0.0 (file:///checkout/src/librustc_driver)
[01:20:19] error[E0425]: cannot find function `TyParam` in module `ty`
[01:20:19]    --> librustc_driver/test.rs:316:34
[01:20:19]     |
[01:20:19] 316 |         self.infcx.tcx.mk_ty(ty::TyParam(ty::GenericParam {
[01:20:19] help: possible candidate is found in another module, you can import it into scope
[01:20:19]     |
[01:20:19] 13  | use rustc::hir::def::Def::TyParam;
[01:20:19]     |
[01:20:19]     |
[01:20:19] 
[01:20:19] error[E0560]: struct `rustc::ty::GenericParam` has no field named `idx`
[01:20:19]    --> librustc_driver/test.rs:317:13
[01:20:19]     |
[01:20:19] 317 |             idx: index,
[01:20:19]     |             ^^^ `rustc::ty::GenericParam` does not have this field
[01:20:19]     |
[01:20:19]     = note: available fields are: `def_id`, `index`
[01:20:19] 
[01:20:19] error[E0560]: struct `rustc::ty::GenericParam` has no field named `name`
[01:20:19]    --> librustc_driver/test.rs:327:13
[01:20:19] 327 |             name,
[01:20:19] 327 |             name,
[01:20:19]     |             ^^^^ `rustc::ty::GenericParam` does not have this field
[01:20:19]     |
[01:20:19]     = note: available fields are: `def_id`, `index`
[01:20:20] error: aborting due to 3 previous errors
[01:20:20] 
[01:20:20] Some errors occurred: E0425, E0560.
[01:20:20] For more information about an error, try `rustc --explain E0425`.
[01:20:20] For more information about an error, try `rustc --explain E0425`.
[01:20:20] error: Could not compile `rustc_driver`.
[01:20:20] 
[01:20:20] To learn more, run the command again with --verbose.
[01:20:20] 
[01:20:20] 
[01:20:20] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "-j" "4" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "-p" "rustc_driver" "--" "--quiet"
[01:20:20] 
[01:20:20] 
[01:20:20] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:20:20] Build completed unsuccessfully in 0:29:27
[01:20:20] Build completed unsuccessfully in 0:29:27
[01:20:20] make: *** [check] Error 1
[01:20:20] Makefile:58: recipe for target 'check' failed

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:1db09b30
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)

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)

[michaelwoerister]

This should be a big improvement for #49300 ❤️

[bors]

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

[varkor]

@rust-timer build bc159c404dc86f12fe6e0315841c8694ed5fa2ca

[rust-timer]

Success: Queued bc159c404dc86f12fe6e0315841c8694ed5fa2ca with parent 63d6649, comparison URL.

[lqd]

Perf results are ready

[eddyb]

@nikomatsakis @michaelwoerister I was worried about this (perf results are pretty bad).

First of all, querying def_key shouldn't be that expensive, do you know what's up with that?

Secondly, why would names of generic parameters be needed outside error messages?
Is it us printing Rust types to give names to LLVM types? Anything else?
Worst offender is serde-check so it can't be anything from rustc_codegen_llvm.

EDIT: looking at uses of generic_param_name, none of them seem like could cause regressions, which would suggest it's actually printing types that's slow.
EDIT2: wait, I forgot about the Self change. I opened #53677 to check it.
EDIT3: it's not the Self change.

[eddyb]

So, by using #53677, I was able to determine that the commits after the first two look roughly neutral, whereas the first two commits themselves contain most, if not all, of the regression.

So it's really having the DefId in there that's slowing things down. Maybe making a lot of types in the interner suddenly unshared? That's one reason using only indices is nicer overall.

[rust-highfive]

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:01:37] sha256:4ce6eec60c5e994eefda6b986f14beb415c0e7e1717cdd560041f17bf77eb518
[00:01:37] Attempting with retry: docker build --rm -t rust-ci -f /home/travis/build/rust-lang/rust/src/ci/docker/x86_64-gnu-llvm-5.0/Dockerfile /home/travis/build/rust-lang/rust/src/ci/docker
[00:01:37] Sending build context to Docker daemon  501.8kB
[00:01:37] Step 1/6 : FROM ubuntu:16.04
[00:01:38] received unexpected HTTP status: 503 Service Unavailable
[00:01:39] Sending build context to Docker daemon  501.8kB
[00:01:39] Step 1/6 : FROM ubuntu:16.04
[00:01:39] Step 1/6 : FROM ubuntu:16.04
[00:01:39] received unexpected HTTP status: 503 Service Unavailable
[00:01:41] Sending build context to Docker daemon  501.8kB
[00:01:41] Step 1/6 : FROM ubuntu:16.04
[00:01:41] Step 1/6 : FROM ubuntu:16.04
[00:01:41] received unexpected HTTP status: 503 Service Unavailable
[00:01:45] Sending build context to Docker daemon  501.8kB
[00:01:45] Step 1/6 : FROM ubuntu:16.04
[00:01:45] Step 1/6 : FROM ubuntu:16.04
[00:01:45] received unexpected HTTP status: 503 Service Unavailable
[00:01:49] Sending build context to Docker daemon  501.8kB
[00:01:49] Step 1/6 : FROM ubuntu:16.04
[00:01:49] Step 1/6 : FROM ubuntu:16.04
[00:01:49] received unexpected HTTP status: 503 Service Unavailable
travis_time:end:1d1593b5:start=1535220128883359671,finish=1535220238619769124,duration=109736409453

The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 1.
travis_time:start:0e1fd25f
---
travis_time:end:0351e184:start=1535220239129111675,finish=1535220239140841667,duration=11729992
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0ac8b2d4
$ 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 -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:11b13f4d
travis_time:start:11b13f4d
$ 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:08cc31cc
$ 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)

[nikomatsakis]

@eddyb

So it's really having the DefId in there that's slowing things down. Maybe making a lot of types in the interner suddenly unshared? That's one reason using only indices is nicer overall.

Interesting. As you know, I sort of want to have only indices anyway, but still, I'm a bit surprised to see the impact as large as it is, given that we already have names. I guess we only use a relative small set of names?

[bors]

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

[michaelwoerister]

Looking at the first two commits, nothing jumps out immediately as being slow. Since the slowdown in serde is so visible, running things in a profiler might give a good indications where the slowdown comes from.

[TimNN]

Ping from triage @eddyb: What are your plans for this PR?

[eddyb]

@TimNN I'm not sure. Maybe if @michaelwoerister or @nnethercote can help me profile it, and we end up finding a problem somewhere else that's causing the slowdown, that'd be great?
But otherwise I'd have to make the jump to indices-only and that's much harder to get right.

[nnethercote]

@eddyb: https://github.com/rust-lang-nursery/rustc-perf/tree/master/collector has recently-improved docs on benchmarking and profiling. If you can use Cachegrind, that's probably best because you can get diffs between two different profiles, which is very useful for understanding regressions.

[eddyb]

Callgrind collects the same information as Cachegrind, plus function call information. So it can be used like either Cachegrind or perf-record. However, it cannot perform diffs between profiles.

That's a shame, I like Callgrind. Anyway, I might have a go at this, but not any time soon, due to time constraints on the edition.

[TimNN]

Ping from triage @eddyb: What is the status of this PR?

[TimNN]

Ping from triage @eddyb: Since we haven't heard from you on this PR in a while and since you stated yourself that you won't have time to work on this PR in the near future I'm closing this as inactive for the moment, in line with our triage guidelines. Please re-open in the future and thanks for all your work on rustc!

[eddyb]

@nikomatsakis Did you have a concrete plan for something like ParamEnv but for a global concept of "generic parameters in scope" that's not semantically relevant, only used for diagnostics?
What should enter/exit it? Should we tie it to the query engine, just like tcx.at(span)?