Use InternedString instead of Symbol for type parameter types by michaelwoerister · Pull Request #49266 · rust-lang/rust

michaelwoerister · 2018-03-22T10:56:51Z

This PR addresses the regression in #48923 and fixes this particular instance. However, since Symbol is still used in other places, there might still be instances where this is a problem. At least, this PR makes sure that we run into an assertion immediately instead of silently corrupting the compiler state.

The underlying problem is that we cannot properly hash "gensymed" Symbols (at least not in a stable way), so that two different symbols, with the same string contents but different interning keys, will result in two different query keys but the same DepNode because the later is based only on the string contents of the symbol.

r? @nikomatsakis

This PR makes the Eq and Hash implementations for InternedString pointer- instead of string-based. It would be great if @eddyb or somebody else from @rust-lang/compiler could double-check the logic here.

michaelwoerister · 2018-03-22T10:57:16Z

@bors p=1 (since this fixes a regression)
cc @rust-lang/release @rust-lang/infra

michaelwoerister · 2018-03-22T11:00:55Z

In theory this bug is present since last summer. However, I think it's only trigger but rather new code, since we haven't seen it before and then multiple people ran into it at once. So I don't think we need to backport.

nikomatsakis · 2018-03-22T14:08:51Z

@michaelwoerister you have to regenerate the ui tests:

https://travis-ci.org/rust-lang/rust/builds/356813280#L2484

some of them dump debug info

eddyb · 2018-03-22T14:12:02Z

If you want to keep this, it should check the verbose flag via ty::tls - also, you meant len instead of ptr, right?

hmm. that would be verbose indeed. I'd almost rather yet another flag -- but really what we need to do is make {:?} and (today's) -Zverbose equivalent. That is, I had hoped that -Zverbose would just make {} and {:?} do the same thing, not go altering debug to dump yet more information.

Whoops, that was not supposed to be part of the comment. It's a leftover from debugging. Unless you thing it is generally useful, I'd just remove it.

eddyb · 2018-03-22T14:16:20Z

Why the changes to InternedString to hash by pointer? Has everything needing the "hash the contents" behavior moved to StableHash? Can't we just use Symbol in that case?
If there are uses of Hash needing to hash the contents, why change InternedString? I'm confused.

nikomatsakis · 2018-03-22T16:52:35Z

 use serialize::{Decodable, Decoder, Encodable, Encoder};
 use std::collections::HashMap;
 use std::fmt;
+use std::ops::Deref;


any particular reason for this change?

(just efficiency, I guess?)

nikomatsakis · 2018-03-22T16:53:40Z

 pub fn mk_param(self,
 index: u32,
- name: Name) -> Ty<'tcx> {
+ name: InternedString) -> Ty<'tcx> {


this change seems fine to me (as discussed on IRC); name should only be used for debug print-outs etc

nikomatsakis · 2018-03-22T16:56:49Z

Hmm, this could affect code, as these names are compared from time to time. That said, I don't think we support hygiene on lifetime names, right?
The fact that this gives an error suggests I am right.
Maybe @petrochenkov knows.

Lifetime names are hygienic in macros 2.0 -- https://play.rust-lang.org/?gist=f3524076b1ad62bacb3979a98ebb72b2&version=nightly

Does it matter this late in the pipeline? I thought hygiene would have been satisfied at this point by assigning different DefIds.

hmm, you might be right.

if that is true, then, given that we have the def-id, we could probably remove the InternedString altogether. The fact that we didn't... well, either an accident, or intentional. I remember @eddyb was playing with this stuff.

nikomatsakis · 2018-03-22T16:58:38Z

hmm. that would be verbose indeed. I'd almost rather yet another flag -- but really what we need to do is make {:?} and (today's) -Zverbose equivalent. That is, I had hoped that -Zverbose would just make {} and {:?} do the same thing, not go altering debug to dump yet more information.

Zoxc · 2018-03-22T18:16:34Z

Why the changes to InternedString to hash by pointer? Has everything needing the "hash the contents" behavior moved to StableHash?

I did run into code relying on this when I tried to make InternedString store a Symbol.

petrochenkov · 2018-03-22T22:38:57Z

I don't quite understand when exactly these names are hashed (and why this PR fixes the issue), so hashing string contents may be okay here if it's done late enough, but in general using string comparisons/hashing for identifiers like lifetimes or type parameters (or comparisons of pointers to strings, which are equivalent with our implementation of string interner and gensyms) is a way to undermine hygiene.

