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

Remove NodeId from even more HIR nodes #58836

Merged
merged 8 commits into from
Mar 2, 2019
Merged

Conversation

ljedrz
Copy link
Contributor

@ljedrz ljedrz commented Mar 1, 2019

The next iteration of HirIdification (#57578).

Removes NodeId from:

  • StructField
  • ForeignItem
  • Item
  • Pat
  • FieldPat
  • VariantData
  • ImplItemId (replaces it with HirId)
  • TraitItemId (replaces it with HirId)

@rust-highfive
Copy link
Collaborator

r? @pnkfelix

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

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 1, 2019
@ljedrz
Copy link
Contributor Author

ljedrz commented Mar 1, 2019

r? @Zoxc

@rust-highfive rust-highfive assigned Zoxc and unassigned pnkfelix Mar 1, 2019
@Centril
Copy link
Contributor

Centril commented Mar 1, 2019

@bors p=1

@rust-highfive
Copy link
Collaborator

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:06be49a3:start=1551436312113186340,finish=1551436314706104526,duration=2592918186
$ 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
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-both (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:14:42] 
[01:14:42] running 119 tests
[01:15:07] .iiiii...i.....i..i...i..i.i..i.ii...i.....i..i....i..........iiii..........i...ii...i.......ii.i.i. 100/119
[01:15:11] i......iii.i.....ii
[01:15:11] 
[01:15:11]  finished in 29.572
[01:15:11] travis_fold:end:test_debuginfo

---
[01:36:14] 
[01:36:14] To learn more, run the command again with --verbose.
[01:36:14] 
[01:36:14] 
[01:36:14] 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" "" "--manifest-path" "/checkout/src/rustc/Cargo.toml" "-p" "rustc_driver" "--" "--quiet"
[01:36:14] 
[01:36:14] 
[01:36:14] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test
[01:36:14] Build completed unsuccessfully in 0:33:12
[01:36:14] Build completed unsuccessfully in 0:33:12
[01:36:14] Makefile:48: recipe for target 'check' failed
[01:36:14] make: *** [check] Error 1
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:04a479dd
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Fri Mar  1 12:08:18 UTC 2019
---
travis_time:end:0f1d7211:start=1551442100434435312,finish=1551442100439075918,duration=4640606
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0ca911c5
$ 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 --batch -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:00437b02
travis_time:start:00437b02
$ 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:0b05b536
$ 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)

@ljedrz
Copy link
Contributor Author

ljedrz commented Mar 1, 2019

An error in dead_code ^^; will send a fix shortly.

@ljedrz
Copy link
Contributor Author

ljedrz commented Mar 1, 2019

Blocked by rust-lang/rust-clippy#3834.

Clippy is ready.

@rust-highfive
Copy link
Collaborator

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:0007ce5d:start=1551465744845749876,finish=1551465747336525646,duration=2490775770
$ 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
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:01:52] 
######################################################################## 100.0%
[00:01:52] extracting /checkout/obj/build/cache/2019-02-17/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:01:52]     Updating crates.io index
[00:02:05]     Updating git repository `https://github.com/mati865/compiletest-rs.git`
[00:02:05] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:02:05] Build completed unsuccessfully in 0:00:27
[00:02:05] make: *** [prepare] Error 1
[00:02:05] Makefile:70: recipe for target 'prepare' failed
[00:02:06] Command failed. Attempt 2/5:
[00:02:06] Command failed. Attempt 2/5:
[00:02:07]     Updating git repository `https://github.com/mati865/compiletest-rs.git`
[00:02:07] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:02:07] Build completed unsuccessfully in 0:00:00
[00:02:07] Makefile:70: recipe for target 'prepare' failed
[00:02:07] make: *** [prepare] Error 1
[00:02:09] Command failed. Attempt 3/5:
[00:02:09] Command failed. Attempt 3/5:
[00:02:09]     Updating git repository `https://github.com/mati865/compiletest-rs.git`
[00:02:09] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:02:09] Build completed unsuccessfully in 0:00:00
[00:02:09] make: *** [prepare] Error 1
[00:02:09] Makefile:70: recipe for target 'prepare' failed
[00:02:12] Command failed. Attempt 4/5:
[00:02:12] Command failed. Attempt 4/5:
[00:02:13]     Updating git repository `https://github.com/mati865/compiletest-rs.git`
[00:02:13] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:02:13] Build completed unsuccessfully in 0:00:00
[00:02:13] make: *** [prepare] Error 1
[00:02:13] Makefile:70: recipe for target 'prepare' failed
[00:02:17] Command failed. Attempt 5/5:
[00:02:17] Command failed. Attempt 5/5:
[00:02:17]     Updating git repository `https://github.com/mati865/compiletest-rs.git`
[00:02:18] error: the lock file /checkout/Cargo.lock needs to be updated but --locked was passed to prevent this
[00:02:18] Build completed unsuccessfully in 0:00:00
[00:02:18] Makefile:70: recipe for target 'prepare' failed
[00:02:18] The command has failed after 5 attempts.
[00:02:18] make: *** [prepare] Error 1
---
travis_time:end:0b4f6446:start=1551465900245205614,finish=1551465900250952207,duration=5746593
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:096fe925
$ 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 --batch -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:018046bf
travis_time:start:018046bf
$ 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:10d798c0
$ 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)

