Uh oh!
There was an error while loading. Please reload this page.
Crate metadata nondefid args - #41780
Conversation
nikomatsakis
commented
May 6, 2017
wait, what? Trans definitely has access to a tcx. |
nikomatsakis
commented
May 6, 2017
@cramertj can you give a more specific pointer to the part of trans you meant? |
cramertj
commented
May 6, 2017
@nikomatsakis Yeah, that's what I would've expected haha. But |
cramertj
commented
May 8, 2017
@nikomatsakis and I just discussed this on IRC. In the LLVM passes there's no |
bors
commented
May 9, 2017
☔ The latest upstream changes (presumably #41709) made this pull request unmergeable. Please resolve the merge conflicts. |
| } | ||
| 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.
| [] 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).
eddyb
commented
May 10, 2017
@cramertj I think the thing to do here is to query the information ahead of time and maybe even not have |
cramertj
commented
May 10, 2017
@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? |
eddyb
commented
May 11, 2017
@cramertj Post-translation should be fully deterministic in terms of dependencies. |
alexcrichton
commented
May 18, 2017
ping @cramertj just wanted to keep this on your radar, do you think you'll have a chance to rebase? |
cramertj
commented
May 18, 2017
@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