Skip to content

Commit

Permalink
Add query for on_unimplemented resolutions
Browse files Browse the repository at this point in the history
  • Loading branch information
cramertj committed Oct 25, 2017
1 parent 82cc990 commit bd36778
Show file tree
Hide file tree
Showing 9 changed files with 111 additions and 35 deletions.
2 changes: 2 additions & 0 deletions src/librustc/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,8 @@ define_dep_nodes!( <'tcx>
[] HasCloneClosures(CrateNum),
[] HasCopyClosures(CrateNum),

[] MatchesResolutions(DefId),

// This query is not expected to have inputs -- as a result, it's
// not a good candidate for "replay" because it's essentially a
// pure function of its input (and hence the expectation is that
Expand Down
52 changes: 50 additions & 2 deletions src/librustc/traits/on_unimplemented.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use ty::{self, TyCtxt};
use util::common::ErrorReported;
use util::nodemap::FxHashMap;

use syntax::ast::{MetaItem, NestedMetaItem};
use syntax::ast::{LitKind, MetaItem, NestedMetaItem};
use syntax::attr;
use syntax_pos::Span;
use syntax_pos::symbol::InternedString;
Expand Down Expand Up @@ -83,7 +83,55 @@ impl<'a, 'gcx, 'tcx> OnUnimplementedDirective {
"invalid on-clause here",
None)
})?;
attr::eval_condition(cond, &tcx.sess.parse_sess, &mut |_| true);
attr::eval_condition_with_custom_list_handler(
cond, &tcx.sess.parse_sess, &mut |_| true,
&mut |attribute, nested_list| {
if attribute.name != "matches" {
return None;
}

let matches_resolutions = tcx.matches_resolutions(trait_def_id)
.expect(&format!(
"No matches resolutions found for trait {:?}", trait_def_id));

let mut bound_name = None;
let mut self_name = None;

for nested_item in nested_list {
if let Some(lit) = nested_item.literal() {
if let LitKind::Str(name, _) = lit.node {
// This is a trait path like
// matches("IsNoneError", Self="T")
// ^^^^^^^^^^^
bound_name = Some(name);

This comment has been minimized.

Copy link
@arielb1

arielb1 Oct 25, 2017

check that there are no duplicates? (e.g. matches("fu", "bar"))?

}
} else if let Some((key, lit)) = nested_item.name_value_literal() {
if key == "Self" {
if let LitKind::Str(name, _) = lit.node {
// This is a self type specification like
// matches("IsNoneError", Self="T")
// ^^^^^^^^
self_name = Some(name);
}
}
}
}

let bound_name = bound_name.expect("matches clause should have a bound literal");
let self_name = self_name.expect("matches clause should have a self KV-pair");

let _bound_id = matches_resolutions.iter()
.find(|res| res.0 == bound_name)
.expect("no resolution for matches bound");

let _self_id = matches_resolutions.iter()
.find(|res| res.0 == self_name)
.expect("no resolution for self found");

// TODO: use these ids to check if the self type implements the bound

This comment has been minimized.

Copy link
@arielb1

arielb1 Oct 25, 2017

This is just the place that parses attributes and checks that they are valid - the "use these ids to check things" part should be in evaluate down in this file.


Some(true)
});
Some(cond.clone())
};

Expand Down
8 changes: 8 additions & 0 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,8 @@ pub struct GlobalCtxt<'tcx> {

maybe_unused_extern_crates: Vec<(DefId, Span)>,

id_to_matches_resolutions: FxHashMap<DefId, Rc<Vec<(Name, DefId)>>>,

// Internal cache for metadata decoding. No need to track deps on this.
pub rcache: RefCell<FxHashMap<ty::CReaderCacheKey, Ty<'tcx>>>,

