Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DONT MERGE] Always validate HIR ID for TypeckTables #64262

Conversation

Xanewok
Copy link
Member

@Xanewok Xanewok commented Sep 7, 2019

...and not only with debug_assertions compiled in - #64250 (comment).

cc @Mark-Simulacrum @michaelwoerister

Let's do a try run to see if this impacts perf anyhow?

r? @ghost

...and not only with `debug_assertions` compiled in
@Xanewok
Copy link
Member Author

Xanewok commented Sep 7, 2019

@bors try

@bors
Copy link
Contributor

bors commented Sep 7, 2019

⌛ Trying commit f6ae4a4 with merge 1c025bb...

bors added a commit that referenced this pull request Sep 7, 2019
…es, r=<try>

[DONT MERGE] Always validate HIR ID for TypeckTables

...and not only with `debug_assertions` compiled in - #64250 (comment).

cc @Mark-Simulacrum @michaelwoerister

Let's do a try run to see if this impacts perf anyhow?

r? @ghost
@bors
Copy link
Contributor

bors commented Sep 7, 2019

💔 Test failed - checks-azure

@rust-highfive
Copy link
Collaborator

The job dist-x86_64-linux-alt of your PR failed (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.
2019-09-07T19:03:13.3436563Z [RUSTC-TIMING] cmake test:false 3.998
2019-09-07T19:03:13.3476284Z    Compiling compiler_builtins v0.1.18
2019-09-07T19:03:16.4180781Z [RUSTC-TIMING] build_script_build test:false 3.064
2019-09-07T19:03:16.4214505Z    Compiling unwind v0.0.0 (/checkout/src/libunwind)
2019-09-07T19:03:16.6846620Z error: internal compiler error: src/librustc/ty/context.rs:214: node type <T>::IntoIter (hir_id=HirId { owner: DefIndex(4038), local_id: 61 }) with HirId::owner DefId(0:4038 ~ core[dca0]::iter[0]::adapters[0]::flatten[0]::{{impl}}[13]::try_fold[0]::flatten[0]) cannot be placed in TypeckTables with local_id_root DefId(0:4034 ~ core[dca0]::iter[0]::adapters[0]::flatten[0]::{{impl}}[13]::try_fold[0])
2019-09-07T19:03:16.6847313Z thread 'rustc' panicked at 'Box<Any>', src/librustc_errors/lib.rs:643:9
2019-09-07T19:03:16.6847434Z note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
2019-09-07T19:03:16.8476406Z error: aborting due to previous error
2019-09-07T19:03:16.8476624Z 
2019-09-07T19:03:16.8476624Z 
2019-09-07T19:03:16.9414430Z 
2019-09-07T19:03:16.9414900Z note: the compiler unexpectedly panicked. this is a bug.
2019-09-07T19:03:16.9419911Z 
2019-09-07T19:03:16.9421742Z note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
2019-09-07T19:03:16.9431241Z 
2019-09-07T19:03:16.9432000Z note: rustc 1.39.0-nightly (1c025bbf2 2019-09-07) running on x86_64-unknown-linux-gnu
2019-09-07T19:03:16.9432070Z 
2019-09-07T19:03:16.9432630Z note: compiler flags: -Z binary-dep-depinfo -Z external-macro-backtrace -Z save-analysis -Z force-unstable-if-unmarked -C opt-level=2 -C linker=clang -C debuginfo=1 -C prefer-dynamic -C linker=clang -C debug-assertions=n -C codegen-units=1 -C link-args=-Wl,-rpath,$ORIGIN/../lib --crate-type lib
2019-09-07T19:03:16.9432885Z note: some of the compiler flags provided by cargo are hidden
2019-09-07T19:03:16.9432954Z 
2019-09-07T19:03:16.9845731Z [RUSTC-TIMING] core test:false 21.083
2019-09-07T19:03:16.9846186Z error: Could not compile `core`.
---
2019-09-07T19:03:17.5871203Z == clock drift check ==
2019-09-07T19:03:17.5892387Z   local time: Sat Sep  7 19:03:17 UTC 2019
2019-09-07T19:03:17.8673895Z   network time: Sat, 07 Sep 2019 19:03:17 GMT
2019-09-07T19:03:17.8675888Z == end clock drift check ==
2019-09-07T19:03:21.9005854Z ##[error]Bash exited with code '1'.
2019-09-07T19:03:21.9047804Z ##[section]Starting: Upload CPU usage statistics
2019-09-07T19:03:21.9053339Z ==============================================================================
2019-09-07T19:03:21.9053462Z Task         : Bash
2019-09-07T19:03:21.9053542Z Description  : Run a Bash script on macOS, Linux, or Windows

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)

@bors bors added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 7, 2019
Fixes an issue where we did not nest tables correctly when resolving
associated types in formal argument/return type positions
@Xanewok
Copy link
Member Author

Xanewok commented Sep 7, 2019

dist run failed as expected because that's what #64250 is meant to fix but since I'm not sure if that generates artifacts that are enough to do a perf run (@Mark-Simulacrum does it?), I'm cherry-picking the patch from #64250 and trying again

@bors try

@bors
Copy link
Contributor

bors commented Sep 7, 2019

⌛ Trying commit 9bc434c with merge 647e87d...

bors added a commit that referenced this pull request Sep 7, 2019
…es, r=<try>

[DONT MERGE] Always validate HIR ID for TypeckTables

...and not only with `debug_assertions` compiled in - #64250 (comment).

cc @Mark-Simulacrum @michaelwoerister

Let's do a try run to see if this impacts perf anyhow?

r? @ghost
@Mark-Simulacrum
Copy link
Member

Uh, if the dist run failed then no artifacts would've been generated AFAIK.

@rust-highfive
Copy link
Collaborator