As long as gensyms exist, we should not rely on string comparison/hashing. If gensyms need to be hashed for incremental compilation or something like this, we need to invent some predictable scheme for encoding them instead (e.g. "base" Symbol + count of gensyms produced from that base).

Regarding replacement of Symbol with InternedString, in general

InternedString is supposed to be used with non-identifier strings for which no hygiene exist and gensyms are impossible (e.g. string literals, or mangled names during linking, or something like this)
Ident is supposed to be used with identifier strings for which hygiene exists and gensyms are possible.
Symbol (aka Name, aka raw interner index) is ideally just an implementation detail of both Ident and InternedString, but right now Symbol is used as Ident in HIR through some horrible scheme with gensyms and side tables (I hope to fix this after AST: Give spans to all identifiers #49154 lands) + InternedString doesn't use Symbol directly (should be fixed by something like Store a Symbol in InternedString #46972).

michaelwoerister · 2018-03-23T09:11:01Z

Why the changes to InternedString to hash by pointer? Has everything needing the "hash the contents" behavior moved to StableHash? Can't we just use Symbol in that case?
If there are uses of Hash needing to hash the contents, why change InternedString? I'm confused.

Comparision by pointer is for efficiency because I saw things like this:

rust/src/librustc/ty/sty.rs

Lines 887 to 894 in 52f7e88

pubfnis_self(&self) -> bool{
ifself.name == keywords::SelfType.name(){
assert_eq!(self.idx,0);
true
}else{
false
}
}

The Hash implementation is updated to use the pointer as well because we rely on the two to be consistent. And it seemed like a good idea to take advantage of InternedString uniqueness as we already do for all other interned things.

The PR tries to move away from Symbol because there is no good way to implement HashStable for it (because of how gensym is implemented).

michaelwoerister · 2018-03-23T09:21:05Z

@petrochenkov

InternedString is supposed to be used with non-identifier strings for which no hygiene exist and gensyms are impossible (e.g. string literals, or mangled names during linking, or something like this)

Yes, that's what this PR tries to do: Replace Symbol with InternedString in a place where only the string contents should matter. Using Ident would also be an option, but I don't think that's possible, right?

petrochenkov · 2018-03-23T10:14:41Z

@michaelwoerister
In longer term we should probably introduce something like ResolvedIdent for cases like this (i.e. "previously a hygienic identifier, now already resolved to DefId").

// Does not implement `PartialEq`structResolvedIdent{// True identity of the identifier, was previously obtained by hygienic resolutiondef_id:DefId,// Purely informational string, should never be compared, but can be e.g. displayed in error messagesinfo:InternedString,}

My goal is to avoid footguns first of all. I've seen enough times code in later stages of compilation (e.g. pattern exhaustiveness checking) behaving like "what hygiene? let's just compare these two fields by their string contents". InternedString for something that previously was an Ident should never go alone.

michaelwoerister · 2018-03-23T10:50:15Z

My goal is to avoid footguns first of all.

That's a very good goal to have! :)

How should we proceed here? I think I'll do a minimal version of the fix and close this one.

michaelwoerister · 2018-03-23T11:31:13Z

Le sigh, now I remember why I didn't keep the fix minimal: because it's ugly and requires converting back and forth between Symbol and InternedString in several places.

I opened #49300 to keep track of the issue. Not sure how to proceed.

petrochenkov · 2018-03-25T23:43:50Z

Ok, now I think I finally understand what causes the ICE.
TypeVariableOrigin and friends (maybe as parts of larger structures) serve as keys in a map that uses derived PartialEq and Hash for mapping (not stable hash), but we need this mapping to be identical with string-content-based stable hashing.
This is why TypeVariableOrigin and friends need to use string types for which derived PartialEq and Hash will be string-content-based too (i.e. InternedString).

Since this is a temporary solution, I'm happy with this PR as is.

Zoxc · 2018-03-25T23:49:31Z

This is why TypeVariableOrigin and friends need to use string types for which derived PartialEq and Hash will be string-content-based too (i.e. InternedString).

This makes InternedString's Hash impl no longer string-content-based though.

petrochenkov · 2018-03-26T00:23:03Z

@Zoxc
I mean equivalent to string-content-based during one compilation session (this should be enough as I understand it) - pointer hashes are equal if and only if string contents are equal with our interning scheme (modulo hash collisions).

TimNN · 2018-04-03T15:24:50Z

Your PR failed on Travis. 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.

Resolving deltas: 100% (607325/607325), completed with 4806 local objects.
---
[00:00:43] configure: rust.quiet-tests := True
---
[00:44:10] ..........................................................................i.........................
[00:44:17] .................i..................................................................................
---
[00:44:54] .............................................................................................i......
[00:45:02] .................................................................i..................................
---
[00:46:00] .............................................i......................................................
---
[00:50:10] .............................i......................................................................
[00:50:25] ..............................................................i.....................................
[00:50:42] ...............................................i....................................................
[00:51:03] ....................................................................................................
[00:51:26] ....................................................................................................
[00:51:49] ....................................................................................................
[00:52:16] .i...............................................................................................i..
[00:52:41] ....................................................................test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:52:55] ................................
[00:53:28] ....................................................................................................
[00:54:07] ..............................................................ii....................................
[00:54:49] .........................i............................................test [run-pass] run-pass/saturating-float-casts.rs has been running for over 60 seconds
[00:55:00] ........i.ii..................
[00:55:43] ...............................F......................................................iiiiiii.......
[00:56:14] ....................................................................................................
[00:56:42] ....................................................................................................
[00:57:07] ....................................................................................................
:23] stderr:
[00:57:23] ------------------------------------------
[00:57:23] error[E0460]: found possibly newer version of crate `a` which `b` depends on
[00:57:23] --> /checkout/src/test/run-pass/svh-add-nothing.rs:19:1
[00:57:23] |
[00:57:23] 19 | extern crate b;
[00:57:23] | ^^^^^^^^^^^^^^^
[00:57:23] |
[00:57:23] = note: perhaps that crate needs to be recompiled?
[00:57:23] = note: the following crate versions were found:
[00:57:23] crate `a`: /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/svh-add-nothing.stage2-x86_64-unknown-linux-gnu.aux/liba.so
[00:57:23] crate `b`: /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/svh-add-nothing.stage2-x86_64-unknown-linux-gnu.aux/libb.so
---
[00:57:23] thread '[run-pass] run-pass/svh-add-nothing.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2901:9
---
[00:57:23] 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/run-pass" "--build-base" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--stage-id" "stage2-x86_64-unknown-linux-gnu" "--mode" "run-pass" "--target" "x86_64-unknown-linux-gnu" "--host" "x86_64-unknown-linux-gnu" "--llvm-filecheck" "/usr/lib/llvm-3.9/bin/FileCheck" "--host-rustcflags" "-Crpath -O -Zmiri -Zunstable-options " "--target-rustcflags" "-Crpath -O -Zmiri -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" "3.9.1\n" "--system-llvm" "--cc" "" "--cxx" "" "--cflags" "" "--llvm-components" "" "--llvm-cxxflags" "" "--adb-path" "adb" "--adb-test-dir" "/data/tmp/work" "--android-cross-path" "" "--color" "always"
[00:57:23] expected success, got: exit code: 101
[00:57:23]
[00:57:23]
[00:57:23] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[00:57:23] Build completed unsuccessfully in 0:14:05
[00:57:23] Makefile:58: recipe for target 'check' failed
[00:57:23] make: *** [check] Error 1
---
56700 ./obj/build/x86_64-unknown-linux-gnu/stage1-rustc/x86_64-unknown-linux-gnu/release/incremental/syntax-33oa6nnkk1g08/s-ezry20nkee-1ezllpu-2cfdg74j6tjst
---
34/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:0338468a:start=1522769086835762324,finish=1522769086843587149,duration=7824825
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:0141643a
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:0141643a:start=1522769086850024326,finish=1522769086857617025,duration=7592699
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:21947c2e
$ dmesg | grep -i kill
[ 10.534524] init: failsafe main process (1094) killed by TERM signal

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.