Expand Down Expand Up @@ -1164,6 +1166,11 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
.into_iter()
.map(|(id, sp)| (hir.local_def_id(id), sp))
.collect(),
id_to_matches_resolutions:
resolutions.id_to_matches_resolutions
.into_iter()
.map(|(id, matches)| (id, Rc::new(matches)))
.collect(),
hir,
def_path_hash_to_def_id,
maps: maps::Maps::new(providers),
Expand Down Expand Up @@ -2252,6 +2259,7 @@ pub fn provide(providers: &mut ty::maps::Providers) {
// FIXME(#44234) - almost all of these queries have no sub-queries and
// therefore no actual inputs, they're just reading tables calculated in
// resolve! Does this work? Unsure! That's what the issue is about
providers.matches_resolutions = |tcx, id| tcx.gcx.id_to_matches_resolutions.get(&id).cloned();
providers.in_scope_traits_map = |tcx, id| tcx.gcx.trait_map.get(&id).cloned();
providers.module_exports = |tcx, id| tcx.gcx.export_map.get(&id).cloned();
providers.named_region_map = |tcx, id| tcx.gcx.named_region_map.defs.get(&id).cloned();
Expand Down
3 changes: 3 additions & 0 deletions src/librustc/ty/maps/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,6 +345,9 @@ define_maps! { <'tcx>
[] fn has_copy_closures: HasCopyClosures(CrateNum) -> bool,
[] fn has_clone_closures: HasCloneClosures(CrateNum) -> bool,

// Resolutions for rustc_on_unimplemented "matches" clause
[] fn matches_resolutions: MatchesResolutions(DefId) -> Option<Rc<Vec<(ast::Name, DefId)>>>,

// Erases regions from `ty` to yield a new type.
// Normally you would just use `tcx.erase_regions(&value)`,
// however, which uses this query as a kind of cache.
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/maps/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -758,6 +758,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>,
DepKind::SpecializationGraph => { force!(specialization_graph_of, def_id!()); }
DepKind::ObjectSafety => { force!(is_object_safe, def_id!()); }
DepKind::TraitImpls => { force!(trait_impls_of, def_id!()); }
DepKind::MatchesResolutions => { force!(matches_resolutions, def_id!()); }

DepKind::ParamEnv => { force!(param_env, def_id!()); }
DepKind::DescribeDef => { force!(describe_def, def_id!()); }
Expand Down
1 change: 1 addition & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ pub struct Resolutions {
pub trait_map: TraitMap,
pub maybe_unused_trait_imports: NodeSet,
pub maybe_unused_extern_crates: Vec<(NodeId, Span)>,
pub id_to_matches_resolutions: DefIdMap<Vec<(Name, DefId)>>,
pub export_map: ExportMap,
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_driver/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,7 @@ pub fn phase_2_configure_and_expand<F>(sess: &Session,
trait_map: resolver.trait_map,
maybe_unused_trait_imports: resolver.maybe_unused_trait_imports,
maybe_unused_extern_crates: resolver.maybe_unused_extern_crates,
id_to_matches_resolutions: resolver.id_to_matches_resolutions,
},
hir_forest,
})
Expand Down
59 changes: 28 additions & 31 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ use syntax::ast::{FnDecl, ForeignItem, ForeignItemKind, Generics};
use syntax::ast::{Item, ItemKind, ImplItem, ImplItemKind};
use syntax::ast::{Local, Mutability, Pat, PatKind, Path};
use syntax::ast::{QSelf, TraitItemKind, TraitRef, Ty, TyKind};
use syntax::ast::{Attribute, LitKind, MetaItemKind, NestedMetaItem, PathSegment};
use syntax::ast::{Attribute, LitKind, NestedMetaItem, PathSegment};
use syntax::feature_gate::{feature_err, emit_feature_err, GateIssue};

use syntax_pos::{Span, DUMMY_SP, MultiSpan};
Expand Down Expand Up @@ -3703,49 +3703,46 @@ impl<'a> Resolver<'a> {
let mut matches = Vec::new();
for attr in attrs {
self.resolve_matches(
node_id, &mut matches, attr.meta_item_list().as_ref().map(|x| x.as_ref()));
node_id, false, &mut matches, attr.meta_item_list().as_ref().map(|x| x.as_ref()));
}
let def_id = self.definitions.local_def_id(node_id);
self.id_to_matches_resolutions.insert(def_id, matches);
}

