Skip to content

query for def_span #41593

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Apr 30, 2017
Merged

query for def_span #41593

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
932d251
query for def_span
hackeryarn Apr 28, 2017
93ac5df
review updateds
hackeryarn Apr 28, 2017
ba90718
found the stack overflow culprit
hackeryarn Apr 29, 2017
2f73b17
Merge branch 'master' into def_span
hackeryarn Apr 29, 2017
ad1959e
added dep nodes and comment
hackeryarn Apr 29, 2017
00d02cf
chaned dep nodes impl
hackeryarn Apr 29, 2017
d3b7af0
removed custom functions and comment
hackeryarn Apr 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions src/librustc/middle/cstore.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ pub trait CrateStore {
fn crate_data_as_rc_any(&self, krate: CrateNum) -> Rc<Any>;

// item info
fn def_span(&self, sess: &Session, def: DefId) -> Span;
fn stability(&self, def: DefId) -> Option<attr::Stability>;
fn deprecation(&self, def: DefId) -> Option<attr::Deprecation>;
fn visibility(&self, def: DefId) -> ty::Visibility;
Expand Down Expand Up @@ -312,7 +311,6 @@ impl CrateStore for DummyCrateStore {
fn crate_data_as_rc_any(&self, krate: CrateNum) -> Rc<Any>
{ bug!("crate_data_as_rc_any") }
// item info
fn def_span(&self, sess: &Session, def: DefId) -> Span { bug!("def_span") }
fn stability(&self, def: DefId) -> Option<attr::Stability> { bug!("stability") }
fn deprecation(&self, def: DefId) -> Option<attr::Deprecation> { bug!("deprecation") }
fn visibility(&self, def: DefId) -> ty::Visibility { bug!("visibility") }
Expand Down
9 changes: 8 additions & 1 deletion src/librustc/ty/maps.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,12 @@ impl<'tcx> QueryDescription for queries::describe_def<'tcx> {
}
}

impl<'tcx> QueryDescription for queries::def_span<'tcx> {
fn describe(_: TyCtxt, _: DefId) -> String {
bug!("def_span")
}
}

macro_rules! define_maps {
(<$tcx:tt>
$($(#[$attr:meta])*
Expand Down Expand Up @@ -547,7 +553,8 @@ define_maps! { <'tcx>
pub def_symbol_name: SymbolName(DefId) -> ty::SymbolName,
pub symbol_name: symbol_name_dep_node(ty::Instance<'tcx>) -> ty::SymbolName,

pub describe_def: meta_data_node(DefId) -> Option<Def>
pub describe_def: meta_data_node(DefId) -> Option<Def>,
pub def_span: meta_data_node(DefId) -> Span
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, what is meta_data_node? First off, it doesn't need a function, it could just be MetaData.
Secondly, if you have a local crate implementation, you kind of need a node that's not MetaData.
But if @nikomatsakis doesn't think this impacts incremental this can be left as-is for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was actually added as part of my last PR. I will remove it form both instances and replace it with just MetaData.

}

fn coherent_trait_dep_node((_, def_id): (CrateNum, DefId)) -> DepNode<DefId> {
Expand Down
19 changes: 11 additions & 8 deletions src/librustc/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2265,14 +2265,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
}
}

pub fn def_span(self, def_id: DefId) -> Span {
if let Some(id) = self.hir.as_local_node_id(def_id) {
self.hir.span(id)
} else {
self.sess.cstore.def_span(&self.sess, def_id)
}
}

pub fn vis_is_accessible_from(self, vis: Visibility, block: NodeId) -> bool {
vis.is_accessible_from(self.hir.local_def_id(self.hir.get_module_parent(block)), self)
}
Expand Down Expand Up @@ -2675,12 +2667,23 @@ fn associated_item_def_ids<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
Rc::new(vec)
}

fn def_span<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) -> Span {
match tcx.hir.span_if_local(def_id) {
Some(span) => span,
None => {
let node_id = tcx.sess.cstore.item_body(tcx, def_id).id().node_id;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised you found item_body - that's really only meant to be used by const evaluation.
You should really be panicking if the DefId isn't local (i.e. you can .unwrap() the result of span_if_local instead of handling the None case) - because the line you added in src/librustc_metadata/cstore_impl.rs is what actually handles that case.

tcx.hir.span(node_id)
},
}
}

pub fn provide(providers: &mut ty::maps::Providers) {
*providers = ty::maps::Providers {
associated_item,
associated_item_def_ids,
adt_sized_constraint,
adt_dtorck_constraint,
def_span,
..*providers
};
}
Expand Down
6 changes: 1 addition & 5 deletions src/librustc_metadata/cstore_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,18 +114,14 @@ provide! { <'tcx> tcx, def_id, cdata
inherent_impls => { Rc::new(cdata.get_inherent_implementations_for_type(def_id.index)) }
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) }
}

impl CrateStore for cstore::CStore {
fn crate_data_as_rc_any(&self, krate: CrateNum) -> Rc<Any> {
self.get_crate_data(krate)
}

fn def_span(&self, sess: &Session, def: DefId) -> Span {
self.dep_graph.read(DepNode::MetaData(def));
self.get_crate_data(def.krate).get_span(def.index, sess)
}

fn stability(&self, def: DefId) -> Option<attr::Stability> {
self.dep_graph.read(DepNode::MetaData(def));
self.get_crate_data(def.krate).get_stability(def.index)
Expand Down