@rust-highfive
Copy link
Collaborator

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:0d34612f:start=1551466573845139466,finish=1551466576196769896,duration=2351630430
$ 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
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
[00:01:54] 
######################################################################## 100.0%
[00:01:54] extracting /checkout/obj/build/cache/2019-02-17/cargo-beta-x86_64-unknown-linux-gnu.tar.gz
[00:01:54]     Updating crates.io index
[00:02:07]     Updating git repository `https://github.com/mati865/compiletest-rs.git`
[00:02:08]   Downloaded num_cpus v1.8.0
[00:02:08]   Downloaded serde_json v1.0.33
[00:02:08]   Downloaded time v0.1.40
[00:02:08]   Downloaded filetime v0.2.4
---
tidy check
[00:04:04] * 569 error codes
[00:04:04] * highest error code: E0724
[00:04:05] * 252 features
[00:04:05] invalid source: "git+https://github.com/mati865/compiletest-rs.git?branch=rustup#2bf88c77e56a873a7d90dc5e2c8efeb0e2dc9d00"
[00:04:05] some tidy checks failed
[00:04:05] 
[00:04:05] 
[00:04:05] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-tools-bin/tidy" "/checkout/src" "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "--no-vendor" "--quiet"
[00:04:05] 
[00:04:05] 
[00:04:05] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap test src/tools/tidy
[00:04:05] Build completed unsuccessfully in 0:00:48
[00:04:05] Build completed unsuccessfully in 0:00:48
[00:04:05] make: *** [tidy] Error 1
[00:04:05] Makefile:68: recipe for target 'tidy' failed
The command "stamp sh -x -c "$RUN_SCRIPT"" exited with 2.
travis_time:start:091c49d2
$ date && (curl -fs --head https://google.com | grep ^Date: | sed 's/Date: //g' || true)
Fri Mar  1 19:00:32 UTC 2019
---
travis_time:end:014da2b4:start=1551466833753090548,finish=1551466833758668097,duration=5577549
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:08de0a2a
$ 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 --batch -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:1e1336da
travis_time:start:1e1336da
$ 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:0fac0a85
$ 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)

@ljedrz
Copy link
Contributor Author

ljedrz commented Mar 1, 2019

I guess the compiletest-rs crate needs to merge the fix branch first; other than that, the PR is ready for a review.

@ljedrz ljedrz mentioned this pull request Mar 2, 2019
@ljedrz
Copy link
Contributor Author

ljedrz commented Mar 2, 2019

Removing the clippy update for now due to the aforementioned compiletest-rs incompatibility and to make sure all the other tests pass.

I can append it again later if it gets merged and a proper version is released.

@Zoxc
Copy link
Contributor

Zoxc commented Mar 2, 2019

@bors r+

@bors
Copy link
Contributor

bors commented Mar 2, 2019

📌 Commit 299ed9a has been approved by Zoxc

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 2, 2019
@Zoxc
Copy link
Contributor

Zoxc commented Mar 2, 2019

Feel free to open further PRs. We can just combine them if bors is slow. Don't worry about breaking clippy, you can send PRs to fix it after the fact npw.

@Xanewok
Copy link
Member

Xanewok commented Mar 2, 2019

Out of curiosity, how much of the work is left wrt pruning NodeIds from HIR? (by the way @ljedrz thanks for all this work, I'm sure everyone appreciates it very much 😍 )

@ljedrz
Copy link
Contributor Author

ljedrz commented Mar 2, 2019

@Xanewok it's a bit hard to tell; I've already HirIdified the most crucial spots (intravisit and late lints) and converted most of the applicable objects, but there are a few trickier HIR nodes left. There are also some spots I need to clean up after all the affected objects have been converted.

Oh, and it will probably also be a good idea to later completely remove the NodeId methods in favor of the HirId ones; this will probably require rethinking some of the bigger HIR structs.

@bors
Copy link
Contributor

bors commented Mar 2, 2019

⌛ Testing commit 299ed9a with merge 0ea2271...

bors added a commit that referenced this pull request Mar 2, 2019
Remove NodeId from even more HIR nodes

The next iteration of HirIdification (#57578).

Removes `NodeId` from:

- [x] `StructField`
- [x] `ForeignItem`
- [x] `Item`
- [x] `Pat`
- [x] `FieldPat`
- [x] `VariantData`
- [x] `ImplItemId` (replaces it with `HirId`)
- [x] `TraitItemId` (replaces it with `HirId`)
@bors
Copy link
Contributor

bors commented Mar 2, 2019

☀️ Test successful - checks-travis, status-appveyor
Approved by: Zoxc
Pushing 0ea2271 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 2, 2019
@bors bors merged commit 299ed9a into rust-lang:master Mar 2, 2019
@ljedrz ljedrz deleted the begone_NodeId branch March 3, 2019 06:07
bors added a commit to rust-lang/rust-clippy that referenced this pull request Mar 3, 2019
HirIdification fixes

Supersedes #3828, enables rust-lang/rust#58836.

As usual, requesting a branch.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants