process trait/impl items directly from the visitor callback #39000
Conversation
|
LGTM. I agree with |
|
Not as an argument against eliminating |
|
The whole closure situation is a bit troublesome. It'd be better to actually compute (copy, really) that information from the surrounding function's |
| whether the node is inlined or not and do the right thing. | ||
| whether the node is inlined or not and do the right thing. You can | ||
| also use `tcx.map.is_inlined_def_id()` and | ||
| `tcx.map.is_inlined_node_id()` to test. |
There was a problem hiding this comment.
None of this happens anymore, inlined bodies don't get their own local NodeIds or DefIds.
|
Looks good to me! One nit: "Tables" is a rather generic name for the dep-node that says little about what it represents. If you can think of a more descriptive name, now would be the time to change it, I guess. |
|
|
The current setup processes impl/trait items while visiting
the impl/trait. This means we basically have this setup:
<Lots> -> TypeckItemBody(Impl) -> Tables(ImplItem{0,1,2,3})
But this was largely an artifact of the older code. By moving the
processing of items into method dedicated for their use, we produce this
setup:
<Little> -> TypeckItemBody(ImplItem0) -> Tables(ImplItem0)
...
<Little> -> TypeckItemBody(ImplItem3) -> Tables(ImplItem3)
nikomatsakis
force-pushed
the
incr_comp_crosscontaminate_impl_item
branch
from
2595ae8 to
282f7a3
Compare
|
@bors r+ |
|
📌 Commit 282f7a3 has been approved by |
|
⌛ Testing commit 282f7a3 with merge 2f0463a... |
…tem, r=michaelwoerister
process trait/impl items directly from the visitor callback
The current setup processes impl/trait items while visiting
the impl/trait. This means we basically have this setup:
<Lots> -> TypeckItemBody(Impl) -> Tables(ImplItem{0,1,2,3})
But this was largely an artifact of the older code. By moving the
processing of items into method dedicated for their use, we produce this
setup:
<Little> -> TypeckItemBody(ImplItem0) -> Tables(ImplItem0)
...
<Little> -> TypeckItemBody(ImplItem3) -> Tables(ImplItem3)
r? @michaelwoerister
Also, we might consider removing the `TypeckItemBody` node altogether and just using `Tables` as the task. `Tables` is its primary output, I imagine? That would reduce size of dep-graph somewhat.
cc @eddyb -- perhaps this pattern applies elsewhere?
|
☀️ Test successful - status-appveyor, status-travis |
The current setup processes impl/trait items while visiting
the impl/trait. This means we basically have this setup:
But this was largely an artifact of the older code. By moving the
processing of items into method dedicated for their use, we produce this
setup:
r? @michaelwoerister
Also, we might consider removing the
TypeckItemBodynode altogether and just usingTablesas the task.Tablesis its primary output, I imagine? That would reduce size of dep-graph somewhat.cc @eddyb -- perhaps this pattern applies elsewhere?