fn resolve_matches(&mut self,
node_id: NodeId,
seen_on_unimplemented: bool,
matches: &mut Vec<(Name, DefId)>,
meta_item_list: Option<&[NestedMetaItem]>)
{
for item in meta_item_list.into_iter().flat_map(|x| x) {
// Run recursively
self.resolve_matches(node_id, matches, item.meta_item_list());
let seen_on_unimplemented = seen_on_unimplemented || item.check_name("rustc_on_unimplemented");
self.resolve_matches(node_id, seen_on_unimplemented, matches, item.meta_item_list());

// Check for "matches"
if item.check_name("matches") {
// Check for "matches" inside of "rustc_on_unimplemented" attributes
if seen_on_unimplemented && item.check_name("matches") {
for nested_item in item.meta_item_list().into_iter().flat_map(|x| x) {
if let Some(meta_item) = nested_item.meta_item() {
let (to_resolve, path_source, path_span) = match meta_item.node {
MetaItemKind::Word => {
// This is a trait path like
let mut to_resolve = None;
if let Some(lit) = nested_item.literal() {
if let LitKind::Str(name, _) = lit.node {
// This is a trait path like
// matches("IsNoneError", Self="T")
// ^^^^^^^^^^^
to_resolve = Some((name, PathSource::Trait, lit.span));
}
} else if let Some((key, lit)) = nested_item.name_value_literal() {
if key == "Self" {
if let LitKind::Str(name, _) = lit.node {
// This is a self type specification like
// matches("IsNoneError", Self="T")
// ^^^^^^^^^^^
(meta_item.name, PathSource::Trait, meta_item.span)
}
MetaItemKind::NameValue(ref lit) => {
if let LitKind::Str(symbol, _) = lit.node {
if meta_item.name == "Self" {
// This is a self type specification like
// matches("IsNoneError", Self="T")
// ^^^^^^^^
(symbol, PathSource::Type, lit.span)
} else {
continue;
}
} else {
continue
}
// ^^^^^^^^
to_resolve = Some((name, PathSource::Type, lit.span));
}
MetaItemKind::List(_) => continue,
};
}
}

if let Some((name, path_source, span)) = to_resolve {
// TODO(cramertj) this will only work for single-part paths such as
// `T` or `IsNoneError`-- it won't work for `::mymod::IsNoneError`
// or `IsNoneError<T>`-- I'm not sure how best to handle this.
Expand All @@ -3755,17 +3752,17 @@ impl<'a> Resolver<'a> {
node_id,
None,
&Path {
span: path_span,
span: span,
segments: vec![PathSegment {
identifier: to_resolve.to_ident(),
span: path_span,
identifier: name.to_ident(),
span: span,
parameters: None,
}],
},
path_source
);

matches.push((to_resolve, resolution.base_def().def_id()));
matches.push((name, resolution.base_def().def_id()));
}
}
}
Expand Down
19 changes: 17 additions & 2 deletions src/libsyntax/attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,6 +588,17 @@ pub fn cfg_matches(cfg: &ast::MetaItem, sess: &ParseSess, features: Option<&Feat
pub fn eval_condition<F>(cfg: &ast::MetaItem, sess: &ParseSess, eval: &mut F)
-> bool
where F: FnMut(&ast::MetaItem) -> bool
{
eval_condition_with_custom_list_handler(cfg, sess, eval, &mut |_, _| None)
}

/// Evaluate a cfg-like condition (with `any` and `all`), using `eval` to
/// evaluate individual items.
pub fn eval_condition_with_custom_list_handler<F, P>(
cfg: &ast::MetaItem, sess: &ParseSess, eval: &mut F, custom_list_handler: &mut P)
-> bool
where F: FnMut(&ast::MetaItem) -> bool,
P: FnMut(&ast::MetaItem, &[NestedMetaItem]) -> Option<bool>,
{
match cfg.node {
ast::MetaItemKind::List(ref mis) => {
Expand Down Expand Up @@ -616,8 +627,12 @@ pub fn eval_condition<F>(cfg: &ast::MetaItem, sess: &ParseSess, eval: &mut F)
!eval_condition(mis[0].meta_item().unwrap(), sess, eval)
},
p => {
span_err!(sess.span_diagnostic, cfg.span, E0537, "invalid predicate `{}`", p);
false
if let Some(res) = custom_list_handler(cfg, mis) {
res
} else {
span_err!(sess.span_diagnostic, cfg.span, E0537, "invalid predicate `{}`", p);
false
}
}
}
},
Expand Down

0 comments on commit bd36778

Please sign in to comment.