The job LinuxTools of your PR failed (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.
2019-09-07T19:19:12.3228506Z ##[command]git remote add origin https://github.com/rust-lang/rust
2019-09-07T19:19:12.3381513Z ##[command]git config gc.auto 0
2019-09-07T19:19:12.3457064Z ##[command]git config --get-all http.https://github.com/rust-lang/rust.extraheader
2019-09-07T19:19:12.3521805Z ##[command]git config --get-all http.proxy
2019-09-07T19:19:12.3686631Z ##[command]git -c http.extraheader="AUTHORIZATION: basic ***" fetch --force --tags --prune --progress --no-recurse-submodules --depth=2 origin +refs/heads/*:refs/remotes/origin/* +refs/pull/64262/merge:refs/remotes/pull/64262/merge
---
2019-09-07T21:19:16.8714600Z    Compiling rustbook v0.1.0 (/checkout/src/tools/rustbook)
2019-09-07T21:19:21.8454776Z     Finished release [optimized] target(s) in 9m 09s
2019-09-07T21:21:13.7524783Z Error: there are broken links
2019-09-07T21:21:13.7526153Z  Caused By: There was an error while fetching "https://public.etherpad-mozilla.org/p/rust-compiler-meeting", https://public.etherpad-mozilla.org/p/rust-compiler-meeting: error trying to connect: Connection refused (os error 111)
2019-09-07T21:21:13.7526939Z  Caused By: There was an error while fetching "http://www.cs.ucla.edu/~palsberg/tba/papers/tofte-talpin-iandc97.pdf", http://www.cs.ucla.edu/~palsberg/tba/papers/tofte-talpin-iandc97.pdf: timed out
2019-09-07T21:21:13.7537536Z 
2019-09-07T21:21:13.7538411Z command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/rustbook" "linkcheck" "/checkout/src/doc/rustc-guide"
2019-09-07T21:21:13.7538512Z expected success, got: exit code: 101
2019-09-07T21:21:13.7538544Z 
---
2019-09-07T21:34:12.1856541Z Verifying status of rustfmt...
2019-09-07T21:34:12.1872506Z Verifying status of clippy-driver...
2019-09-07T21:34:12.1886576Z This PR updated 'src/tools/clippy', verifying if status is 'test-pass'...
2019-09-07T21:34:12.1906906Z 
2019-09-07T21:34:12.1907465Z ⚠️ We detected that this PR updated 'clippy-driver', but its tests failed.
2019-09-07T21:34:12.1907528Z 
2019-09-07T21:34:12.1910713Z If you do intend to update 'clippy-driver', please check the error messages above and
2019-09-07T21:34:12.1910793Z commit another update.
2019-09-07T21:34:12.1911642Z 
2019-09-07T21:34:12.1912028Z If you do NOT intend to update 'clippy-driver', please ensure you did not accidentally
2019-09-07T21:34:12.1912305Z change the submodule at 'src/tools/clippy'. You may ask your reviewer for the
2019-09-07T21:34:12.1912356Z proper steps.
2019-09-07T21:34:12.1927985Z   local time: Sat Sep  7 21:34:12 UTC 2019
2019-09-07T21:34:12.2829109Z   network time: Sat, 07 Sep 2019 21:34:12 GMT
2019-09-07T21:34:12.2834891Z == end clock drift check ==
2019-09-07T21:34:12.2834891Z == end clock drift check ==
2019-09-07T21:34:13.8651715Z ##[error]Bash exited with code '3'.
2019-09-07T21:34:13.8694471Z ##[section]Starting: Checkout
2019-09-07T21:34:13.8696789Z ==============================================================================
2019-09-07T21:34:13.8696844Z Task         : Get sources
2019-09-07T21:34:13.8697102Z Description  : Get sources from a repository. Supports Git, TfsVC, and SVN repositories.

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)

@bors
Copy link
Contributor

bors commented Sep 7, 2019

☀️ Try build successful - checks-azure
Build commit: 647e87d

@Xanewok
Copy link
Member Author

Xanewok commented Sep 7, 2019

@rust-timer build 647e87d

@rust-timer
Copy link
Collaborator

Success: Queued 647e87d with parent ef54f57, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit 647e87d, comparison URL.

@Xanewok
Copy link
Member Author

Xanewok commented Sep 8, 2019

I tried to pick an early base but I could only do with a date start: https://perf.rust-lang.org/compare.html?start=2019-09-06&end=647e87d6d5f99f6a8f2497a7af33393598398b67 (later commits are not available?)

Instruction-wise, inflate regressed ~2% and keccak ~1%, rest seems to be a noise?

@Mark-Simulacrum I'd appreciate another set of eyes on the perf results; do these seem reasonable?

@Xanewok
Copy link
Member Author

Xanewok commented Sep 8, 2019

Okay, the immediate results are in and I believe these look like noise, so maybe we can land it?

@Mark-Simulacrum
Copy link
Member

Yeah, I'll go ahead and close this PR but the other PR should now not have any performance concerns.

@Xanewok Xanewok deleted the always-validate-hir-id-for-typeck-tables branch September 8, 2019 19:10
Xanewok added a commit to Xanewok/rust that referenced this pull request Sep 8, 2019
Performance shouldn't be impacted (see [1] for a perf run) and this
should allow us to catch more bugs, e.g. [2] and [3].

[1]: rust-lang#64262
[2]: rust-lang#64250
[3]: rust-lang#57298
Xanewok added a commit to Xanewok/rust that referenced this pull request Sep 13, 2019
Performance shouldn't be impacted (see [1] for a perf run) and this
should allow us to catch more bugs, e.g. [2] and [3].

[1]: rust-lang#64262
[2]: rust-lang#64250
[3]: rust-lang#57298
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants