Restructure hir::map::Node and hir::map::Entry #53616

varkor · 2018-08-22T21:26:04Z

Moves hir::map::Node to hir::Node and removes the Node* prefix from its variants.
Changes hir::map::Entry to a struct hir::map::Entry.
Removes the Node* prefix from each of AnnNodes variants.

bors · 2018-08-24T21:49:58Z

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

eddyb · 2018-08-25T05:03:22Z

Entry being an enum is a dead optimization FWIW (because of DepNodeIndex), what you want is this:

struct Entry {
    parent: NodeId,
    dep_node: DepNodeIndex,
    node: Node,
}

Add RootCrate to Node as just Crate, then NotPresent is replaced by having Option<Entry> (which nowadays should be the same size as Entry).

Also, since Node isn't a struct, you can just have e.g. Node::Expr which is IMO nicer.
Oh and you can move Node to hir, keep only Entry in hir::map.

varkor · 2018-08-25T18:41:51Z

All done!

eddyb · 2018-08-25T21:36:25Z

src/librustc/hir/map/collector.rs

@@ -114,7 +114,11 @@ impl<'a, 'hir> NodeCollector<'a, 'hir> {
            hcx,
            hir_body_nodes,
        };
-        collector.insert_entry(CRATE_NODE_ID, RootCrate(root_mod_sig_dep_index));
+        collector.insert_entry(CRATE_NODE_ID, Entry {
+            parent: ast::DUMMY_NODE_ID,


I think you actually want CRATE_NODE_ID here.

eddyb · 2018-08-25T21:37:35Z

src/librustc/hir/map/mod.rs

-            RootCrate(_) => return None
-        })
+        match self.node {
+            Node::Crate => None,


I don't think this is useful.

eddyb · 2018-08-25T21:39:03Z

src/librustc/hir/mod.rs

+    Visibility(&'hir Visibility),
+
+    /// Roots for node trees. Its DepNodeIndex when in `Entry`
+    /// is the dependency node of the crate's root module.


I don't think this is super relevant. Also, we could technically make this point to data as well, it'd just not be cleanly separated yet, so we can't do it (would break incremental).

eddyb · 2018-08-25T21:39:54Z

LGTM! r? @nikomatsakis or @michaelwoerister

rust-highfive · 2018-08-25T22:53:02Z

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.

151200 ./src/tools/clang
149112 ./src/llvm-emscripten/test
148696 ./obj/build/bootstrap/debug/incremental
134264 ./obj/build/bootstrap/debug/incremental/bootstrap-1fo02lb1azu6i
134260 ./obj/build/bootstrap/debug/incremental/bootstrap-1fo02lb1azu6i/s-f471wmgryq-spukfx-heneaay3bnr5
103868 ./src/tools/lldb
98948 ./obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/codegen-backends
93756 ./src/tools/clang/test
90416 ./obj/build/x86_64-unknown-linux-gnu/stage1

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)

varkor · 2018-08-25T23:13:27Z

@eddyb: well, it didn't like those changes. I guess excluding the crate root was important.

rust-highfive · 2018-08-25T23:44:21Z

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:20:37]    Compiling rustc_asan v0.0.0 (file:///checkout/src/librustc_asan)
[00:20:37]    Compiling rustc_tsan v0.0.0 (file:///checkout/src/librustc_tsan)
[00:20:38]    Compiling rustc_lsan v0.0.0 (file:///checkout/src/librustc_lsan)
[00:20:38]    Compiling rustc_msan v0.0.0 (file:///checkout/src/librustc_msan)
[00:21:03] error: internal compiler error: librustc/middle/reachable.rs:324: found unexpected thingy in worklist: root_crate
[00:21:03] thread 'main' panicked at 'Box<Any>', librustc_errors/lib.rs:586:9
[00:21:03] note: Run with `RUST_BACKTRACE=1` for a backtrace.
inux-gnu
285392 ./src/tools
---
151200 ./src/tools/clang
149116 ./src/llvm-emscripten/test
148696 ./obj/build/bootstrap/debug/incremental
134264 ./obj/build/bootstrap/debug/incremental/bootstrap-1fo02lb1azu6i
134260 ./obj/build/bootstrap/debug/incremental/bootstrap-1fo02lb1azu6i/s-f473byonm0-ymn7ph-heneaay3bnr5
103868 ./src/tools/lldb
98952 ./obj/build/x86_64-unknown-linux-gnu/stage0/lib/rustlib/x86_64-unknown-linux-gnu/codegen-backends
93756 ./src/tools/clang/test
90416 ./obj/build/x86_64-unknown-linux-gnu/stage1

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)

eddyb · 2018-08-26T06:03:12Z

@varkor That just means there are bugs lurking around and they warrant further investigation.

bors · 2018-08-27T04:16:35Z

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

nikomatsakis · 2018-08-27T19:52:38Z

@bors r+

bors · 2018-08-27T19:52:38Z

📌 Commit 55c34e5c6e0a4f18e89828f6cf8e1581b8d8ed2e has been approved by nikomatsakis

nikomatsakis · 2018-08-27T19:52:43Z

@bors r-

nikomatsakis · 2018-08-27T19:52:48Z

r=me post rebase =)

nikomatsakis · 2018-08-27T19:52:03Z

src/librustc/hir/map/collector.rs

@@ -29,7 +29,7 @@ pub(super) struct NodeCollector<'a, 'hir> {
    /// The crate
    krate: &'hir Crate,
    /// The node map
-    map: Vec<EntryKind<'hir>>,
+    map: Vec<Option<Entry<'hir>>>,


varkor · 2018-08-27T20:51:06Z

@bors r=nikomatsakis

bors · 2018-08-27T20:51:07Z

📌 Commit a9d075e has been approved by nikomatsakis

bors · 2018-08-28T06:44:20Z

⌛ Testing commit a9d075e with merge 59e52b1...

@eddyb

Restructure hir::map::Node and hir::map::Entry - Moves `hir::map::Node` to `hir::Node` and removes the `Node*` prefix from its variants. - Changes `hir::map::Entry` to a struct `hir::map::Entry`. - Removes the `Node*` prefix from each of `AnnNode`s variants. r? @eddyb

bors · 2018-08-28T08:55:20Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing 59e52b1 to master...

rust-highfive · 2018-08-28T08:57:00Z

📣 Toolstate changed by #53616!

Tested on commit 59e52b1.
Direct link to PR: #53616

💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra).
💔 rls on windows: test-pass → build-fail (cc @nrc, @rust-lang/infra).
💔 rls on linux: test-pass → build-fail (cc @nrc, @rust-lang/infra).

@Manishearth

Tested on commit rust-lang/rust@59e52b1. Direct link to PR: <rust-lang/rust#53616> 💔 clippy-driver on windows: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). 💔 clippy-driver on linux: test-pass → build-fail (cc @Manishearth @llogiq @mcarton @oli-obk, @rust-lang/infra). 💔 rls on windows: test-pass → build-fail (cc @nrc, @rust-lang/infra). 💔 rls on linux: test-pass → build-fail (cc @nrc, @rust-lang/infra).

oli-obk · 2018-08-28T08:59:39Z

on it wrt clippy failure

rust-highfive assigned eddyb Aug 22, 2018

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

This comment has been minimized.

Sign in to view

varkor force-pushed the hir-map-rename branch from a5467b2 to ed7bb39 Compare August 25, 2018 13:40

varkor changed the title ~~Rename hir::map::Node and hir::map::Entry~~ Restructure hir::map::Node and hir::map::Entry Aug 25, 2018

This comment has been minimized.

Sign in to view

varkor force-pushed the hir-map-rename branch from 1048f81 to 5350667 Compare August 25, 2018 16:26

This comment has been minimized.

Sign in to view

varkor force-pushed the hir-map-rename branch from 5350667 to 67517f1 Compare August 25, 2018 18:41

eddyb reviewed Aug 25, 2018

View reviewed changes

rust-highfive assigned nikomatsakis and unassigned eddyb Aug 25, 2018

This comment has been minimized.

Sign in to view

varkor force-pushed the hir-map-rename branch from 18e503b to dc409c1 Compare August 25, 2018 22:29

varkor force-pushed the hir-map-rename branch from 352b852 to 55c34e5 Compare August 26, 2018 10:44

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 Aug 27, 2018

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 27, 2018

nikomatsakis approved these changes Aug 27, 2018

View reviewed changes

varkor added 8 commits August 27, 2018 21:45

Rename MapEntry to EntryKind

a1a338f

Rename hir::map::Node to hir::map::NodeKind

befc4b1

Remove Node* prefix from AnnNode

4b12f70

Remove path prefixes from NodeKind

11665ca

Convert EntryKind to a struct, Entry

ecbdfb4

Rename hir::map::NodeKind to hir::Node

e2a1cce

Make small modifications

6642462

Revert crate root changes

a9d075e

varkor force-pushed the hir-map-rename branch from 55c34e5 to a9d075e Compare August 27, 2018 20:50

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 27, 2018

bors merged commit a9d075e into rust-lang:master Aug 28, 2018

This was referenced Aug 28, 2018

Implement the min_const_fn feature gate #53604

Merged

Make librustc_codegen_llvm aware of LLVM address spaces. #51576

Closed

#53576 Renaming TyAnon -> TyOpaque #53705

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Restructure hir::map::Node and hir::map::Entry #53616

Restructure hir::map::Node and hir::map::Entry #53616

varkor commented Aug 22, 2018 •

edited

Loading

This comment has been minimized.

bors commented Aug 24, 2018

eddyb commented Aug 25, 2018

This comment has been minimized.

This comment has been minimized.

varkor commented Aug 25, 2018

eddyb Aug 25, 2018

eddyb Aug 25, 2018

eddyb Aug 25, 2018 •

edited

Loading

eddyb commented Aug 25, 2018

This comment has been minimized.

rust-highfive commented Aug 25, 2018

varkor commented Aug 25, 2018

rust-highfive commented Aug 25, 2018

eddyb commented Aug 26, 2018

bors commented Aug 27, 2018

nikomatsakis commented Aug 27, 2018

bors commented Aug 27, 2018

nikomatsakis commented Aug 27, 2018

nikomatsakis commented Aug 27, 2018

nikomatsakis Aug 27, 2018

varkor commented Aug 27, 2018

bors commented Aug 27, 2018

bors commented Aug 28, 2018

bors commented Aug 28, 2018

rust-highfive commented Aug 28, 2018

oli-obk commented Aug 28, 2018

Restructure hir::map::Node and hir::map::Entry #53616

Restructure hir::map::Node and hir::map::Entry #53616

Conversation

varkor commented Aug 22, 2018 • edited Loading

This comment has been minimized.

bors commented Aug 24, 2018

eddyb commented Aug 25, 2018

This comment has been minimized.

This comment has been minimized.

varkor commented Aug 25, 2018

eddyb Aug 25, 2018

Choose a reason for hiding this comment

eddyb Aug 25, 2018

Choose a reason for hiding this comment

eddyb Aug 25, 2018 • edited Loading

Choose a reason for hiding this comment

eddyb commented Aug 25, 2018

This comment has been minimized.

rust-highfive commented Aug 25, 2018

varkor commented Aug 25, 2018

rust-highfive commented Aug 25, 2018

eddyb commented Aug 26, 2018

bors commented Aug 27, 2018

nikomatsakis commented Aug 27, 2018

bors commented Aug 27, 2018

nikomatsakis commented Aug 27, 2018

nikomatsakis commented Aug 27, 2018

nikomatsakis Aug 27, 2018

Choose a reason for hiding this comment

varkor commented Aug 27, 2018

bors commented Aug 27, 2018

bors commented Aug 28, 2018

bors commented Aug 28, 2018

rust-highfive commented Aug 28, 2018

oli-obk commented Aug 28, 2018

varkor commented Aug 22, 2018 •

edited

Loading

eddyb Aug 25, 2018 •

edited

Loading