TimNN · 2018-04-03T16:57:56Z

Your PR failed on Travis. 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.

Resolving deltas: 100% (607334/607334), completed with 4807 local objects.
---
[00:00:46] configure: rust.quiet-tests := True
---
[00:41:22] ..........................................................................i.........................
[00:41:29] .................i..................................................................................
---
[00:42:09] .............................................................................................i......
[00:42:16] .................................................................i..................................
---
[00:43:14] .............................................i......................................................
---
[00:47:18] .............................i......................................................................
[00:47:34] ..............................................................i.....................................
[00:47:50] ...............................................i....................................................
[00:48:10] ....................................................................................................
[00:48:32] ....................................................................................................
[00:48:53] ....................................................................................................
[00:49:19] .i...............................................................................................i..
[00:49:46] ................................................................................test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:49:57] ....................
[00:50:28] ....................................................................................................
[00:51:03] ..............................................................ii....................................
[00:51:53] .........................i....................................................i.ii..................
[00:52:33] ...............................F......................................................iiiiiii.......
---
[00:54:09] command: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc" "/checkout/src/test/run-pass/svh-add-nothing.rs" "-L" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass" "--target=x86_64-unknown-linux-gnu" "-C" "prefer-dynamic" "-o" "/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/svh-add-nothing.stage2-x86_64-unknown-linux-gnu" "-Crpath" "-O" "-Zmiri" "-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/run-pass/svh-add-nothing.stage2-x86_64-unknown-linux-gnu.aux"
---
[00:54:09] error[E0460]: found possibly newer version of crate `a` which `b` depends on
[00:54:09] --> /checkout/src/test/run-pass/svh-add-nothing.rs:19:1
[00:54:09] |
[00:54:09] 19 | extern crate b;
[00:54:09] | ^^^^^^^^^^^^^^^
[00:54:09] |
[00:54:09] = note: perhaps that crate needs to be recompiled?
[00:54:09] = note: the following crate versions were found:
[00:54:09] crate `a`: /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/svh-add-nothing.stage2-x86_64-unknown-linux-gnu.aux/liba.so
[00:54:09] crate `b`: /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-pass/svh-add-nothing.stage2-x86_64-unknown-linux-gnu.aux/libb.so
---
[00:54:09] thread '[run-pass] run-pass/svh-add-nothing.rs' panicked at 'explicit panic', tools/compiletest/src/runtest.rs:2901:9
---
$ dmesg | grep -i kill
[ 12.944780] init: failsafe main process (1094) killed by TERM signal

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.

