Crate metadata nondefid args #41780
Crate metadata nondefid args #41780
Conversation
wait, what? Trans definitely has access to a tcx. |
|
@cramertj can you give a more specific pointer to the part of trans you meant? |
|
@nikomatsakis Yeah, that's what I would've expected haha. But |
Mark-Simulacrum
added
the
S-waiting-on-review
|
@nikomatsakis and I just discussed this on IRC. In the LLVM passes there's no |
|
☔ The latest upstream changes (presumably #41709) made this pull request unmergeable. Please resolve the merge conflicts. |
| @@ -804,6 +845,10 @@ fn metadata_dep_node(def_id: DefId) -> DepNode<DefId> { | |||
| DepNode::MetaData(def_id) | |||
| } | |||
|
|
|||
| fn cratenum_metadata_dep_node(crate_num: CrateNum) -> DepNode<DefId> { | |||
| DepNode::MetaDataByCrateNum(crate_num) | |||
There was a problem hiding this comment.
You can just use MetaDataByCrateNum above, the functions are used when the variants don't match the key exactly.
There was a problem hiding this comment.
I don't think we should have DepNode with a cratenum in it, though, at least not yet, as it won't get remapped properly when being loaded back up.
There was a problem hiding this comment.
@nikomatsakis what would you suggest instead?
| @@ -786,6 +818,15 @@ define_maps! { <'tcx> | |||
| [] item_body_nested_bodies: metadata_dep_node(DefId) -> Rc<BTreeMap<hir::BodyId, hir::Body>>, | |||
| [] const_is_rvalue_promotable_to_static: metadata_dep_node(DefId) -> bool, | |||
| [] is_mir_available: metadata_dep_node(DefId) -> bool, | |||
|
|
|||
| [] is_const_fn: metadata_dep_node(DefId) -> bool, | |||
There was a problem hiding this comment.
There's something in MIR const qualif doing this check, so that could be fully replaced by this (right now users of this query have to take into account the fact that it only works cross-crate).
|
@cramertj I think the thing to do here is to query the information ahead of time and maybe even not have |
|
@eddyb What's the right way to do that so as not to mess up dependency tracking? Or do we even care about dependencies post-translation? |
|
@cramertj Post-translation should be fully deterministic in terms of dependencies. |
Mark-Simulacrum
added
S-waiting-on-author
|
ping @cramertj just wanted to keep this on your radar, do you think you'll have a chance to rebase? |
|
@alexcrichton This is based on top of another PR of mine that requires a test on Windows, and I haven't been able to get LLVM to build properly on my Windows machine. I'll close this for now and open a new PR when I have that sorted out. |
Builds on #41766
cc #41417
I started working on queryifying the crate metadata functions that take
CrateNumas an argument. However, a lot of these showed up in trans, which doesn't currently have access totcx. I plumbed it through, but after doing so I'm not sure how to set it up in driver, which makes me think this wasn't the right thing to do 😆 I'm imagining there's a reason that trans relies uponSessioninstead ofTyCtxt. If that's the case, how should I proceed?r? @nikomatsakis