Queryify more metadata #41766
Queryify more metadata #41766
Conversation
|
Thanks @cramertj ! We'll get @nikomatsakis to give this a thorough review shortly! |
shepmaster
added
the
S-waiting-on-review
src/librustc_metadata/cstore_impl.rs
Outdated
| @@ -128,6 +128,23 @@ provide! { <'tcx> tcx, def_id, cdata | |||
| !cdata.is_proc_macro(def_id.index) && | |||
| cdata.maybe_entry(def_id.index).and_then(|item| item.decode(cdata).mir).is_some() | |||
| } | |||
| is_const_fn => { | |||
| let constness = match cdata.entry(def_id.index).kind { | |||
There was a problem hiding this comment.
@cramertj we had a discussion with @nikomatsakis about leaving any implementations that are longer than single liners as part of CrateMetadata and just calling those methods on cdata here. This would keep the macro calls clean and simple.
There was a problem hiding this comment.
Okay! I'll change that. Thanks for the heads-up.
src/librustc/ty/mod.rs
Outdated
| /// If the given def ID describes an item belonging to a trait, | ||
| /// return the ID of the trait that the trait item belongs to. | ||
| /// Otherwise, return `None`. | ||
| fn trait_of_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Option<DefId> { |
There was a problem hiding this comment.
I believe your error is coming from here. I actually implemented this also... It has to do with you not handling the case where it's not part of local crate. See https://github.com/rust-lang/rust/pull/41724/files#diff-c888afeccd78f53e5ea3eb04089e46ddR2683
You can feel free to fix this or just remove this implementation.
There was a problem hiding this comment.
If you already did it I'll just remove it from this PR. We should probably come up with a better system for tracking this.
There was a problem hiding this comment.
Well there are about 38 more conversions left. How about I just take the first half and you can take the rest? I will start where you left off and take it up until fn derive_registrar_fn(&self, cnum: CrateNum) -> Option<DefId>;, and you can start form there. If one of us finishes first and wants to continue we can just touch base on the original issue.
There was a problem hiding this comment.
I'm not sure exactly how many there really are left, though, because I skipped a bunch that were used in librustc_resolve. Also, we still have to figure out what we're doing about the ones that take CrateNum as an arg-- I was planning to make a separate macro or adapt the existing one to support those.
There was a problem hiding this comment.
I think we can still make CrateNum work. There are a few queries using it now. Like:
There was a problem hiding this comment.
Yes, it uses it as a map, but it's not provided by the provide macro in cstore_impl, which assumes that the arguments are DefIds..
cramertj
force-pushed
the
more-crate-metadata
branch
2 times, most recently
from
2e81139 to
e349452
Compare
There was a problem hiding this comment.
ok, so, @achernyak already pointed out the conflict with the other PR, and the idea of keeping macro simple, but other than that r=me
|
@nikomatsakis I think I already addressed those comments before you made your comment. Are there still changes you'd like to see? |
|
@bors r+ |
|
📌 Commit e349452 has been approved by |
|
@bors r- this hit an error in windows https://ci.appveyor.com/project/rust-lang/rust/build/1.0.3191/job/3chceujkqmrufpe9 |
arielb1
added
S-waiting-on-author
|
☔ The latest upstream changes (presumably #41709) made this pull request unmergeable. Please resolve the merge conflicts. |
cramertj
force-pushed
the
more-crate-metadata
branch
from
e349452 to
7382526
Compare
|
☔ The latest upstream changes (presumably #41905) made this pull request unmergeable. Please resolve the merge conflicts. |
|
@cramertj Looks like there are still some merge conflicts to resolve. |
|
Closing this to keep the PR list clear. I'll open a new PR once I've had a chance to test on windows. |
cc #41417
r? @nikomatsakis