-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More Queries for Crate Metadata #41724
Conversation
Thanks for the PR @achernyak! We'll make sure @nikomatsakis or another reviewer gets to this soon. |
@bors r+ |
📌 Commit 5a7946d has been approved by |
src/librustc/ty/mod.rs
Outdated
/// Otherwise, return `None`. | ||
fn trait_of_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Option<DefId> { | ||
if def_id.krate != LOCAL_CRATE { | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Er, wait, this seems surprising. What is going on here exactly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I expected this code to only run when the krate is LOCAL_CRATE
, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't that what this is doing? This returns None
if it's not a LOCAL_CRATE
.
src/librustc_metadata/cstore_impl.rs
Outdated
@@ -119,6 +119,7 @@ provide! { <'tcx> tcx, def_id, cdata | |||
// This is only used by rustdoc anyway, which shouldn't have | |||
// incremental recompilation ever enabled. | |||
fn_arg_names => { cdata.get_fn_arg_names(def_id.index) } | |||
trait_of_item => { cdata.get_trait_of_item(def_id.index) } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...and I expected this code to run for other crates.
@bors r- |
@nikomatsakis I think I covered what you were asking for. |
@cramertj This PR should now cover all the simple cases for |
src/librustc/ty/mod.rs
Outdated
/// Otherwise, return `None`. | ||
fn trait_of_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Option<DefId> { | ||
if def_id.krate != LOCAL_CRATE { | ||
return tcx.trait_of_item(def_id) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am pretty sure that this if
can be removed -- or else there is a bug. After all, tcx.trait_of_item(def_id)
should just call back into this code, since this is a query implementation.
@nikomatsakis you were absolutely right. It can be removed! |
☔ The latest upstream changes (presumably #41709) made this pull request unmergeable. Please resolve the merge conflicts. |
6b3900a
to
2486a8e
Compare
41f03e4
to
a468875
Compare
@bors r+ |
📌 Commit 35812d1 has been approved by |
More Queries for Crate Metadata This covers a little bit of clean up and the following parts of rust-lang#41417: * `fn item_attrs(&self, def_id: DefId) -> Vec<ast::Attribute>;` * `fn fn_arg_names(&self, did: DefId) -> Vec<ast::Name>;` * `fn trait_of_item(&self, def_id: DefId) -> Option<DefId>;` * `fn impl_parent(&self, impl_def_id: DefId) -> Option<DefId>;` * ` fn is_foreign_item(&self, did: DefId) -> bool;` * `fn is_exported_symbol(&self, def_id: DefId) -> bool;` r? @nikomatsakis
More Queries for Crate Metadata This covers a little bit of clean up and the following parts of rust-lang#41417: * `fn item_attrs(&self, def_id: DefId) -> Vec<ast::Attribute>;` * `fn fn_arg_names(&self, did: DefId) -> Vec<ast::Name>;` * `fn trait_of_item(&self, def_id: DefId) -> Option<DefId>;` * `fn impl_parent(&self, impl_def_id: DefId) -> Option<DefId>;` * ` fn is_foreign_item(&self, did: DefId) -> bool;` * `fn is_exported_symbol(&self, def_id: DefId) -> bool;` r? @nikomatsakis
This covers a little bit of clean up and the following parts of #41417:
fn item_attrs(&self, def_id: DefId) -> Vec<ast::Attribute>;
fn fn_arg_names(&self, did: DefId) -> Vec<ast::Name>;
fn trait_of_item(&self, def_id: DefId) -> Option<DefId>;
fn impl_parent(&self, impl_def_id: DefId) -> Option<DefId>;
fn is_foreign_item(&self, did: DefId) -> bool;
fn is_exported_symbol(&self, def_id: DefId) -> bool;
r? @nikomatsakis