More Queries for Crate Metadata by hackeryarn · Pull Request #41724 · rust-lang/rust

hackeryarn · 2017-05-03T14:05:35Z

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

aidanhs · 2017-05-03T19:48:30Z

Thanks for the PR @achernyak! We'll make sure @nikomatsakis or another reviewer gets to this soon.

nikomatsakis · 2017-05-05T19:03:46Z

@bors r+

bors · 2017-05-05T19:03:47Z

📌 Commit 5a7946d has been approved by nikomatsakis

nikomatsakis · 2017-05-05T19:06:10Z

+/// 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


Er, wait, this seems surprising. What is going on here exactly?

I think I expected this code to only run when the krate isLOCAL_CRATE, no?

Isn't that what this is doing? This returns None if it's not a LOCAL_CRATE.

nikomatsakis · 2017-05-05T19:06:56Z

 // 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) }


...and I expected this code to run for other crates.

nikomatsakis · 2017-05-05T19:07:09Z

@bors r-

hackeryarn · 2017-05-05T19:41:30Z

@nikomatsakis I think I covered what you were asking for.

hackeryarn · 2017-05-08T02:10:14Z

@cramertj This PR should now cover all the simple cases for DefId based queries. I've skipped any queries that would require providing tcx access to functions that currently don't have it, and so on.

nikomatsakis · 2017-05-08T18:05:43Z

+/// 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)


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.

hackeryarn · 2017-05-09T00:02:01Z

@nikomatsakis you were absolutely right. It can be removed!

bors · 2017-05-09T14:35:29Z

☔ The latest upstream changes (presumably #41709) made this pull request unmergeable. Please resolve the merge conflicts.

nikomatsakis · 2017-05-10T21:35:06Z

@bors r+

bors · 2017-05-10T21:35:07Z

📌 Commit 35812d1 has been approved by nikomatsakis

@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

@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

Rollup of 5 pull requests - Successful merges: #41192, #41724, #41873, #41877, #41889 - Failed merges:

hackeryarn added 3 commits May 3, 2017 08:17

all queries use dep nodes
4d7c0b6

item_attrs
a12a55f

fn_arg_names
c72a16b

rust-highfive assigned nikomatsakis May 3, 2017

aidanhs added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 3, 2017

hackeryarn added 3 commits May 4, 2017 08:27

trait_of_item
aa5a532

impl_parent
03fe10d

moved metadata provider impls to decoder
5b71d76

hackeryarn force-pushed the master branch from 5e9b473 to 5b71d76Compare May 4, 2017 17:46

is_foreign_item
5a7946d

nikomatsakis mentioned this pull request May 5, 2017
incr.comp.: Crate Metadata not corresponding to a DefId is untracked. #41417
Closed

nikomatsakis approved these changes May 5, 2017
View reviewed changes

nikomatsakis requested changes May 5, 2017
View reviewed changes

propper trait of item impl
d561d4c

is_exported_symbol
1f532bf

nikomatsakis requested changes May 8, 2017
View reviewed changes

removed unnecessary if
dfb740f

hackeryarn force-pushed the master branch 5 times, most recently from 6b3900a to 2486a8eCompare May 9, 2017 17:55

hackeryarn force-pushed the master branch 3 times, most recently from 41f03e4 to a468875Compare May 9, 2017 19:40

resolved merge conflicts
35812d1

hackeryarn force-pushed the master branch from a468875 to 35812d1Compare May 9, 2017 22:24

nikomatsakis approved these changes May 10, 2017
View reviewed changes

frewsxcv mentioned this pull request May 11, 2017
Rollup of 10 pull requests #41901
Closed

frewsxcv mentioned this pull request May 11, 2017
Rollup of 5 pull requests #41905
Merged

bors added a commit that referenced this pull request May 11, 2017
Auto merge of#41905- frewsxcv:rollup, r=frewsxcv
24ea08e
Rollup of 5 pull requests - Successful merges: #41192, #41724, #41873, #41877, #41889 - Failed merges:

bors merged commit 35812d1 into rust-lang:masterMay 11, 2017

Uh oh!

Uh oh!

Conversation

hackeryarn commented May 3, 2017• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aidanhs commented May 3, 2017

Uh oh!

nikomatsakis commented May 5, 2017

Uh oh!

bors commented May 5, 2017

Uh oh!

nikomatsakisMay 5, 2017

Choose a reason for hiding this comment

Uh oh!

nikomatsakisMay 5, 2017

Choose a reason for hiding this comment

Uh oh!

hackeryarnMay 5, 2017

Choose a reason for hiding this comment

Uh oh!

nikomatsakisMay 5, 2017

Choose a reason for hiding this comment

Uh oh!

nikomatsakis commented May 5, 2017

Uh oh!

hackeryarn commented May 5, 2017

Uh oh!

hackeryarn commented May 8, 2017

Uh oh!

nikomatsakisMay 8, 2017

Choose a reason for hiding this comment

Uh oh!

hackeryarn commented May 9, 2017

Uh oh!

bors commented May 9, 2017

Uh oh!

nikomatsakis commented May 10, 2017

Uh oh!

bors commented May 10, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

hackeryarn commented May 3, 2017•
edited
Loading