hir: replace "items" terminology with "nodes" where appropriate.#70092
hir: replace "items" terminology with "nodes" where appropriate.#70092bors merged 2 commits intorust-lang:masterfrom
Conversation
rust-highfive
added
the
S-waiting-on-review
src/librustc/query/mod.rs
Outdated
There was a problem hiding this comment.
These comments turned out to be inaccurate since some nodes have both a DefId and a HirId without being a HIR owner themselves. Maybe fix them?
There was a problem hiding this comment.
These comments turned out to be inaccurate since some nodes have both a
DefIdand aHirIdwithout being a HIR owner themselves. Maybe fix them?
Right, currently only a subset of DefIds are HIR owners. I'll take a second look at these comments to try and make that clear.
There was a problem hiding this comment.
Tried something, but whew that phrasing is hard to use.
I would be happier with hir::Owner (or I guess hir::OwnerKind) being a subset of Node and explicitly listing all of the supported node types.
Then it sort of self-documents what kinds of nodes are owners.
And I could also use it to add a visit_owner method to HIR visitors that would work with exhaustive matching.
But I didn't want to put any changes like that into this tiny PR.
There was a problem hiding this comment.
I think it's probably better to assume that the reader know what a HIR owner is here. I can barely make sense of the comments and I wrote the code =P
eddyb
force-pushed
the
hir-items-are-just-nodes
branch
from
907fba3 to
3177889
Compare
This comment has been minimized.
This comment has been minimized.
eddyb
force-pushed
the
hir-items-are-just-nodes
branch
from
3177889 to
262ee32
Compare
eddyb
force-pushed
the
hir-items-are-just-nodes
branch
from
262ee32 to
41ecc0f
Compare
|
cc @petrochenkov (since you've reviewed my recent changes around HIR) |
|
The PR looks good to me excluding the comments on the queries. |
eddyb
force-pushed
the
hir-items-are-just-nodes
branch
from
41ecc0f to
be9679d
Compare
|
@bors r+ |
|
📌 Commit be9679d has been approved by |
bors
added
S-waiting-on-bors
Rollup of 8 pull requests Successful merges: - rust-lang#67888 (Prefetch some queries used by the metadata encoder) - rust-lang#69934 (Update the mir inline costs) - rust-lang#69965 (Refactorings to get rid of rustc_codegen_utils) - rust-lang#70054 (Build dist-android with --enable-profiler) - rust-lang#70089 (rustc_infer: remove InferCtxt::closure_sig as the FnSig is always shallowly known.) - rust-lang#70092 (hir: replace "items" terminology with "nodes" where appropriate.) - rust-lang#70138 (do not 'return' in 'throw_' macros) - rust-lang#70151 (Update stdarch submodule) Failed merges: - rust-lang#70074 (Expand: nix all fatal errors) r? @ghost
4 participants
The newly added
HirOwnerItemsconfused me before I realized that "items" there actually referred to HIR nodes, nothir:Itemor "item-like" (which we should IMO replace with "owner").I suspect the naming had something to do with
ItemLocalId's use of "item".That is,
ItemLocalIdcould be interpreted to mean one of two things:IntraItemNodeIdi.e.IntraOwnerNodeIdIntraOwnerItemIdHirOwnerItemswould seem to implyr? @Zoxc cc @michaelwoerister @nikomatsakis