Skip to content

Commit

Permalink
Auto merge of #44818 - petrochenkov:astymac2, r=jseyfried
Browse files Browse the repository at this point in the history
Improve resolution of associated types in declarative macros 2.0

Make various identifier comparisons for associated types (and sometimes other associated items) hygienic.
Now declarative macros 2.0 can use `Self::AssocTy`, `TyParam::AssocTy`, `Trait<AssocTy = u8>` where `AssocTy` is an associated type of a trait `Trait` visible from the macro. Also, `Trait` can now be implemented inside the macro and specialization should work properly (fixes #40847 (comment)).

r? @jseyfried or @eddyb
  • Loading branch information
bors committed Oct 6, 2017
2 parents b915820 + 2d9161d commit ed1cffd
Show file tree
Hide file tree
Showing 11 changed files with 182 additions and 37 deletions.
15 changes: 7 additions & 8 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ use infer::{InferCtxt, InferOk};
use infer::type_variable::TypeVariableOrigin;
use middle::const_val::ConstVal;
use rustc_data_structures::snapshot_map::{Snapshot, SnapshotMap};
use syntax::ast;
use syntax::symbol::Symbol;
use ty::subst::{Subst, Substs};
use ty::{self, ToPredicate, ToPolyTraitRef, Ty, TyCtxt};
Expand Down Expand Up @@ -1044,10 +1043,9 @@ fn assemble_candidates_from_impls<'cx, 'gcx, 'tcx>(
// In either case, we handle this by not adding a
// candidate for an impl if it contains a `default`
// type.
let item_name = selcx.tcx().associated_item(obligation.predicate.item_def_id).name;
let node_item = assoc_ty_def(selcx,
impl_data.impl_def_id,
item_name);
obligation.predicate.item_def_id);

