Queries for Crate Metadata #41653
Queries for Crate Metadata #41653
Conversation
| cstore.item_generics_cloned(def_id).types.into_iter().map(|def| { | ||
| def.object_lifetime_default | ||
| }).collect() | ||
| ty::tls::with_opt(|opt_tcx| { |
There was a problem hiding this comment.
@nikomatsakis it seems that this call isn't able to retrieve tcx properly in a few scenarios. Is there a more appropriate way to reach tcx from this location?
There was a problem hiding this comment.
Hmm, yes, the reason you are having this problem is that the tcx is not constructed yet. In general, fetching the tcx out of TLS is frowned upon, except for very limited cases (e.g., in a Debug impl) where there is basically no way to thread it down manually.
For now, you might want to leave those methods that are in code without access to a tcx -- but really the right answer is to convert these computations into "on-demand computations" themselves (e.g., resolve-lifetimes probably wants to become a query).
There was a problem hiding this comment.
The reason item_generics_cloned is named the way it is (I should've commented it, shouldn't I?) is because it's item_generics (what is now generics_of), but returning a clone of the data that does not require a TyCtxt.
|
@nikomatsakis thanks for explanation. I've removed the commits that were causing the issues. This should be ready for review now. I've also started a comment on the original issue to keep track of all the conversions that needed to be skipped at the time. #41417 (comment) |
|
☔ The latest upstream changes (presumably #41611) made this pull request unmergeable. Please resolve the merge conflicts. |
src/librustc/ty/maps.rs
Outdated
| @@ -599,7 +611,8 @@ define_maps! { <'tcx> | |||
|
|
|||
| [] describe_def: DescribeDef(DefId) -> Option<Def>, | |||
| [] def_span: DefSpan(DefId) -> Span, | |||
|
|
|||
| [] stability: Stability(DefId) -> Option<attr::Stability>, | |||
| [] deprecation: Deprecation(DefId) -> Option<attr::Deprecation> | |||
There was a problem hiding this comment.
@nikomatsakis how do you prefer this to be implemented. I noticed that @cramertj and I diverged here. @cramertj is using the metadata_dep_node function while I am declaring new DepNode methods for these, as suggested by @eddyb in #41593.
I just want alignment here since we still have majority of the queries to convert.
| @@ -115,6 +115,8 @@ provide! { <'tcx> tcx, def_id, cdata | |||
| is_foreign_item => { cdata.is_foreign_item(def_id.index) } | |||
| describe_def => { cdata.get_def(def_id.index) } | |||
| def_span => { cdata.get_span(def_id.index, &tcx.sess) } | |||
| stability => { cdata.get_stability(def_id.index) } | |||
| deprecation => { cdata.get_deprecation(def_id.index) } | |||
There was a problem hiding this comment.
@nikomatsakis this is another place of divergence with @cramertj. Should we be referencing the methods on cdata, or moving them into this implementation?
There was a problem hiding this comment.
it probably depends on how big the method is. I think I'd rather keep the methods if they are more than a line or two, so that the macro remains only short expressions.
|
@achernyak You'll want to use |
2fb07a4 to
120fe3f
Compare
|
@eddyb I've removed the merge commits. |
|
@bors r+ |
|
📌 Commit c1d97c7 has been approved by |
Queries for Crate Metadata This resolves following parts of rust-lang#41417: * `fn stability(&self, def: DefId) -> Option<attr::Stability>;` * `fn deprecation(&self, def: DefId) -> Option<attr::Deprecation>;` r? @nikomatsakis
This resolves following parts of #41417:
fn stability(&self, def: DefId) -> Option<attr::Stability>;fn deprecation(&self, def: DefId) -> Option<attr::Deprecation>;r? @nikomatsakis