TimNN · 2018-04-04T15:34:41Z

Your PR failed on Travis. 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.

Resolving deltas: 100% (607380/607380), completed with 4806 local objects.
---
[00:00:53] configure: rust.quiet-tests := True
---
[00:44:27] ..........................................................................i.........................
[00:44:33] .................i..................................................................................
---
[00:45:09] .............................................................................................i......
[00:45:16] ................................................................i...................................
---
[00:46:12] .............................................i......................................................
---
[00:50:10] .............................i......................................................................
[00:50:24] ...............................................................i....................................
[00:50:40] ...............................................i....................................................
[00:51:00] ....................................................................................................
[00:51:22] ....................................................................................................
[00:51:44] ....................................................................................................
[00:52:10] .i...............................................................................................i..
[00:52:36] ..............................................................................test [run-pass] run-pass/mir_heavy_promoted.rs has been running for over 60 seconds
[00:52:48] ......................
[00:53:18] ....................................................................................................
[00:53:56] .............................................................ii.....................................
[00:54:39] ........................i....................................................i.ii.test [run-pass] run-pass/saturating-float-casts.rs has been running for over 60 seconds
[00:54:47] ..................
[00:55:28] .....................................................................................iiiiiii........
---
[00:57:31] ....................................i...............................................................
[00:57:39] ....................................................................................................
[00:57:46] ..................i............................................................ii.iii...............
[00:57:54] ....................................................................................................
[00:58:02] ........i..............................i............................................................
[00:58:10] ....................................................................................................
[00:58:17] ....................i...............................................................................
[00:58:25] ....................................................................................................
[00:58:35] ....................................................................................................
[00:58:46] ....................................................................................................
[00:58:57] ....................................................................................................
[00:59:11] ....................................................................................................
[00:59:20] .............i......................................................................................
[00:59:30] ................i..ii...............................................................................
[00:59:40] ....................................................................................................
[00:59:50] ....................................................................................................
[01:00:00] ..................................................................................i.................
[01:00:11] ............................i.......................................................................
---
[01:00:48] ...........................i........................................................................
[01:00:50] ....................................................................i...............................
[01:00:51] ................i.......................................................
---
[01:01:06] ...........i........................
---
[01:01:36] i...i..ii....i.............ii........iii......i..i...i...ii..i..i..ii.....
---
[01:01:39] i.......i......................i......
---
[01:02:17] iiii.......i..i........i..i.i.............i..........iiii...........i...i..........ii.i.i.......ii..
[01:02:19] ....ii...
---
[01:11:54] ...i................................................................................................
---
[01:13:53] .....................................i..............................................................
[01:14:14] ....................................................................................................
[01:14:35] ..........................................i.........................................................
---
[01:16:04] .........................................................ii.........................................
---
[01:17:10] ..............................................................i.....................................
---
[01:22:06] ii..................................................................................................
[01:22:25] ....................................................................................................
[01:22:41] ...................iii......i......i...i......i.....................................................
[01:22:52] ....................................................................................................
[01:23:07] ........................................iiii........ii..............................................
[01:23:19] ....................................................................................................
[01:23:36] .......................................................................................i............
---
[01:27:12] 306 | self.infcx.tcx.mk_param(index, Symbol::intern(&name))
[01:27:12] | ^^^^^^^^^^^^^^^^^^^^^ expected struct `syntax::symbol::InternedString`, found struct `syntax::ast::Symbol`
---
[01:27:13] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "test" "--target" "x86_64-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--features" " jemalloc" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "-p" "rustc_driver" "--" "--quiet"
[01:27:13] expected success, got: exit code: 101
[01:27:13]
[01:27:13]
[01:27:13] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:27:13] Build completed unsuccessfully in 0:43:34
[01:27:13] make: *** [check] Error 1
[01:27:13] Makefile:58: recipe for target 'check' failed
---
$ ls -lat $HOME/Library/Logs/DiagnosticReports/
ls: cannot access /home/travis/Library/Logs/DiagnosticReports/: No such file or directory
travis_time:end:0b2bc66c:start=1522856078564547021,finish=1522856078572098732,duration=7551711
travis_fold:end:after_failure.2
travis_fold:start:after_failure.3
travis_time:start:23aec513
$ find $HOME/Library/Logs/DiagnosticReports -type f -name '*.crash' -not -name '*.stage2-*.crash' -not -name 'com.apple.CoreSimulator.CoreSimulatorService-*.crash' -exec printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" {} \; -exec head -750 {} \; -exec echo travis_fold":"end:crashlog \; || true
find: `/home/travis/Library/Logs/DiagnosticReports': No such file or directory
travis_time:end:23aec513:start=1522856078578242413,finish=1522856078584843690,duration=6601277
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:01877b6c
$ dmesg | grep -i kill
[ 10.763333] init: failsafe main process (1092) killed by TERM signal

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.

TimNN · 2018-04-04T17:46:30Z

Your PR failed on Travis. 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.

Resolving deltas: 100% (607386/607386), completed with 4808 local objects.

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.

michaelwoerister · 2018-04-04T18:12:47Z

Unstable symbol names probably due to this line:

rust/src/librustc_trans_utils/symbol_names.rs

Line 197 in 5758c2d

hasher.hash(tcx.crate_name.as_str());

nikomatsakis · 2018-04-04T20:59:47Z

I'm a bit confused as to the status of this PR. Are we waiting to see if we can make it pass travis? :)

michaelwoerister · 2018-04-05T10:14:34Z

I'm trying to do a trimmed down version suitable backporting. This version would only change type parameter names but not regions.

michaelwoerister · 2018-04-05T12:25:17Z

OK, so this version fixes the regression reported in #48923. It only changes the ParamTy::name field to InternedString while leaving region things alone. It does contain the change to by-pointer comparisons for InternedString.

michaelwoerister · 2018-04-05T14:37:30Z

@eddyb has some reservations about the changes to InternedString. I'll do a separate PR without those changes to test whether performance would regress much. If it doesn't, we can merge and backport that and do the InternedString stuff in a separate PR.

Use InternedString instead of Symbol for type parameter types Reduced alternative to #49266. Let's see if this causes a performance regression.

bors · 2018-04-05T19:28:22Z

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

Use InternedString instead of Symbol for type parameter types (2) Reduced alternative to #49266. Let's see if this causes a performance regression.

michaelwoerister · 2018-04-07T09:56:14Z

Closing in favor of #49695.

…akis Use InternedString instead of Symbol for type parameter types (2) Reduced alternative to #49266. Let's see if this causes a performance regression.

michaelwoerister added 3 commits March 21, 2018 14:40

Compare ast::symbol::InternedString by pointer.
0776b37

Use InternedString instead of Symbol for type parameters.
0451083

InternedString: Make Hash and Eq behave consistently.
833a4d9

rust-highfive assigned nikomatsakis Mar 22, 2018

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

michaelwoerister changed the title ~~Use InternedString instead of Symbol~~Use InternedString instead of Symbol for type and region parameter typesMar 22, 2018

petrochenkov self-assigned this Mar 22, 2018

eddyb reviewed Mar 22, 2018
View reviewed changes

nikomatsakis reviewed Mar 22, 2018
View reviewed changes

petrochenkov approved these changes Mar 25, 2018
View reviewed changes

michaelwoerister added 4 commits April 3, 2018 15:50

Improve assertion in Query::force().
19e5b52

Also compare Symbol/InternedString by pointer
7286ab2

Fix rustdoc after changing type param representation.
103eb00

Make TypeIdHasher not hash type param names by pointer.
1768d4a

michaelwoerister force-pushed the unhygienic-ty branch from aaf19e8 to 1768d4aCompare April 3, 2018 14:22

Don't use InternedString as sorting key during stable hashing.
a27fb55

kennytm mentioned this pull request Apr 4, 2018
Log analysis error for PR 49266 at 2018-04-04T17:46:30Z (PR-CI test, run-make failure) rust-lang/rust-log-analyzer#2
Open

michaelwoerister added 3 commits April 5, 2018 12:21

Add comment about InternedString not being compared lexicographically.
ee89c31

Fix error in rustc_driver::test.
a21a50b

Fix instability in SVH and DefPathHash computation.
697b4d4

michaelwoerister force-pushed the unhygienic-ty branch from 82bd929 to 697b4d4Compare April 5, 2018 10:22

michaelwoerister changed the title ~~Use InternedString instead of Symbol for type and region parameter types~~Use InternedString instead of Symbol for type parameter typesApr 5, 2018

michaelwoerister mentioned this pull request Apr 5, 2018
Use InternedString instead of Symbol for type parameter types (2) #49695
Merged

bors added a commit that referenced this pull request Apr 5, 2018
Auto merge of#49695- michaelwoerister:unhygienic-ty-min, r=<try>
c645af8
Use InternedString instead of Symbol for type parameter types Reduced alternative to #49266. Let's see if this causes a performance regression.

bors added a commit that referenced this pull request Apr 6, 2018
Auto merge of#49695- michaelwoerister:unhygienic-ty-min, r=<try>
b7383f5
Use InternedString instead of Symbol for type parameter types (2) Reduced alternative to #49266. Let's see if this causes a performance regression.

michaelwoerister closed this Apr 7, 2018

Uh oh!

Uh oh!

Conversation

michaelwoerister commented Mar 22, 2018

Uh oh!

michaelwoerister commented Mar 22, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelwoerister commented Mar 22, 2018

Uh oh!

nikomatsakis commented Mar 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eddyb commented Mar 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Zoxc commented Mar 22, 2018

Uh oh!

petrochenkov commented Mar 22, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelwoerister commented Mar 23, 2018

Uh oh!

michaelwoerister commented Mar 23, 2018

Uh oh!

petrochenkov commented Mar 23, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelwoerister commented Mar 23, 2018

Uh oh!

michaelwoerister commented Mar 23, 2018

Uh oh!

petrochenkov commented Mar 25, 2018

Uh oh!

Zoxc commented Mar 25, 2018

Uh oh!

petrochenkov commented Mar 26, 2018

Uh oh!

TimNN commented Apr 3, 2018

Uh oh!

TimNN commented Apr 3, 2018

Uh oh!

TimNN commented Apr 4, 2018

Uh oh!

TimNN commented Apr 4, 2018

Uh oh!

michaelwoerister commented Apr 4, 2018

Uh oh!

nikomatsakis commented Apr 4, 2018

Uh oh!

michaelwoerister commented Apr 5, 2018

Uh oh!

michaelwoerister commented Apr 5, 2018

Uh oh!

michaelwoerister commented Apr 5, 2018

Uh oh!

bors commented Apr 5, 2018

Uh oh!

michaelwoerister commented Mar 22, 2018•
edited
Loading

petrochenkov commented Mar 22, 2018•
edited
Loading

petrochenkov commented Mar 23, 2018•
edited
Loading