let is_default = if node_item.node.is_from_trait() {
// If true, the impl inherited a `type Foo = Bar`
Expand Down Expand Up @@ -1441,8 +1439,7 @@ fn confirm_impl_candidate<'cx, 'gcx, 'tcx>(

let tcx = selcx.tcx();
let param_env = obligation.param_env;
let assoc_ty = assoc_ty_def(selcx, impl_def_id,
tcx.associated_item(obligation.predicate.item_def_id).name);
let assoc_ty = assoc_ty_def(selcx, impl_def_id, obligation.predicate.item_def_id);

let ty = if !assoc_ty.item.defaultness.has_value() {
// This means that the impl is missing a definition for the
Expand Down Expand Up @@ -1471,10 +1468,11 @@ fn confirm_impl_candidate<'cx, 'gcx, 'tcx>(
fn assoc_ty_def<'cx, 'gcx, 'tcx>(
selcx: &SelectionContext<'cx, 'gcx, 'tcx>,
impl_def_id: DefId,
assoc_ty_name: ast::Name)
assoc_ty_def_id: DefId)
-> specialization_graph::NodeItem<ty::AssociatedItem>
{
let tcx = selcx.tcx();
let assoc_ty_name = tcx.associated_item(assoc_ty_def_id).name;
let trait_def_id = tcx.impl_trait_ref(impl_def_id).unwrap().def_id;
let trait_def = tcx.trait_def(trait_def_id);

Expand All @@ -1486,7 +1484,8 @@ fn assoc_ty_def<'cx, 'gcx, 'tcx>(
// cycle error if the specialization graph is currently being built.
let impl_node = specialization_graph::Node::Impl(impl_def_id);
for item in impl_node.items(tcx) {
if item.kind == ty::AssociatedKind::Type && item.name == assoc_ty_name {
if item.kind == ty::AssociatedKind::Type &&
tcx.hygienic_eq(item.name, assoc_ty_name, trait_def_id) {
return specialization_graph::NodeItem {
node: specialization_graph::Node::Impl(impl_def_id),
item,
Expand All @@ -1496,7 +1495,7 @@ fn assoc_ty_def<'cx, 'gcx, 'tcx>(

if let Some(assoc_item) = trait_def
.ancestors(tcx, impl_def_id)
.defs(tcx, assoc_ty_name, ty::AssociatedKind::Type)
.defs(tcx, assoc_ty_name, ty::AssociatedKind::Type, trait_def_id)
.next() {
assoc_item
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/traits/specialize/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ pub fn find_associated_item<'a, 'tcx>(
let trait_def = tcx.trait_def(trait_def_id);

let ancestors = trait_def.ancestors(tcx, impl_data.impl_def_id);
match ancestors.defs(tcx, item.name, item.kind).next() {
match ancestors.defs(tcx, item.name, item.kind, trait_def_id).next() {
Some(node_item) => {
let substs = tcx.infer_ctxt().enter(|infcx| {
let param_env = ty::ParamEnv::empty(Reveal::All);
Expand Down
9 changes: 6 additions & 3 deletions src/librustc/traits/specialize/specialization_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,11 +346,14 @@ impl<'a, 'gcx, 'tcx> Ancestors {
/// Search the items from the given ancestors, returning each definition
/// with the given name and the given kind.
#[inline] // FIXME(#35870) Avoid closures being unexported due to impl Trait.
pub fn defs(self, tcx: TyCtxt<'a, 'gcx, 'tcx>, name: Name, kind: ty::AssociatedKind)
pub fn defs(self, tcx: TyCtxt<'a, 'gcx, 'tcx>, trait_item_name: Name,
trait_item_kind: ty::AssociatedKind, trait_def_id: DefId)
-> impl Iterator<Item = NodeItem<ty::AssociatedItem>> + 'a {
self.flat_map(move |node| {
node.items(tcx).filter(move |item| item.kind == kind && item.name == name)
.map(move |item| NodeItem { node: node, item: item })
node.items(tcx).filter(move |impl_item| {
impl_item.kind == trait_item_kind &&
tcx.hygienic_eq(impl_item.name, trait_item_name, trait_def_id)
}).map(move |item| NodeItem { node: node, item: item })
})
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2345,6 +2345,13 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
}
}

// Hygienically compare a use-site name (`use_name`) for a field or an associated item with its
// supposed definition name (`def_name`). The method also needs `DefId` of the supposed
// definition's parent/scope to perform comparison.
pub fn hygienic_eq(self, use_name: Name, def_name: Name, def_parent_def_id: DefId) -> bool {
self.adjust(use_name, def_parent_def_id, DUMMY_NODE_ID).0 == def_name.to_ident()
}

pub fn adjust(self, name: Name, scope: DefId, block: NodeId) -> (Ident, DefId) {
self.adjust_ident(name.to_ident(), scope, block)
}
Expand All @@ -2356,6 +2363,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
};
let scope = match ident.ctxt.adjust(expansion) {
Some(macro_def) => self.hir.definitions().macro_def_scope(macro_def),
None if block == DUMMY_NODE_ID => DefId::local(CRATE_DEF_INDEX), // Dummy DefId
None => self.hir.get_module_parent(block),
};
(ident, scope)
Expand Down
7 changes: 4 additions & 3 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -596,9 +596,10 @@ impl<'a, 'tcx> ProjectionTy<'tcx> {
pub fn from_ref_and_name(
tcx: TyCtxt, trait_ref: ty::TraitRef<'tcx>, item_name: Name
) -> ProjectionTy<'tcx> {
let item_def_id = tcx.associated_items(trait_ref.def_id).find(
|item| item.name == item_name && item.kind == ty::AssociatedKind::Type
).unwrap().def_id;
let item_def_id = tcx.associated_items(trait_ref.def_id).find(|item| {
item.kind == ty::AssociatedKind::Type &&
tcx.hygienic_eq(item_name, item.name, trait_ref.def_id)
}).unwrap().def_id;

ProjectionTy {
substs: trait_ref.substs,
Expand Down
26 changes: 11 additions & 15 deletions src/librustc_typeck/astconv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -356,9 +356,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
poly_projections.extend(assoc_bindings.iter().filter_map(|binding| {
// specify type to assert that error was already reported in Err case:
let predicate: Result<_, ErrorReported> =
self.ast_type_binding_to_poly_projection_predicate(trait_ref.ref_id,
poly_trait_ref,
binding);
self.ast_type_binding_to_poly_projection_predicate(poly_trait_ref, binding);
predicate.ok() // ok to ignore Err() because ErrorReported (see above)
}));

Expand Down Expand Up @@ -423,13 +421,13 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
-> bool
{
self.tcx().associated_items(trait_def_id).any(|item| {
item.kind == ty::AssociatedKind::Type && item.name == assoc_name
item.kind == ty::AssociatedKind::Type &&
self.tcx().hygienic_eq(assoc_name, item.name, trait_def_id)
})
}

fn ast_type_binding_to_poly_projection_predicate(
&self,
_path_id: ast::NodeId,
trait_ref: ty::PolyTraitRef<'tcx>,
binding: &ConvertedBinding<'tcx>)
-> Result<ty::PolyProjectionPredicate<'tcx>, ErrorReported>
Expand Down Expand Up @@ -504,7 +502,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {

let candidate = self.one_bound_for_assoc_type(candidates,
&trait_ref.to_string(),
&binding.item_name.as_str(),
binding.item_name,
binding.span)?;

Ok(candidate.map_bound(|trait_ref| {
Expand Down Expand Up @@ -702,7 +700,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
let param_name = tcx.hir.ty_param_name(param_node_id);
self.one_bound_for_assoc_type(suitable_bounds,
&param_name.as_str(),
&assoc_name.as_str(),
assoc_name,
span)
}

Expand All @@ -712,7 +710,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
fn one_bound_for_assoc_type<I>(&self,
mut bounds: I,
ty_param_name: &str,
assoc_name: &str,
assoc_name: ast::Name,
span: Span)
-> Result<ty::PolyTraitRef<'tcx>, ErrorReported>
where I: Iterator<Item=ty::PolyTraitRef<'tcx>>
Expand Down Expand Up @@ -741,7 +739,8 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {

for bound in bounds {
let bound_span = self.tcx().associated_items(bound.def_id()).find(|item| {
item.kind == ty::AssociatedKind::Type && item.name == assoc_name
item.kind == ty::AssociatedKind::Type &&
self.tcx().hygienic_eq(assoc_name, item.name, bound.def_id())
})
.and_then(|item| self.tcx().hir.span_if_local(item.def_id));

Expand Down Expand Up @@ -802,10 +801,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
.filter(|r| self.trait_defines_associated_type_named(r.def_id(),
assoc_name));

match self.one_bound_for_assoc_type(candidates,
"Self",
&assoc_name.as_str(),
span) {
match self.one_bound_for_assoc_type(candidates, "Self", assoc_name, span) {
Ok(bound) => bound,
Err(ErrorReported) => return (tcx.types.err, Def::Err),
}
Expand All @@ -830,14 +826,14 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o {
};

let trait_did = bound.0.def_id;
let item = tcx.associated_items(trait_did).find(|i| i.name == assoc_name)
let (assoc_ident, def_scope) = tcx.adjust(assoc_name, trait_did, ref_id);
let item = tcx.associated_items(trait_did).find(|i| i.name.to_ident() == assoc_ident)
.expect("missing associated type");

let ty = self.projected_ty_from_poly_trait_ref(span, item.def_id, bound);
let ty = self.normalize_ty(span, ty);

let def = Def::AssociatedTy(item.def_id);
let def_scope = tcx.adjust(assoc_name, item.container.id(), ref_id).1;
if !item.vis.is_accessible_from(def_scope, tcx) {
let msg = format!("{} `{}` is private", def.kind_name(), assoc_name);
tcx.sess.span_err(span, &msg);
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_typeck/check/method/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -373,7 +373,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> {
/// and return it, or `None`, if no such item was defined there.
pub fn associated_item(&self, def_id: DefId, item_name: ast::Name)
-> Option<ty::AssociatedItem> {
let ident = self.tcx.adjust(item_name, def_id, self.body_id).0;
self.tcx.associated_items(def_id).find(|item| item.name.to_ident() == ident)
self.tcx.associated_items(def_id)
.find(|item| self.tcx.hygienic_eq(item_name, item.name, def_id))

}
}
12 changes: 7 additions & 5 deletions src/librustc_typeck/check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1248,6 +1248,7 @@ fn report_forbidden_specialization<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,

fn check_specialization_validity<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
trait_def: &ty::TraitDef,
trait_item: &ty::AssociatedItem,
impl_id: DefId,
impl_item: &hir::ImplItem)
{
Expand All @@ -1258,7 +1259,8 @@ fn check_specialization_validity<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
hir::ImplItemKind::Method(..) => ty::AssociatedKind::Method,
hir::ImplItemKind::Type(_) => ty::AssociatedKind::Type
};
let parent = ancestors.defs(tcx, impl_item.name, kind).skip(1).next()

let parent = ancestors.defs(tcx, trait_item.name, kind, trait_def.def_id).skip(1).next()
.map(|node_item| node_item.map(|parent| parent.defaultness));

if let Some(parent) = parent {
Expand Down Expand Up @@ -1290,7 +1292,7 @@ fn check_impl_items_against_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
for impl_item in impl_items() {
let ty_impl_item = tcx.associated_item(tcx.hir.local_def_id(impl_item.id));
let ty_trait_item = tcx.associated_items(impl_trait_ref.def_id)
.find(|ac| ac.name == ty_impl_item.name);
.find(|ac| tcx.hygienic_eq(ty_impl_item.name, ac.name, impl_trait_ref.def_id));

// Check that impl definition matches trait definition
if let Some(ty_trait_item) = ty_trait_item {
Expand Down Expand Up @@ -1371,9 +1373,9 @@ fn check_impl_items_against_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}
}
}
}

check_specialization_validity(tcx, trait_def, impl_id, impl_item);
check_specialization_validity(tcx, trait_def, &ty_trait_item, impl_id, impl_item);
}
}

// Check for missing items from trait
Expand All @@ -1382,7 +1384,7 @@ fn check_impl_items_against_trait<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let associated_type_overridden = overridden_associated_type.is_some();
for trait_item in tcx.associated_items(impl_trait_ref.def_id) {
let is_implemented = trait_def.ancestors(tcx, impl_id)
.defs(tcx, trait_item.name, trait_item.kind)
.defs(tcx, trait_item.name, trait_item.kind, impl_trait_ref.def_id)
.next()
.map(|node_item| !node_item.node.is_from_trait())
.unwrap_or(false);
Expand Down
52 changes: 52 additions & 0 deletions src/test/compile-fail/hygiene/assoc_item_ctxt.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// ignore-pretty pretty-printing is unhygienic

#![feature(decl_macro)]
#![allow(unused)]

mod ok {
macro mac_trait_item($method: ident) {
fn $method();
}

trait Tr {
mac_trait_item!(method);
}

macro mac_trait_impl() {
impl Tr for u8 { // OK
fn method() {} // OK
}
}

mac_trait_impl!();
}

mod error {
macro mac_trait_item() {
fn method();
}

trait Tr {
mac_trait_item!();
}

macro mac_trait_impl() {
impl Tr for u8 { //~ ERROR not all trait items implemented, missing: `method`
fn method() {} //~ ERROR method `method` is not a member of trait `Tr`
}
}

mac_trait_impl!();
}

fn main() {}
49 changes: 49 additions & 0 deletions src/test/compile-fail/hygiene/assoc_ty_bindings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// ignore-pretty pretty-printing is unhygienic

#![feature(decl_macro, associated_type_defaults)]
#![feature(rustc_attrs)]

trait Base {
type AssocTy;
fn f();
}
trait Derived: Base {
fn g();
}

macro mac() {
type A = Base<AssocTy = u8>;
type B = Derived<AssocTy = u8>;

impl Base for u8 {
type AssocTy = u8;
fn f() {
let _: Self::AssocTy;
}
}
impl Derived for u8 {
fn g() {
let _: Self::AssocTy;
}
}

fn h<T: Base, U: Derived>() {
let _: T::AssocTy;
let _: U::AssocTy;
}
}

mac!();

#[rustc_error]
fn main() {} //~ ERROR compilation successful
Loading

0 comments on commit ed1cffd

Please sign in to comment.