From f8a2f9838d9df85d757118a44855004f9805792f Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 4 Dec 2016 00:28:30 +0200 Subject: [PATCH 1/7] coherence: move the builtin trait checks to their own module no functional changes --- src/librustc_typeck/coherence/builtin.rs | 357 +++++++++++++++++++++++ src/librustc_typeck/coherence/mod.rs | 356 +--------------------- 2 files changed, 361 insertions(+), 352 deletions(-) create mode 100644 src/librustc_typeck/coherence/builtin.rs diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs new file mode 100644 index 0000000000000..6fde18dbf82a4 --- /dev/null +++ b/src/librustc_typeck/coherence/builtin.rs @@ -0,0 +1,357 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +//! Check properties that are required by built-in traits and set +//! up data structures required by type-checking/translation. + +use rustc::middle::free_region::FreeRegionMap; +use rustc::middle::lang_items::UnsizeTraitLangItem; + +use rustc::traits::{self, ObligationCause, Reveal}; +use rustc::ty::{self, Ty, TyCtxt}; +use rustc::ty::ParameterEnvironment; +use rustc::ty::TypeFoldable; +use rustc::ty::subst::Subst; +use rustc::ty::util::CopyImplementationError; +use rustc::infer; + +use rustc::hir::map as hir_map; +use rustc::hir::{self, ItemImpl}; + +pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + populate_destructors(tcx); + check_implementations_of_copy(tcx); + check_implementations_of_coerce_unsized(tcx); +} + +fn populate_destructors<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + let drop_trait = match tcx.lang_items.drop_trait() { + Some(id) => id, + None => return, + }; + tcx.populate_implementations_for_trait_if_necessary(drop_trait); + let drop_trait = tcx.lookup_trait_def(drop_trait); + + drop_trait.for_each_impl(tcx, |impl_did| { + let items = tcx.associated_item_def_ids(impl_did); + if items.is_empty() { + // We'll error out later. For now, just don't ICE. + return; + } + let method_def_id = items[0]; + + let self_type = tcx.item_type(impl_did); + match self_type.sty { + ty::TyAdt(type_def, _) => { + type_def.set_destructor(method_def_id); + } + _ => { + // Destructors only work on nominal types. + if let Some(impl_node_id) = tcx.map.as_local_node_id(impl_did) { + match tcx.map.find(impl_node_id) { + Some(hir_map::NodeItem(item)) => { + let span = match item.node { + ItemImpl(.., ref ty, _) => ty.span, + _ => item.span, + }; + struct_span_err!(tcx.sess, + span, + E0120, + "the Drop trait may only be implemented on \ + structures") + .span_label(span, + &format!("implementing Drop requires a struct")) + .emit(); + } + _ => { + bug!("didn't find impl in ast map"); + } + } + } else { + bug!("found external impl of Drop trait on \ + something other than a struct"); + } + } + } + }); +} + +fn check_implementations_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + let copy_trait = match tcx.lang_items.copy_trait() { + Some(id) => id, + None => return, + }; + tcx.populate_implementations_for_trait_if_necessary(copy_trait); + let copy_trait = tcx.lookup_trait_def(copy_trait); + + copy_trait.for_each_impl(tcx, |impl_did| { + debug!("check_implementations_of_copy: impl_did={:?}", impl_did); + + let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { + n + } else { + debug!("check_implementations_of_copy(): impl not in this \ + crate"); + return; + }; + + let self_type = tcx.item_type(impl_did); + debug!("check_implementations_of_copy: self_type={:?} (bound)", + self_type); + + let span = tcx.map.span(impl_node_id); + let param_env = ParameterEnvironment::for_item(tcx, impl_node_id); + let self_type = self_type.subst(tcx, ¶m_env.free_substs); + assert!(!self_type.has_escaping_regions()); + + debug!("check_implementations_of_copy: self_type={:?} (free)", + self_type); + + match param_env.can_type_implement_copy(tcx, self_type, span) { + Ok(()) => {} + Err(CopyImplementationError::InfrigingField(name)) => { + struct_span_err!(tcx.sess, + span, + E0204, + "the trait `Copy` may not be implemented for this type") + .span_label(span, &format!("field `{}` does not implement `Copy`", name)) + .emit() + } + Err(CopyImplementationError::InfrigingVariant(name)) => { + let item = tcx.map.expect_item(impl_node_id); + let span = if let ItemImpl(.., Some(ref tr), _, _) = item.node { + tr.path.span + } else { + span + }; + + struct_span_err!(tcx.sess, + span, + E0205, + "the trait `Copy` may not be implemented for this type") + .span_label(span, + &format!("variant `{}` does not implement `Copy`", name)) + .emit() + } + Err(CopyImplementationError::NotAnAdt) => { + let item = tcx.map.expect_item(impl_node_id); + let span = if let ItemImpl(.., ref ty, _) = item.node { + ty.span + } else { + span + }; + + struct_span_err!(tcx.sess, + span, + E0206, + "the trait `Copy` may not be implemented for this type") + .span_label(span, &format!("type is not a structure or enumeration")) + .emit(); + } + Err(CopyImplementationError::HasDestructor) => { + struct_span_err!(tcx.sess, + span, + E0184, + "the trait `Copy` may not be implemented for this type; the \ + type has a destructor") + .span_label(span, &format!("Copy not allowed on types with destructors")) + .emit(); + } + } + }); +} + +fn check_implementations_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { + let coerce_unsized_trait = match tcx.lang_items.coerce_unsized_trait() { + Some(id) => id, + None => return, + }; + let unsize_trait = match tcx.lang_items.require(UnsizeTraitLangItem) { + Ok(id) => id, + Err(err) => { + tcx.sess.fatal(&format!("`CoerceUnsized` implementation {}", err)); + } + }; + + let trait_def = tcx.lookup_trait_def(coerce_unsized_trait); + + trait_def.for_each_impl(tcx, |impl_did| { + debug!("check_implementations_of_coerce_unsized: impl_did={:?}", + impl_did); + + let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { + n + } else { + debug!("check_implementations_of_coerce_unsized(): impl not \ + in this crate"); + return; + }; + + let source = tcx.item_type(impl_did); + let trait_ref = tcx.impl_trait_ref(impl_did).unwrap(); + let target = trait_ref.substs.type_at(1); + debug!("check_implementations_of_coerce_unsized: {:?} -> {:?} (bound)", + source, + target); + + let span = tcx.map.span(impl_node_id); + let param_env = ParameterEnvironment::for_item(tcx, impl_node_id); + let source = source.subst(tcx, ¶m_env.free_substs); + let target = target.subst(tcx, ¶m_env.free_substs); + assert!(!source.has_escaping_regions()); + + debug!("check_implementations_of_coerce_unsized: {:?} -> {:?} (free)", + source, + target); + + tcx.infer_ctxt(None, Some(param_env), Reveal::ExactMatch).enter(|infcx| { + let cause = ObligationCause::misc(span, impl_node_id); + let check_mutbl = |mt_a: ty::TypeAndMut<'tcx>, + mt_b: ty::TypeAndMut<'tcx>, + mk_ptr: &Fn(Ty<'tcx>) -> Ty<'tcx>| { + if (mt_a.mutbl, mt_b.mutbl) == (hir::MutImmutable, hir::MutMutable) { + infcx.report_mismatched_types(&cause, + mk_ptr(mt_b.ty), + target, + ty::error::TypeError::Mutability) + .emit(); + } + (mt_a.ty, mt_b.ty, unsize_trait, None) + }; + let (source, target, trait_def_id, kind) = match (&source.sty, &target.sty) { + (&ty::TyBox(a), &ty::TyBox(b)) => (a, b, unsize_trait, None), + + (&ty::TyRef(r_a, mt_a), &ty::TyRef(r_b, mt_b)) => { + infcx.sub_regions(infer::RelateObjectBound(span), r_b, r_a); + check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ref(r_b, ty)) + } + + (&ty::TyRef(_, mt_a), &ty::TyRawPtr(mt_b)) | + (&ty::TyRawPtr(mt_a), &ty::TyRawPtr(mt_b)) => { + check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ptr(ty)) + } + + (&ty::TyAdt(def_a, substs_a), &ty::TyAdt(def_b, substs_b)) + if def_a.is_struct() && def_b.is_struct() => { + if def_a != def_b { + let source_path = tcx.item_path_str(def_a.did); + let target_path = tcx.item_path_str(def_b.did); + span_err!(tcx.sess, + span, + E0377, + "the trait `CoerceUnsized` may only be implemented \ + for a coercion between structures with the same \ + definition; expected {}, found {}", + source_path, + target_path); + return; + } + + let fields = &def_a.struct_variant().fields; + let diff_fields = fields.iter() + .enumerate() + .filter_map(|(i, f)| { + let (a, b) = (f.ty(tcx, substs_a), f.ty(tcx, substs_b)); + + if tcx.item_type(f.did).is_phantom_data() { + // Ignore PhantomData fields + return None; + } + + // Ignore fields that aren't significantly changed + if let Ok(ok) = infcx.sub_types(false, &cause, b, a) { + if ok.obligations.is_empty() { + return None; + } + } + + // Collect up all fields that were significantly changed + // i.e. those that contain T in coerce_unsized T -> U + Some((i, a, b)) + }) + .collect::>(); + + if diff_fields.is_empty() { + span_err!(tcx.sess, + span, + E0374, + "the trait `CoerceUnsized` may only be implemented \ + for a coercion between structures with one field \ + being coerced, none found"); + return; + } else if diff_fields.len() > 1 { + let item = tcx.map.expect_item(impl_node_id); + let span = if let ItemImpl(.., Some(ref t), _, _) = item.node { + t.path.span + } else { + tcx.map.span(impl_node_id) + }; + + let mut err = struct_span_err!(tcx.sess, + span, + E0375, + "implementing the trait \ + `CoerceUnsized` requires multiple \ + coercions"); + err.note("`CoerceUnsized` may only be implemented for \ + a coercion between structures with one field being coerced"); + err.note(&format!("currently, {} fields need coercions: {}", + diff_fields.len(), + diff_fields.iter() + .map(|&(i, a, b)| { + format!("{} ({} to {})", fields[i].name, a, b) + }) + .collect::>() + .join(", "))); + err.span_label(span, &format!("requires multiple coercions")); + err.emit(); + return; + } + + let (i, a, b) = diff_fields[0]; + let kind = ty::adjustment::CustomCoerceUnsized::Struct(i); + (a, b, coerce_unsized_trait, Some(kind)) + } + + _ => { + span_err!(tcx.sess, + span, + E0376, + "the trait `CoerceUnsized` may only be implemented \ + for a coercion between structures"); + return; + } + }; + + let mut fulfill_cx = traits::FulfillmentContext::new(); + + // Register an obligation for `A: Trait`. + let cause = traits::ObligationCause::misc(span, impl_node_id); + let predicate = + tcx.predicate_for_trait_def(cause, trait_def_id, 0, source, &[target]); + fulfill_cx.register_predicate_obligation(&infcx, predicate); + + // Check that all transitive obligations are satisfied. + if let Err(errors) = fulfill_cx.select_all_or_error(&infcx) { + infcx.report_fulfillment_errors(&errors); + } + + // Finally, resolve all regions. + let mut free_regions = FreeRegionMap::new(); + free_regions.relate_free_regions_from_predicates(&infcx.parameter_environment + .caller_bounds); + infcx.resolve_regions_and_report_errors(&free_regions, impl_node_id); + + if let Some(kind) = kind { + tcx.custom_coerce_unsized_kinds.borrow_mut().insert(impl_did, kind); + } + }); + }); +} diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 3bbe5aa1fef37..3293818348a94 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -16,28 +16,23 @@ // mappings. That mapping code resides here. use hir::def_id::DefId; -use middle::lang_items::UnsizeTraitLangItem; -use rustc::ty::subst::Subst; +use rustc::traits::Reveal; use rustc::ty::{self, TyCtxt, TypeFoldable}; -use rustc::traits::{self, ObligationCause, Reveal}; -use rustc::ty::ParameterEnvironment; use rustc::ty::{Ty, TyBool, TyChar, TyError}; use rustc::ty::{TyParam, TyRawPtr}; use rustc::ty::{TyRef, TyAdt, TyDynamic, TyNever, TyTuple}; use rustc::ty::{TyStr, TyArray, TySlice, TyFloat, TyInfer, TyInt}; use rustc::ty::{TyUint, TyClosure, TyBox, TyFnDef, TyFnPtr}; use rustc::ty::{TyProjection, TyAnon}; -use rustc::ty::util::CopyImplementationError; -use middle::free_region::FreeRegionMap; use CrateCtxt; -use rustc::infer::{self, InferCtxt}; +use rustc::infer::{InferCtxt}; use syntax_pos::Span; use rustc::dep_graph::DepNode; -use rustc::hir::map as hir_map; use rustc::hir::itemlikevisit::ItemLikeVisitor; use rustc::hir::{Item, ItemImpl}; use rustc::hir; +mod builtin; mod orphan; mod overlap; mod unsafety; @@ -96,18 +91,7 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { self.crate_context.tcx.visit_all_item_likes_in_krate( DepNode::CoherenceCheckImpl, &mut CoherenceCheckVisitor { cc: self }); - - // Populate the table of destructors. It might seem a bit strange to - // do this here, but it's actually the most convenient place, since - // the coherence tables contain the trait -> type mappings. - self.populate_destructors(); - - // Check to make sure implementations of `Copy` are legal. - self.check_implementations_of_copy(); - - // Check to make sure implementations of `CoerceUnsized` are legal - // and collect the necessary information from them. - self.check_implementations_of_coerce_unsized(); + builtin::check(self.crate_context.tcx); } fn check_implementation(&self, item: &Item) { @@ -161,338 +145,6 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { let trait_def = self.crate_context.tcx.lookup_trait_def(impl_trait_ref.def_id); trait_def.record_local_impl(self.crate_context.tcx, impl_def_id, impl_trait_ref); } - - // Destructors - // - - fn populate_destructors(&self) { - let tcx = self.crate_context.tcx; - let drop_trait = match tcx.lang_items.drop_trait() { - Some(id) => id, - None => return, - }; - tcx.populate_implementations_for_trait_if_necessary(drop_trait); - let drop_trait = tcx.lookup_trait_def(drop_trait); - - drop_trait.for_each_impl(tcx, |impl_did| { - let items = tcx.associated_item_def_ids(impl_did); - if items.is_empty() { - // We'll error out later. For now, just don't ICE. - return; - } - let method_def_id = items[0]; - - let self_type = tcx.item_type(impl_did); - match self_type.sty { - ty::TyAdt(type_def, _) => { - type_def.set_destructor(method_def_id); - } - _ => { - // Destructors only work on nominal types. - if let Some(impl_node_id) = tcx.map.as_local_node_id(impl_did) { - match tcx.map.find(impl_node_id) { - Some(hir_map::NodeItem(item)) => { - let span = match item.node { - ItemImpl(.., ref ty, _) => ty.span, - _ => item.span, - }; - struct_span_err!(tcx.sess, - span, - E0120, - "the Drop trait may only be implemented on \ - structures") - .span_label(span, - &format!("implementing Drop requires a struct")) - .emit(); - } - _ => { - bug!("didn't find impl in ast map"); - } - } - } else { - bug!("found external impl of Drop trait on \ - something other than a struct"); - } - } - } - }); - } - - /// Ensures that implementations of the built-in trait `Copy` are legal. - fn check_implementations_of_copy(&self) { - let tcx = self.crate_context.tcx; - let copy_trait = match tcx.lang_items.copy_trait() { - Some(id) => id, - None => return, - }; - tcx.populate_implementations_for_trait_if_necessary(copy_trait); - let copy_trait = tcx.lookup_trait_def(copy_trait); - - copy_trait.for_each_impl(tcx, |impl_did| { - debug!("check_implementations_of_copy: impl_did={:?}", impl_did); - - let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { - n - } else { - debug!("check_implementations_of_copy(): impl not in this \ - crate"); - return; - }; - - let self_type = tcx.item_type(impl_did); - debug!("check_implementations_of_copy: self_type={:?} (bound)", - self_type); - - let span = tcx.map.span(impl_node_id); - let param_env = ParameterEnvironment::for_item(tcx, impl_node_id); - let self_type = self_type.subst(tcx, ¶m_env.free_substs); - assert!(!self_type.has_escaping_regions()); - - debug!("check_implementations_of_copy: self_type={:?} (free)", - self_type); - - match param_env.can_type_implement_copy(tcx, self_type, span) { - Ok(()) => {} - Err(CopyImplementationError::InfrigingField(name)) => { - struct_span_err!(tcx.sess, - span, - E0204, - "the trait `Copy` may not be implemented for this type") - .span_label(span, &format!("field `{}` does not implement `Copy`", name)) - .emit() - } - Err(CopyImplementationError::InfrigingVariant(name)) => { - let item = tcx.map.expect_item(impl_node_id); - let span = if let ItemImpl(.., Some(ref tr), _, _) = item.node { - tr.path.span - } else { - span - }; - - struct_span_err!(tcx.sess, - span, - E0205, - "the trait `Copy` may not be implemented for this type") - .span_label(span, - &format!("variant `{}` does not implement `Copy`", name)) - .emit() - } - Err(CopyImplementationError::NotAnAdt) => { - let item = tcx.map.expect_item(impl_node_id); - let span = if let ItemImpl(.., ref ty, _) = item.node { - ty.span - } else { - span - }; - - struct_span_err!(tcx.sess, - span, - E0206, - "the trait `Copy` may not be implemented for this type") - .span_label(span, &format!("type is not a structure or enumeration")) - .emit(); - } - Err(CopyImplementationError::HasDestructor) => { - struct_span_err!(tcx.sess, - span, - E0184, - "the trait `Copy` may not be implemented for this type; the \ - type has a destructor") - .span_label(span, &format!("Copy not allowed on types with destructors")) - .emit(); - } - } - }); - } - - /// Process implementations of the built-in trait `CoerceUnsized`. - fn check_implementations_of_coerce_unsized(&self) { - let tcx = self.crate_context.tcx; - let coerce_unsized_trait = match tcx.lang_items.coerce_unsized_trait() { - Some(id) => id, - None => return, - }; - let unsize_trait = match tcx.lang_items.require(UnsizeTraitLangItem) { - Ok(id) => id, - Err(err) => { - tcx.sess.fatal(&format!("`CoerceUnsized` implementation {}", err)); - } - }; - - let trait_def = tcx.lookup_trait_def(coerce_unsized_trait); - - trait_def.for_each_impl(tcx, |impl_did| { - debug!("check_implementations_of_coerce_unsized: impl_did={:?}", - impl_did); - - let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { - n - } else { - debug!("check_implementations_of_coerce_unsized(): impl not \ - in this crate"); - return; - }; - - let source = tcx.item_type(impl_did); - let trait_ref = self.crate_context.tcx.impl_trait_ref(impl_did).unwrap(); - let target = trait_ref.substs.type_at(1); - debug!("check_implementations_of_coerce_unsized: {:?} -> {:?} (bound)", - source, - target); - - let span = tcx.map.span(impl_node_id); - let param_env = ParameterEnvironment::for_item(tcx, impl_node_id); - let source = source.subst(tcx, ¶m_env.free_substs); - let target = target.subst(tcx, ¶m_env.free_substs); - assert!(!source.has_escaping_regions()); - - debug!("check_implementations_of_coerce_unsized: {:?} -> {:?} (free)", - source, - target); - - tcx.infer_ctxt(None, Some(param_env), Reveal::ExactMatch).enter(|infcx| { - let cause = ObligationCause::misc(span, impl_node_id); - let check_mutbl = |mt_a: ty::TypeAndMut<'gcx>, - mt_b: ty::TypeAndMut<'gcx>, - mk_ptr: &Fn(Ty<'gcx>) -> Ty<'gcx>| { - if (mt_a.mutbl, mt_b.mutbl) == (hir::MutImmutable, hir::MutMutable) { - infcx.report_mismatched_types(&cause, - mk_ptr(mt_b.ty), - target, - ty::error::TypeError::Mutability).emit(); - } - (mt_a.ty, mt_b.ty, unsize_trait, None) - }; - let (source, target, trait_def_id, kind) = match (&source.sty, &target.sty) { - (&ty::TyBox(a), &ty::TyBox(b)) => (a, b, unsize_trait, None), - - (&ty::TyRef(r_a, mt_a), &ty::TyRef(r_b, mt_b)) => { - infcx.sub_regions(infer::RelateObjectBound(span), r_b, r_a); - check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ref(r_b, ty)) - } - - (&ty::TyRef(_, mt_a), &ty::TyRawPtr(mt_b)) | - (&ty::TyRawPtr(mt_a), &ty::TyRawPtr(mt_b)) => { - check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ptr(ty)) - } - - (&ty::TyAdt(def_a, substs_a), &ty::TyAdt(def_b, substs_b)) - if def_a.is_struct() && def_b.is_struct() => { - if def_a != def_b { - let source_path = tcx.item_path_str(def_a.did); - let target_path = tcx.item_path_str(def_b.did); - span_err!(tcx.sess, - span, - E0377, - "the trait `CoerceUnsized` may only be implemented \ - for a coercion between structures with the same \ - definition; expected {}, found {}", - source_path, - target_path); - return; - } - - let fields = &def_a.struct_variant().fields; - let diff_fields = fields.iter() - .enumerate() - .filter_map(|(i, f)| { - let (a, b) = (f.ty(tcx, substs_a), f.ty(tcx, substs_b)); - - if tcx.item_type(f.did).is_phantom_data() { - // Ignore PhantomData fields - return None; - } - - // Ignore fields that aren't significantly changed - if let Ok(ok) = infcx.sub_types(false, &cause, b, a) { - if ok.obligations.is_empty() { - return None; - } - } - - // Collect up all fields that were significantly changed - // i.e. those that contain T in coerce_unsized T -> U - Some((i, a, b)) - }) - .collect::>(); - - if diff_fields.is_empty() { - span_err!(tcx.sess, - span, - E0374, - "the trait `CoerceUnsized` may only be implemented \ - for a coercion between structures with one field \ - being coerced, none found"); - return; - } else if diff_fields.len() > 1 { - let item = tcx.map.expect_item(impl_node_id); - let span = if let ItemImpl(.., Some(ref t), _, _) = item.node { - t.path.span - } else { - tcx.map.span(impl_node_id) - }; - - let mut err = struct_span_err!(tcx.sess, - span, - E0375, - "implementing the trait \ - `CoerceUnsized` requires multiple \ - coercions"); - err.note("`CoerceUnsized` may only be implemented for \ - a coercion between structures with one field being coerced"); - err.note(&format!("currently, {} fields need coercions: {}", - diff_fields.len(), - diff_fields.iter() - .map(|&(i, a, b)| { - format!("{} ({} to {})", fields[i].name, a, b) - }) - .collect::>() - .join(", "))); - err.span_label(span, &format!("requires multiple coercions")); - err.emit(); - return; - } - - let (i, a, b) = diff_fields[0]; - let kind = ty::adjustment::CustomCoerceUnsized::Struct(i); - (a, b, coerce_unsized_trait, Some(kind)) - } - - _ => { - span_err!(tcx.sess, - span, - E0376, - "the trait `CoerceUnsized` may only be implemented \ - for a coercion between structures"); - return; - } - }; - - let mut fulfill_cx = traits::FulfillmentContext::new(); - - // Register an obligation for `A: Trait`. - let cause = traits::ObligationCause::misc(span, impl_node_id); - let predicate = - tcx.predicate_for_trait_def(cause, trait_def_id, 0, source, &[target]); - fulfill_cx.register_predicate_obligation(&infcx, predicate); - - // Check that all transitive obligations are satisfied. - if let Err(errors) = fulfill_cx.select_all_or_error(&infcx) { - infcx.report_fulfillment_errors(&errors); - } - - // Finally, resolve all regions. - let mut free_regions = FreeRegionMap::new(); - free_regions.relate_free_regions_from_predicates(&infcx.parameter_environment - .caller_bounds); - infcx.resolve_regions_and_report_errors(&free_regions, impl_node_id); - - if let Some(kind) = kind { - tcx.custom_coerce_unsized_kinds.borrow_mut().insert(impl_did, kind); - } - }); - }); - } } fn enforce_trait_manually_implementable(tcx: TyCtxt, sp: Span, trait_def_id: DefId) { From 7309babf1773cde3ee6d9e5ef2b177b44278a809 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 4 Dec 2016 00:58:13 +0200 Subject: [PATCH 2/7] coherence: code cleanup --- src/librustc_typeck/coherence/builtin.rs | 569 +++++++++++------------ src/librustc_typeck/coherence/mod.rs | 46 +- 2 files changed, 298 insertions(+), 317 deletions(-) diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index 6fde18dbf82a4..8a12eba7d5d20 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -22,336 +22,329 @@ use rustc::ty::subst::Subst; use rustc::ty::util::CopyImplementationError; use rustc::infer; +use rustc::hir::def_id::DefId; use rustc::hir::map as hir_map; use rustc::hir::{self, ItemImpl}; pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - populate_destructors(tcx); - check_implementations_of_copy(tcx); - check_implementations_of_coerce_unsized(tcx); -} + if let Some(drop_trait) = tcx.lang_items.drop_trait() { + tcx.lookup_trait_def(drop_trait).for_each_impl(tcx, |impl_did| { + visit_implementation_of_drop(tcx, impl_did) + }); + } -fn populate_destructors<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - let drop_trait = match tcx.lang_items.drop_trait() { - Some(id) => id, - None => return, - }; - tcx.populate_implementations_for_trait_if_necessary(drop_trait); - let drop_trait = tcx.lookup_trait_def(drop_trait); - - drop_trait.for_each_impl(tcx, |impl_did| { - let items = tcx.associated_item_def_ids(impl_did); - if items.is_empty() { - // We'll error out later. For now, just don't ICE. - return; - } - let method_def_id = items[0]; + if let Some(copy_trait) = tcx.lang_items.copy_trait() { + tcx.lookup_trait_def(copy_trait).for_each_impl(tcx, |impl_did| { + visit_implementation_of_copy(tcx, impl_did) + }); + } - let self_type = tcx.item_type(impl_did); - match self_type.sty { - ty::TyAdt(type_def, _) => { - type_def.set_destructor(method_def_id); + if let Some(coerce_unsized_trait) = tcx.lang_items.coerce_unsized_trait() { + let unsize_trait = match tcx.lang_items.require(UnsizeTraitLangItem) { + Ok(id) => id, + Err(err) => { + tcx.sess.fatal(&format!("`CoerceUnsized` implementation {}", err)); } - _ => { - // Destructors only work on nominal types. - if let Some(impl_node_id) = tcx.map.as_local_node_id(impl_did) { - match tcx.map.find(impl_node_id) { - Some(hir_map::NodeItem(item)) => { - let span = match item.node { - ItemImpl(.., ref ty, _) => ty.span, - _ => item.span, - }; - struct_span_err!(tcx.sess, - span, - E0120, - "the Drop trait may only be implemented on \ - structures") - .span_label(span, - &format!("implementing Drop requires a struct")) - .emit(); - } - _ => { - bug!("didn't find impl in ast map"); - } + }; + + tcx.lookup_trait_def(coerce_unsized_trait).for_each_impl(tcx, |impl_did| { + visit_implementation_of_coerce_unsized(tcx, impl_did, + unsize_trait, coerce_unsized_trait) + }); + } +} + +fn visit_implementation_of_drop<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId) { + let items = tcx.associated_item_def_ids(impl_did); + if items.is_empty() { + // We'll error out later. For now, just don't ICE. + return; + } + let method_def_id = items[0]; + + let self_type = tcx.item_type(impl_did); + match self_type.sty { + ty::TyAdt(type_def, _) => { + type_def.set_destructor(method_def_id); + } + _ => { + // Destructors only work on nominal types. + if let Some(impl_node_id) = tcx.map.as_local_node_id(impl_did) { + match tcx.map.find(impl_node_id) { + Some(hir_map::NodeItem(item)) => { + let span = match item.node { + ItemImpl(.., ref ty, _) => ty.span, + _ => item.span, + }; + struct_span_err!(tcx.sess, + span, + E0120, + "the Drop trait may only be implemented on \ + structures") + .span_label(span, + &format!("implementing Drop requires a struct")) + .emit(); + } + _ => { + bug!("didn't find impl in ast map"); } - } else { - bug!("found external impl of Drop trait on \ - something other than a struct"); } + } else { + bug!("found external impl of Drop trait on \ + something other than a struct"); } } - }); + } } -fn check_implementations_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - let copy_trait = match tcx.lang_items.copy_trait() { - Some(id) => id, - None => return, +fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId) { + debug!("visit_implementation_of_copy: impl_did={:?}", impl_did); + + let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { + n + } else { + debug!("visit_implementation_of_copy(): impl not in this \ + crate"); + return; }; - tcx.populate_implementations_for_trait_if_necessary(copy_trait); - let copy_trait = tcx.lookup_trait_def(copy_trait); - - copy_trait.for_each_impl(tcx, |impl_did| { - debug!("check_implementations_of_copy: impl_did={:?}", impl_did); - - let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { - n - } else { - debug!("check_implementations_of_copy(): impl not in this \ - crate"); - return; - }; - let self_type = tcx.item_type(impl_did); - debug!("check_implementations_of_copy: self_type={:?} (bound)", - self_type); - - let span = tcx.map.span(impl_node_id); - let param_env = ParameterEnvironment::for_item(tcx, impl_node_id); - let self_type = self_type.subst(tcx, ¶m_env.free_substs); - assert!(!self_type.has_escaping_regions()); - - debug!("check_implementations_of_copy: self_type={:?} (free)", - self_type); - - match param_env.can_type_implement_copy(tcx, self_type, span) { - Ok(()) => {} - Err(CopyImplementationError::InfrigingField(name)) => { - struct_span_err!(tcx.sess, - span, - E0204, - "the trait `Copy` may not be implemented for this type") - .span_label(span, &format!("field `{}` does not implement `Copy`", name)) - .emit() - } - Err(CopyImplementationError::InfrigingVariant(name)) => { - let item = tcx.map.expect_item(impl_node_id); - let span = if let ItemImpl(.., Some(ref tr), _, _) = item.node { - tr.path.span - } else { - span - }; - - struct_span_err!(tcx.sess, - span, - E0205, - "the trait `Copy` may not be implemented for this type") - .span_label(span, - &format!("variant `{}` does not implement `Copy`", name)) - .emit() - } - Err(CopyImplementationError::NotAnAdt) => { - let item = tcx.map.expect_item(impl_node_id); - let span = if let ItemImpl(.., ref ty, _) = item.node { - ty.span - } else { - span - }; - - struct_span_err!(tcx.sess, - span, - E0206, - "the trait `Copy` may not be implemented for this type") - .span_label(span, &format!("type is not a structure or enumeration")) - .emit(); - } - Err(CopyImplementationError::HasDestructor) => { - struct_span_err!(tcx.sess, - span, - E0184, - "the trait `Copy` may not be implemented for this type; the \ - type has a destructor") - .span_label(span, &format!("Copy not allowed on types with destructors")) - .emit(); - } + let self_type = tcx.item_type(impl_did); + debug!("visit_implementation_of_copy: self_type={:?} (bound)", + self_type); + + let span = tcx.map.span(impl_node_id); + let param_env = ParameterEnvironment::for_item(tcx, impl_node_id); + let self_type = self_type.subst(tcx, ¶m_env.free_substs); + assert!(!self_type.has_escaping_regions()); + + debug!("visit_implementation_of_copy: self_type={:?} (free)", + self_type); + + match param_env.can_type_implement_copy(tcx, self_type, span) { + Ok(()) => {} + Err(CopyImplementationError::InfrigingField(name)) => { + struct_span_err!(tcx.sess, + span, + E0204, + "the trait `Copy` may not be implemented for this type") + .span_label(span, &format!("field `{}` does not implement `Copy`", name)) + .emit() } - }); -} + Err(CopyImplementationError::InfrigingVariant(name)) => { + let item = tcx.map.expect_item(impl_node_id); + let span = if let ItemImpl(.., Some(ref tr), _, _) = item.node { + tr.path.span + } else { + span + }; -fn check_implementations_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - let coerce_unsized_trait = match tcx.lang_items.coerce_unsized_trait() { - Some(id) => id, - None => return, - }; - let unsize_trait = match tcx.lang_items.require(UnsizeTraitLangItem) { - Ok(id) => id, - Err(err) => { - tcx.sess.fatal(&format!("`CoerceUnsized` implementation {}", err)); + struct_span_err!(tcx.sess, + span, + E0205, + "the trait `Copy` may not be implemented for this type") + .span_label(span, + &format!("variant `{}` does not implement `Copy`", name)) + .emit() } - }; + Err(CopyImplementationError::NotAnAdt) => { + let item = tcx.map.expect_item(impl_node_id); + let span = if let ItemImpl(.., ref ty, _) = item.node { + ty.span + } else { + span + }; - let trait_def = tcx.lookup_trait_def(coerce_unsized_trait); + struct_span_err!(tcx.sess, + span, + E0206, + "the trait `Copy` may not be implemented for this type") + .span_label(span, &format!("type is not a structure or enumeration")) + .emit(); + } + Err(CopyImplementationError::HasDestructor) => { + struct_span_err!(tcx.sess, + span, + E0184, + "the trait `Copy` may not be implemented for this type; the \ + type has a destructor") + .span_label(span, &format!("Copy not allowed on types with destructors")) + .emit(); + } + } +} - trait_def.for_each_impl(tcx, |impl_did| { - debug!("check_implementations_of_coerce_unsized: impl_did={:?}", - impl_did); +fn visit_implementation_of_coerce_unsized<'a, 'tcx>( + tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId, + unsize_trait: DefId, coerce_unsized_trait: DefId) +{ + debug!("visit_implementation_of_coerce_unsized: impl_did={:?}", + impl_did); + + let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { + n + } else { + debug!("visit_implementation_of_coerce_unsized(): impl not \ + in this crate"); + return; + }; - let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { - n - } else { - debug!("check_implementations_of_coerce_unsized(): impl not \ - in this crate"); - return; + let source = tcx.item_type(impl_did); + let trait_ref = tcx.impl_trait_ref(impl_did).unwrap(); + let target = trait_ref.substs.type_at(1); + debug!("visit_implementation_of_coerce_unsized: {:?} -> {:?} (bound)", + source, + target); + + let span = tcx.map.span(impl_node_id); + let param_env = ParameterEnvironment::for_item(tcx, impl_node_id); + let source = source.subst(tcx, ¶m_env.free_substs); + let target = target.subst(tcx, ¶m_env.free_substs); + assert!(!source.has_escaping_regions()); + + debug!("visit_implementation_of_coerce_unsized: {:?} -> {:?} (free)", + source, + target); + + tcx.infer_ctxt(None, Some(param_env), Reveal::ExactMatch).enter(|infcx| { + let cause = ObligationCause::misc(span, impl_node_id); + let check_mutbl = |mt_a: ty::TypeAndMut<'tcx>, + mt_b: ty::TypeAndMut<'tcx>, + mk_ptr: &Fn(Ty<'tcx>) -> Ty<'tcx>| { + if (mt_a.mutbl, mt_b.mutbl) == (hir::MutImmutable, hir::MutMutable) { + infcx.report_mismatched_types(&cause, + mk_ptr(mt_b.ty), + target, + ty::error::TypeError::Mutability) + .emit(); + } + (mt_a.ty, mt_b.ty, unsize_trait, None) }; + let (source, target, trait_def_id, kind) = match (&source.sty, &target.sty) { + (&ty::TyBox(a), &ty::TyBox(b)) => (a, b, unsize_trait, None), - let source = tcx.item_type(impl_did); - let trait_ref = tcx.impl_trait_ref(impl_did).unwrap(); - let target = trait_ref.substs.type_at(1); - debug!("check_implementations_of_coerce_unsized: {:?} -> {:?} (bound)", - source, - target); - - let span = tcx.map.span(impl_node_id); - let param_env = ParameterEnvironment::for_item(tcx, impl_node_id); - let source = source.subst(tcx, ¶m_env.free_substs); - let target = target.subst(tcx, ¶m_env.free_substs); - assert!(!source.has_escaping_regions()); - - debug!("check_implementations_of_coerce_unsized: {:?} -> {:?} (free)", - source, - target); - - tcx.infer_ctxt(None, Some(param_env), Reveal::ExactMatch).enter(|infcx| { - let cause = ObligationCause::misc(span, impl_node_id); - let check_mutbl = |mt_a: ty::TypeAndMut<'tcx>, - mt_b: ty::TypeAndMut<'tcx>, - mk_ptr: &Fn(Ty<'tcx>) -> Ty<'tcx>| { - if (mt_a.mutbl, mt_b.mutbl) == (hir::MutImmutable, hir::MutMutable) { - infcx.report_mismatched_types(&cause, - mk_ptr(mt_b.ty), - target, - ty::error::TypeError::Mutability) - .emit(); - } - (mt_a.ty, mt_b.ty, unsize_trait, None) - }; - let (source, target, trait_def_id, kind) = match (&source.sty, &target.sty) { - (&ty::TyBox(a), &ty::TyBox(b)) => (a, b, unsize_trait, None), + (&ty::TyRef(r_a, mt_a), &ty::TyRef(r_b, mt_b)) => { + infcx.sub_regions(infer::RelateObjectBound(span), r_b, r_a); + check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ref(r_b, ty)) + } - (&ty::TyRef(r_a, mt_a), &ty::TyRef(r_b, mt_b)) => { - infcx.sub_regions(infer::RelateObjectBound(span), r_b, r_a); - check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ref(r_b, ty)) - } + (&ty::TyRef(_, mt_a), &ty::TyRawPtr(mt_b)) | + (&ty::TyRawPtr(mt_a), &ty::TyRawPtr(mt_b)) => { + check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ptr(ty)) + } - (&ty::TyRef(_, mt_a), &ty::TyRawPtr(mt_b)) | - (&ty::TyRawPtr(mt_a), &ty::TyRawPtr(mt_b)) => { - check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ptr(ty)) + (&ty::TyAdt(def_a, substs_a), &ty::TyAdt(def_b, substs_b)) + if def_a.is_struct() && def_b.is_struct() => { + if def_a != def_b { + let source_path = tcx.item_path_str(def_a.did); + let target_path = tcx.item_path_str(def_b.did); + span_err!(tcx.sess, + span, + E0377, + "the trait `CoerceUnsized` may only be implemented \ + for a coercion between structures with the same \ + definition; expected {}, found {}", + source_path, + target_path); + return; } - (&ty::TyAdt(def_a, substs_a), &ty::TyAdt(def_b, substs_b)) - if def_a.is_struct() && def_b.is_struct() => { - if def_a != def_b { - let source_path = tcx.item_path_str(def_a.did); - let target_path = tcx.item_path_str(def_b.did); - span_err!(tcx.sess, - span, - E0377, - "the trait `CoerceUnsized` may only be implemented \ - for a coercion between structures with the same \ - definition; expected {}, found {}", - source_path, - target_path); - return; - } + let fields = &def_a.struct_variant().fields; + let diff_fields = fields.iter() + .enumerate() + .filter_map(|(i, f)| { + let (a, b) = (f.ty(tcx, substs_a), f.ty(tcx, substs_b)); - let fields = &def_a.struct_variant().fields; - let diff_fields = fields.iter() - .enumerate() - .filter_map(|(i, f)| { - let (a, b) = (f.ty(tcx, substs_a), f.ty(tcx, substs_b)); + if tcx.item_type(f.did).is_phantom_data() { + // Ignore PhantomData fields + return None; + } - if tcx.item_type(f.did).is_phantom_data() { - // Ignore PhantomData fields + // Ignore fields that aren't significantly changed + if let Ok(ok) = infcx.sub_types(false, &cause, b, a) { + if ok.obligations.is_empty() { return None; } + } - // Ignore fields that aren't significantly changed - if let Ok(ok) = infcx.sub_types(false, &cause, b, a) { - if ok.obligations.is_empty() { - return None; - } - } - - // Collect up all fields that were significantly changed - // i.e. those that contain T in coerce_unsized T -> U - Some((i, a, b)) - }) - .collect::>(); - - if diff_fields.is_empty() { - span_err!(tcx.sess, - span, - E0374, - "the trait `CoerceUnsized` may only be implemented \ - for a coercion between structures with one field \ - being coerced, none found"); - return; - } else if diff_fields.len() > 1 { - let item = tcx.map.expect_item(impl_node_id); - let span = if let ItemImpl(.., Some(ref t), _, _) = item.node { - t.path.span - } else { - tcx.map.span(impl_node_id) - }; - - let mut err = struct_span_err!(tcx.sess, - span, - E0375, - "implementing the trait \ - `CoerceUnsized` requires multiple \ - coercions"); - err.note("`CoerceUnsized` may only be implemented for \ - a coercion between structures with one field being coerced"); - err.note(&format!("currently, {} fields need coercions: {}", - diff_fields.len(), - diff_fields.iter() - .map(|&(i, a, b)| { - format!("{} ({} to {})", fields[i].name, a, b) - }) - .collect::>() - .join(", "))); - err.span_label(span, &format!("requires multiple coercions")); - err.emit(); - return; - } + // Collect up all fields that were significantly changed + // i.e. those that contain T in coerce_unsized T -> U + Some((i, a, b)) + }) + .collect::>(); - let (i, a, b) = diff_fields[0]; - let kind = ty::adjustment::CustomCoerceUnsized::Struct(i); - (a, b, coerce_unsized_trait, Some(kind)) - } - - _ => { + if diff_fields.is_empty() { span_err!(tcx.sess, span, - E0376, + E0374, "the trait `CoerceUnsized` may only be implemented \ - for a coercion between structures"); + for a coercion between structures with one field \ + being coerced, none found"); + return; + } else if diff_fields.len() > 1 { + let item = tcx.map.expect_item(impl_node_id); + let span = if let ItemImpl(.., Some(ref t), _, _) = item.node { + t.path.span + } else { + tcx.map.span(impl_node_id) + }; + + let mut err = struct_span_err!(tcx.sess, + span, + E0375, + "implementing the trait \ + `CoerceUnsized` requires multiple \ + coercions"); + err.note("`CoerceUnsized` may only be implemented for \ + a coercion between structures with one field being coerced"); + err.note(&format!("currently, {} fields need coercions: {}", + diff_fields.len(), + diff_fields.iter() + .map(|&(i, a, b)| { + format!("{} ({} to {})", fields[i].name, a, b) + }) + .collect::>() + .join(", "))); + err.span_label(span, &format!("requires multiple coercions")); + err.emit(); return; } - }; - - let mut fulfill_cx = traits::FulfillmentContext::new(); - // Register an obligation for `A: Trait`. - let cause = traits::ObligationCause::misc(span, impl_node_id); - let predicate = - tcx.predicate_for_trait_def(cause, trait_def_id, 0, source, &[target]); - fulfill_cx.register_predicate_obligation(&infcx, predicate); + let (i, a, b) = diff_fields[0]; + let kind = ty::adjustment::CustomCoerceUnsized::Struct(i); + (a, b, coerce_unsized_trait, Some(kind)) + } - // Check that all transitive obligations are satisfied. - if let Err(errors) = fulfill_cx.select_all_or_error(&infcx) { - infcx.report_fulfillment_errors(&errors); + _ => { + span_err!(tcx.sess, + span, + E0376, + "the trait `CoerceUnsized` may only be implemented \ + for a coercion between structures"); + return; } + }; - // Finally, resolve all regions. - let mut free_regions = FreeRegionMap::new(); - free_regions.relate_free_regions_from_predicates(&infcx.parameter_environment - .caller_bounds); - infcx.resolve_regions_and_report_errors(&free_regions, impl_node_id); + let mut fulfill_cx = traits::FulfillmentContext::new(); - if let Some(kind) = kind { - tcx.custom_coerce_unsized_kinds.borrow_mut().insert(impl_did, kind); - } - }); + // Register an obligation for `A: Trait`. + let cause = traits::ObligationCause::misc(span, impl_node_id); + let predicate = + tcx.predicate_for_trait_def(cause, trait_def_id, 0, source, &[target]); + fulfill_cx.register_predicate_obligation(&infcx, predicate); + + // Check that all transitive obligations are satisfied. + if let Err(errors) = fulfill_cx.select_all_or_error(&infcx) { + infcx.report_fulfillment_errors(&errors); + } + + // Finally, resolve all regions. + let mut free_regions = FreeRegionMap::new(); + free_regions.relate_free_regions_from_predicates(&infcx.parameter_environment + .caller_bounds); + infcx.resolve_regions_and_report_errors(&free_regions, impl_node_id); + + if let Some(kind) = kind { + tcx.custom_coerce_unsized_kinds.borrow_mut().insert(impl_did, kind); + } }); } diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 3293818348a94..316e6555b41ec 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -16,7 +16,6 @@ // mappings. That mapping code resides here. use hir::def_id::DefId; -use rustc::traits::Reveal; use rustc::ty::{self, TyCtxt, TypeFoldable}; use rustc::ty::{Ty, TyBool, TyChar, TyError}; use rustc::ty::{TyParam, TyRawPtr}; @@ -25,7 +24,6 @@ use rustc::ty::{TyStr, TyArray, TySlice, TyFloat, TyInfer, TyInt}; use rustc::ty::{TyUint, TyClosure, TyBox, TyFnDef, TyFnPtr}; use rustc::ty::{TyProjection, TyAnon}; use CrateCtxt; -use rustc::infer::{InferCtxt}; use syntax_pos::Span; use rustc::dep_graph::DepNode; use rustc::hir::itemlikevisit::ItemLikeVisitor; @@ -37,16 +35,15 @@ mod orphan; mod overlap; mod unsafety; -struct CoherenceChecker<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { - crate_context: &'a CrateCtxt<'a, 'gcx>, - inference_context: InferCtxt<'a, 'gcx, 'tcx>, +struct CoherenceChecker<'a, 'tcx: 'a> { + tcx: TyCtxt<'a, 'tcx, 'tcx>, } -struct CoherenceCheckVisitor<'a, 'gcx: 'a + 'tcx, 'tcx: 'a> { - cc: &'a CoherenceChecker<'a, 'gcx, 'tcx>, +struct CoherenceCheckVisitor<'a, 'tcx: 'a> { + cc: &'a CoherenceChecker<'a, 'tcx>, } -impl<'a, 'gcx, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceCheckVisitor<'a, 'gcx, 'tcx> { +impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceCheckVisitor<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { if let ItemImpl(..) = item.node { self.cc.check_implementation(item) @@ -60,7 +57,7 @@ impl<'a, 'gcx, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceCheckVisitor<'a, 'gcx, } } -impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { +impl<'a, 'tcx> CoherenceChecker<'a, 'tcx> { // Returns the def ID of the base type, if there is one. fn get_base_type_def_id(&self, span: Span, ty: Ty<'tcx>) -> Option { match ty.sty { @@ -68,7 +65,7 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { TyDynamic(ref t, ..) => t.principal().map(|p| p.def_id()), - TyBox(_) => self.inference_context.tcx.lang_items.owned_box(), + TyBox(_) => self.tcx.lang_items.owned_box(), TyBool | TyChar | TyInt(..) | TyUint(..) | TyFloat(..) | TyStr | TyArray(..) | TySlice(..) | TyFnDef(..) | TyFnPtr(_) | TyTuple(..) | TyParam(..) | TyError | @@ -88,21 +85,21 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { // Check implementations and traits. This populates the tables // containing the inherent methods and extension methods. It also // builds up the trait inheritance table. - self.crate_context.tcx.visit_all_item_likes_in_krate( + self.tcx.visit_all_item_likes_in_krate( DepNode::CoherenceCheckImpl, &mut CoherenceCheckVisitor { cc: self }); - builtin::check(self.crate_context.tcx); + builtin::check(self.tcx); } fn check_implementation(&self, item: &Item) { - let tcx = self.crate_context.tcx; + let tcx = self.tcx; let impl_did = tcx.map.local_def_id(item.id); let self_type = tcx.item_type(impl_did); // If there are no traits, then this implementation must have a // base type. - if let Some(trait_ref) = self.crate_context.tcx.impl_trait_ref(impl_did) { + if let Some(trait_ref) = self.tcx.impl_trait_ref(impl_did) { debug!("(checking implementation) adding impl for trait '{:?}', item '{}'", trait_ref, item.name); @@ -113,9 +110,7 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { return; } - enforce_trait_manually_implementable(self.crate_context.tcx, - item.span, - trait_ref.def_id); + enforce_trait_manually_implementable(self.tcx, item.span, trait_ref.def_id); self.add_trait_impl(trait_ref, impl_did); } else { // Skip inherent impls where the self type is an error @@ -134,16 +129,15 @@ impl<'a, 'gcx, 'tcx> CoherenceChecker<'a, 'gcx, 'tcx> { } fn add_inherent_impl(&self, base_def_id: DefId, impl_def_id: DefId) { - let tcx = self.crate_context.tcx; - tcx.inherent_impls.borrow_mut().push(base_def_id, impl_def_id); + self.tcx.inherent_impls.borrow_mut().push(base_def_id, impl_def_id); } - fn add_trait_impl(&self, impl_trait_ref: ty::TraitRef<'gcx>, impl_def_id: DefId) { + fn add_trait_impl(&self, impl_trait_ref: ty::TraitRef<'tcx>, impl_def_id: DefId) { debug!("add_trait_impl: impl_trait_ref={:?} impl_def_id={:?}", impl_trait_ref, impl_def_id); - let trait_def = self.crate_context.tcx.lookup_trait_def(impl_trait_ref.def_id); - trait_def.record_local_impl(self.crate_context.tcx, impl_def_id, impl_trait_ref); + let trait_def = self.tcx.lookup_trait_def(impl_trait_ref.def_id); + trait_def.record_local_impl(self.tcx, impl_def_id, impl_trait_ref); } } @@ -176,13 +170,7 @@ fn enforce_trait_manually_implementable(tcx: TyCtxt, sp: Span, trait_def_id: Def pub fn check_coherence(ccx: &CrateCtxt) { let _task = ccx.tcx.dep_graph.in_task(DepNode::Coherence); - ccx.tcx.infer_ctxt(None, None, Reveal::ExactMatch).enter(|infcx| { - CoherenceChecker { - crate_context: ccx, - inference_context: infcx, - } - .check(); - }); + CoherenceChecker { tcx: ccx.tcx }.check(); unsafety::check(ccx.tcx); orphan::check(ccx.tcx); overlap::check(ccx.tcx); From d938ba47699a1539b00cbc59e6fc647010df340a Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 4 Dec 2016 01:32:49 +0200 Subject: [PATCH 3/7] coherence: check builtin impls after the specialization graph is ready Fixes #33187. --- src/librustc_typeck/coherence/mod.rs | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/librustc_typeck/coherence/mod.rs b/src/librustc_typeck/coherence/mod.rs index 316e6555b41ec..6cf752dd69fca 100644 --- a/src/librustc_typeck/coherence/mod.rs +++ b/src/librustc_typeck/coherence/mod.rs @@ -39,14 +39,10 @@ struct CoherenceChecker<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, } -struct CoherenceCheckVisitor<'a, 'tcx: 'a> { - cc: &'a CoherenceChecker<'a, 'tcx>, -} - -impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceCheckVisitor<'a, 'tcx> { +impl<'a, 'tcx, 'v> ItemLikeVisitor<'v> for CoherenceChecker<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { if let ItemImpl(..) = item.node { - self.cc.check_implementation(item) + self.check_implementation(item) } } @@ -81,14 +77,11 @@ impl<'a, 'tcx> CoherenceChecker<'a, 'tcx> { } } - fn check(&self) { + fn check(&mut self) { // Check implementations and traits. This populates the tables // containing the inherent methods and extension methods. It also // builds up the trait inheritance table. - self.tcx.visit_all_item_likes_in_krate( - DepNode::CoherenceCheckImpl, - &mut CoherenceCheckVisitor { cc: self }); - builtin::check(self.tcx); + self.tcx.visit_all_item_likes_in_krate(DepNode::CoherenceCheckImpl, self); } fn check_implementation(&self, item: &Item) { @@ -174,4 +167,5 @@ pub fn check_coherence(ccx: &CrateCtxt) { unsafety::check(ccx.tcx); orphan::check(ccx.tcx); overlap::check(ccx.tcx); + builtin::check(ccx.tcx); } From 243e45aac3fccf9403ad1bed162073f4f314b270 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 4 Dec 2016 02:04:08 +0200 Subject: [PATCH 4/7] normalize field types in copy implementations Fixes #34377. --- src/librustc/ty/util.rs | 52 ++++++++++++++++++-------------- src/test/run-pass/issue-33187.rs | 27 +++++++++++++++++ 2 files changed, 56 insertions(+), 23 deletions(-) create mode 100644 src/test/run-pass/issue-33187.rs diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 0b45ff94a9312..34c07d442e33d 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -147,34 +147,40 @@ impl<'tcx> ParameterEnvironment<'tcx> { self_type: Ty<'tcx>, span: Span) -> Result<(),CopyImplementationError> { // FIXME: (@jroesch) float this code up - tcx.infer_ctxt(None, Some(self.clone()), Reveal::ExactMatch).enter(|infcx| { - let adt = match self_type.sty { - ty::TyAdt(adt, substs) => match adt.adt_kind() { - AdtKind::Struct | AdtKind::Union => { - for field in adt.all_fields() { - let field_ty = field.ty(tcx, substs); - if infcx.type_moves_by_default(field_ty, span) { - return Err(CopyImplementationError::InfrigingField( - field.name)) - } + tcx.infer_ctxt(None, Some(self.clone()), Reveal::NotSpecializable).enter(|infcx| { + let (adt, substs) = match self_type.sty { + ty::TyAdt(adt, substs) => (adt, substs), + _ => return Err(CopyImplementationError::NotAnAdt) + }; + + let field_implements_copy = |field: &ty::FieldDef| { + let cause = traits::ObligationCause::dummy(); + match traits::fully_normalize(&infcx, cause, &field.ty(tcx, substs)) { + Ok(ty) => !infcx.type_moves_by_default(ty, span), + Err(..) => false + } + }; + + match adt.adt_kind() { + AdtKind::Struct | AdtKind::Union => { + for field in adt.all_fields() { + if !field_implements_copy(field) { + return Err(CopyImplementationError::InfrigingField( + field.name)) } - adt } - AdtKind::Enum => { - for variant in &adt.variants { - for field in &variant.fields { - let field_ty = field.ty(tcx, substs); - if infcx.type_moves_by_default(field_ty, span) { - return Err(CopyImplementationError::InfrigingVariant( - variant.name)) - } + } + AdtKind::Enum => { + for variant in &adt.variants { + for field in &variant.fields { + if !field_implements_copy(field) { + return Err(CopyImplementationError::InfrigingVariant( + variant.name)) } } - adt } - }, - _ => return Err(CopyImplementationError::NotAnAdt) - }; + } + } if adt.has_dtor() { return Err(CopyImplementationError::HasDestructor); diff --git a/src/test/run-pass/issue-33187.rs b/src/test/run-pass/issue-33187.rs new file mode 100644 index 0000000000000..477112ab3c5ab --- /dev/null +++ b/src/test/run-pass/issue-33187.rs @@ -0,0 +1,27 @@ +// Copyright 2016 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct Foo(::Data); + +impl Copy for Foo where ::Data: Copy { } +impl Clone for Foo where ::Data: Clone { + fn clone(&self) -> Self { Foo(self.0.clone()) } +} + +trait Repr { + type Data; +} + +impl Repr for A { + type Data = u32; +} + +fn main() { +} From e41920a1c304487e64506adf27100a224c6ef6e6 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 3 Jan 2017 21:51:28 +0200 Subject: [PATCH 5/7] rustfmt coherence::builtin --- src/librustc_typeck/coherence/builtin.rs | 54 ++++++++++++------------ 1 file changed, 26 insertions(+), 28 deletions(-) diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index 8a12eba7d5d20..d6eb7d4b183c0 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -28,15 +28,13 @@ use rustc::hir::{self, ItemImpl}; pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { if let Some(drop_trait) = tcx.lang_items.drop_trait() { - tcx.lookup_trait_def(drop_trait).for_each_impl(tcx, |impl_did| { - visit_implementation_of_drop(tcx, impl_did) - }); + tcx.lookup_trait_def(drop_trait) + .for_each_impl(tcx, |impl_did| visit_implementation_of_drop(tcx, impl_did)); } if let Some(copy_trait) = tcx.lang_items.copy_trait() { - tcx.lookup_trait_def(copy_trait).for_each_impl(tcx, |impl_did| { - visit_implementation_of_copy(tcx, impl_did) - }); + tcx.lookup_trait_def(copy_trait) + .for_each_impl(tcx, |impl_did| visit_implementation_of_copy(tcx, impl_did)); } if let Some(coerce_unsized_trait) = tcx.lang_items.coerce_unsized_trait() { @@ -48,8 +46,10 @@ pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { }; tcx.lookup_trait_def(coerce_unsized_trait).for_each_impl(tcx, |impl_did| { - visit_implementation_of_coerce_unsized(tcx, impl_did, - unsize_trait, coerce_unsized_trait) + visit_implementation_of_coerce_unsized(tcx, + impl_did, + unsize_trait, + coerce_unsized_trait) }); } } @@ -81,8 +81,7 @@ fn visit_implementation_of_drop<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: E0120, "the Drop trait may only be implemented on \ structures") - .span_label(span, - &format!("implementing Drop requires a struct")) + .span_label(span, &format!("implementing Drop requires a struct")) .emit(); } _ => { @@ -173,10 +172,10 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: } } -fn visit_implementation_of_coerce_unsized<'a, 'tcx>( - tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId, - unsize_trait: DefId, coerce_unsized_trait: DefId) -{ +fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + impl_did: DefId, + unsize_trait: DefId, + coerce_unsized_trait: DefId) { debug!("visit_implementation_of_coerce_unsized: impl_did={:?}", impl_did); @@ -212,10 +211,10 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>( mk_ptr: &Fn(Ty<'tcx>) -> Ty<'tcx>| { if (mt_a.mutbl, mt_b.mutbl) == (hir::MutImmutable, hir::MutMutable) { infcx.report_mismatched_types(&cause, - mk_ptr(mt_b.ty), - target, - ty::error::TypeError::Mutability) - .emit(); + mk_ptr(mt_b.ty), + target, + ty::error::TypeError::Mutability) + .emit(); } (mt_a.ty, mt_b.ty, unsize_trait, None) }; @@ -232,8 +231,8 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>( check_mutbl(mt_a, mt_b, &|ty| tcx.mk_imm_ptr(ty)) } - (&ty::TyAdt(def_a, substs_a), &ty::TyAdt(def_b, substs_b)) - if def_a.is_struct() && def_b.is_struct() => { + (&ty::TyAdt(def_a, substs_a), &ty::TyAdt(def_b, substs_b)) if def_a.is_struct() && + def_b.is_struct() => { if def_a != def_b { let source_path = tcx.item_path_str(def_a.did); let target_path = tcx.item_path_str(def_b.did); @@ -299,11 +298,11 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>( err.note(&format!("currently, {} fields need coercions: {}", diff_fields.len(), diff_fields.iter() - .map(|&(i, a, b)| { - format!("{} ({} to {})", fields[i].name, a, b) - }) - .collect::>() - .join(", "))); + .map(|&(i, a, b)| { + format!("{} ({} to {})", fields[i].name, a, b) + }) + .collect::>() + .join(", "))); err.span_label(span, &format!("requires multiple coercions")); err.emit(); return; @@ -328,8 +327,7 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>( // Register an obligation for `A: Trait`. let cause = traits::ObligationCause::misc(span, impl_node_id); - let predicate = - tcx.predicate_for_trait_def(cause, trait_def_id, 0, source, &[target]); + let predicate = tcx.predicate_for_trait_def(cause, trait_def_id, 0, source, &[target]); fulfill_cx.register_predicate_obligation(&infcx, predicate); // Check that all transitive obligations are satisfied. @@ -340,7 +338,7 @@ fn visit_implementation_of_coerce_unsized<'a, 'tcx>( // Finally, resolve all regions. let mut free_regions = FreeRegionMap::new(); free_regions.relate_free_regions_from_predicates(&infcx.parameter_environment - .caller_bounds); + .caller_bounds); infcx.resolve_regions_and_report_errors(&free_regions, impl_node_id); if let Some(kind) = kind { From 4cab2931c89fe09dfa295445ad491c2ece7e7df1 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Tue, 3 Jan 2017 23:54:12 +0200 Subject: [PATCH 6/7] simplify Copy implementation error reporting Span the affected fields instead of reporting the field/variant name. --- src/libcore/marker.rs | 6 ++-- src/librustc/ty/util.rs | 30 +++++----------- src/librustc_typeck/coherence/builtin.rs | 17 +++------ src/librustc_typeck/diagnostics.rs | 2 ++ src/test/compile-fail/E0205.rs | 30 ---------------- src/test/{compile-fail => ui/span}/E0204.rs | 18 +++++++--- src/test/ui/span/E0204.stderr | 38 +++++++++++++++++++++ 7 files changed, 68 insertions(+), 73 deletions(-) delete mode 100644 src/test/compile-fail/E0205.rs rename src/test/{compile-fail => ui/span}/E0204.rs (77%) create mode 100644 src/test/ui/span/E0204.stderr diff --git a/src/libcore/marker.rs b/src/libcore/marker.rs index 9af10966eda4b..ed01b93f1335a 100644 --- a/src/libcore/marker.rs +++ b/src/libcore/marker.rs @@ -234,12 +234,10 @@ pub trait Unsize { /// Generalizing the latter case, any type implementing [`Drop`] can't be `Copy`, because it's /// managing some resource besides its own [`size_of::()`] bytes. /// -/// If you try to implement `Copy` on a struct or enum containing non-`Copy` data, you will get a -/// compile-time error. Specifically, with structs you'll get [E0204] and with enums you'll get -/// [E0205]. +/// If you try to implement `Copy` on a struct or enum containing non-`Copy` data, you will get +/// the error [E0204]. /// /// [E0204]: ../../error-index.html#E0204 -/// [E0205]: ../../error-index.html#E0205 /// /// ## When *should* my type be `Copy`? /// diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index 34c07d442e33d..0b1030f74b0fd 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -15,7 +15,7 @@ use hir::map::DefPathData; use infer::InferCtxt; use hir::map as ast_map; use traits::{self, Reveal}; -use ty::{self, Ty, AdtKind, TyCtxt, TypeAndMut, TypeFlags, TypeFoldable}; +use ty::{self, Ty, TyCtxt, TypeAndMut, TypeFlags, TypeFoldable}; use ty::{Disr, ParameterEnvironment}; use ty::fold::TypeVisitor; use ty::layout::{Layout, LayoutError}; @@ -120,9 +120,8 @@ impl IntTypeExt for attr::IntType { #[derive(Copy, Clone)] -pub enum CopyImplementationError { - InfrigingField(Name), - InfrigingVariant(Name), +pub enum CopyImplementationError<'tcx> { + InfrigingField(&'tcx ty::FieldDef), NotAnAdt, HasDestructor } @@ -145,7 +144,7 @@ pub enum Representability { impl<'tcx> ParameterEnvironment<'tcx> { pub fn can_type_implement_copy<'a>(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>, self_type: Ty<'tcx>, span: Span) - -> Result<(),CopyImplementationError> { + -> Result<(), CopyImplementationError> { // FIXME: (@jroesch) float this code up tcx.infer_ctxt(None, Some(self.clone()), Reveal::NotSpecializable).enter(|infcx| { let (adt, substs) = match self_type.sty { @@ -161,23 +160,10 @@ impl<'tcx> ParameterEnvironment<'tcx> { } }; - match adt.adt_kind() { - AdtKind::Struct | AdtKind::Union => { - for field in adt.all_fields() { - if !field_implements_copy(field) { - return Err(CopyImplementationError::InfrigingField( - field.name)) - } - } - } - AdtKind::Enum => { - for variant in &adt.variants { - for field in &variant.fields { - if !field_implements_copy(field) { - return Err(CopyImplementationError::InfrigingVariant( - variant.name)) - } - } + for variant in &adt.variants { + for field in &variant.fields { + if !field_implements_copy(field) { + return Err(CopyImplementationError::InfrigingField(field)); } } } diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index d6eb7d4b183c0..d067cb99aa096 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -121,15 +121,7 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: match param_env.can_type_implement_copy(tcx, self_type, span) { Ok(()) => {} - Err(CopyImplementationError::InfrigingField(name)) => { - struct_span_err!(tcx.sess, - span, - E0204, - "the trait `Copy` may not be implemented for this type") - .span_label(span, &format!("field `{}` does not implement `Copy`", name)) - .emit() - } - Err(CopyImplementationError::InfrigingVariant(name)) => { + Err(CopyImplementationError::InfrigingField(field)) => { let item = tcx.map.expect_item(impl_node_id); let span = if let ItemImpl(.., Some(ref tr), _, _) = item.node { tr.path.span @@ -139,10 +131,11 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: struct_span_err!(tcx.sess, span, - E0205, + E0204, "the trait `Copy` may not be implemented for this type") - .span_label(span, - &format!("variant `{}` does not implement `Copy`", name)) + .span_label( + tcx.def_span(field.did), + &"this field does not implement `Copy`") .emit() } Err(CopyImplementationError::NotAnAdt) => { diff --git a/src/librustc_typeck/diagnostics.rs b/src/librustc_typeck/diagnostics.rs index d3b671f2a4d6e..1a971be64d819 100644 --- a/src/librustc_typeck/diagnostics.rs +++ b/src/librustc_typeck/diagnostics.rs @@ -2300,6 +2300,7 @@ This fails because `&mut T` is not `Copy`, even when `T` is `Copy` (this differs from the behavior for `&T`, which is always `Copy`). "##, +/* E0205: r##" An attempt to implement the `Copy` trait for an enum failed because one of the variants does not implement `Copy`. To fix this, you must implement `Copy` for @@ -2329,6 +2330,7 @@ enum Foo<'a> { This fails because `&mut T` is not `Copy`, even when `T` is `Copy` (this differs from the behavior for `&T`, which is always `Copy`). "##, +*/ E0206: r##" You can only implement `Copy` for a struct or enum. Both of the following diff --git a/src/test/compile-fail/E0205.rs b/src/test/compile-fail/E0205.rs deleted file mode 100644 index c73e753430105..0000000000000 --- a/src/test/compile-fail/E0205.rs +++ /dev/null @@ -1,30 +0,0 @@ -// Copyright 2016 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 or the MIT license -// , at your -// option. This file may not be copied, modified, or distributed -// except according to those terms. - -enum Foo { - Bar(Vec), - Baz, -} - -impl Copy for Foo { } -//~^ ERROR the trait `Copy` may not be implemented for this type -//~| NOTE variant `Bar` does not implement `Copy` - -#[derive(Copy)] -//~^ ERROR the trait `Copy` may not be implemented for this type -//~| NOTE variant `Bar` does not implement `Copy` -//~| NOTE in this expansion of #[derive(Copy)] -enum Foo2<'a> { - Bar(&'a mut bool), - Baz, -} - -fn main() { -} diff --git a/src/test/compile-fail/E0204.rs b/src/test/ui/span/E0204.rs similarity index 77% rename from src/test/compile-fail/E0204.rs rename to src/test/ui/span/E0204.rs index 0f108a17c95db..9fb37607c7da7 100644 --- a/src/test/compile-fail/E0204.rs +++ b/src/test/ui/span/E0204.rs @@ -13,16 +13,24 @@ struct Foo { } impl Copy for Foo { } -//~^ ERROR E0204 -//~| NOTE field `foo` does not implement `Copy` #[derive(Copy)] -//~^ ERROR E0204 -//~| NOTE field `ty` does not implement `Copy` -//~| NOTE in this expansion of #[derive(Copy)] struct Foo2<'a> { ty: &'a mut bool, } +enum EFoo { + Bar { x: Vec }, + Baz, +} + +impl Copy for EFoo { } + +#[derive(Copy)] +enum EFoo2<'a> { + Bar(&'a mut bool), + Baz, +} + fn main() { } diff --git a/src/test/ui/span/E0204.stderr b/src/test/ui/span/E0204.stderr new file mode 100644 index 0000000000000..ae543ed1a5d53 --- /dev/null +++ b/src/test/ui/span/E0204.stderr @@ -0,0 +1,38 @@ +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/E0204.rs:29:10 + | +29 | #[derive(Copy)] + | ^^^^ +30 | enum EFoo2<'a> { +31 | Bar(&'a mut bool), + | ------------- this field does not implement `Copy` + +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/E0204.rs:17:10 + | +17 | #[derive(Copy)] + | ^^^^ +18 | struct Foo2<'a> { +19 | ty: &'a mut bool, + | ---------------- this field does not implement `Copy` + +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/E0204.rs:27:6 + | +23 | Bar { x: Vec }, + | ----------- this field does not implement `Copy` +... +27 | impl Copy for EFoo { } + | ^^^^ + +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/E0204.rs:15:6 + | +12 | foo: Vec, + | ------------- this field does not implement `Copy` +... +15 | impl Copy for Foo { } + | ^^^^ + +error: aborting due to 4 previous errors + From 5fad51e7f42fc61d6e507dc3a17787534b4acbcc Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Wed, 4 Jan 2017 11:54:57 +0200 Subject: [PATCH 7/7] typeck::coherence::builtin - sort impls in the DefId order this makes error messages consistent across architectures --- src/librustc_typeck/coherence/builtin.rs | 62 +++++++++++++----------- src/test/ui/span/E0204.stderr | 42 ++++++++-------- 2 files changed, 56 insertions(+), 48 deletions(-) diff --git a/src/librustc_typeck/coherence/builtin.rs b/src/librustc_typeck/coherence/builtin.rs index d067cb99aa096..ba95a17989165 100644 --- a/src/librustc_typeck/coherence/builtin.rs +++ b/src/librustc_typeck/coherence/builtin.rs @@ -27,34 +27,34 @@ use rustc::hir::map as hir_map; use rustc::hir::{self, ItemImpl}; pub fn check<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { - if let Some(drop_trait) = tcx.lang_items.drop_trait() { - tcx.lookup_trait_def(drop_trait) - .for_each_impl(tcx, |impl_did| visit_implementation_of_drop(tcx, impl_did)); - } - - if let Some(copy_trait) = tcx.lang_items.copy_trait() { - tcx.lookup_trait_def(copy_trait) - .for_each_impl(tcx, |impl_did| visit_implementation_of_copy(tcx, impl_did)); - } - - if let Some(coerce_unsized_trait) = tcx.lang_items.coerce_unsized_trait() { - let unsize_trait = match tcx.lang_items.require(UnsizeTraitLangItem) { - Ok(id) => id, - Err(err) => { - tcx.sess.fatal(&format!("`CoerceUnsized` implementation {}", err)); - } - }; + check_trait(tcx, tcx.lang_items.drop_trait(), visit_implementation_of_drop); + check_trait(tcx, tcx.lang_items.copy_trait(), visit_implementation_of_copy); + check_trait( + tcx, + tcx.lang_items.coerce_unsized_trait(), + visit_implementation_of_coerce_unsized); +} - tcx.lookup_trait_def(coerce_unsized_trait).for_each_impl(tcx, |impl_did| { - visit_implementation_of_coerce_unsized(tcx, - impl_did, - unsize_trait, - coerce_unsized_trait) +fn check_trait<'a, 'tcx, F>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + trait_def_id: Option, + mut f: F) + where F: FnMut(TyCtxt<'a, 'tcx, 'tcx>, DefId, DefId) +{ + if let Some(trait_def_id) = trait_def_id { + let mut impls = vec![]; + tcx.lookup_trait_def(trait_def_id).for_each_impl(tcx, |did| { + impls.push(did); }); + impls.sort(); + for impl_def_id in impls { + f(tcx, trait_def_id, impl_def_id); + } } } -fn visit_implementation_of_drop<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId) { +fn visit_implementation_of_drop<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + _drop_did: DefId, + impl_did: DefId) { let items = tcx.associated_item_def_ids(impl_did); if items.is_empty() { // We'll error out later. For now, just don't ICE. @@ -96,7 +96,9 @@ fn visit_implementation_of_drop<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: } } -fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: DefId) { +fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, + _copy_did: DefId, + impl_did: DefId) { debug!("visit_implementation_of_copy: impl_did={:?}", impl_did); let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { @@ -166,12 +168,18 @@ fn visit_implementation_of_copy<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, impl_did: } fn visit_implementation_of_coerce_unsized<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, - impl_did: DefId, - unsize_trait: DefId, - coerce_unsized_trait: DefId) { + coerce_unsized_trait: DefId, + impl_did: DefId) { debug!("visit_implementation_of_coerce_unsized: impl_did={:?}", impl_did); + let unsize_trait = match tcx.lang_items.require(UnsizeTraitLangItem) { + Ok(id) => id, + Err(err) => { + tcx.sess.fatal(&format!("`CoerceUnsized` implementation {}", err)); + } + }; + let impl_node_id = if let Some(n) = tcx.map.as_local_node_id(impl_did) { n } else { diff --git a/src/test/ui/span/E0204.stderr b/src/test/ui/span/E0204.stderr index ae543ed1a5d53..81a60a9dd3099 100644 --- a/src/test/ui/span/E0204.stderr +++ b/src/test/ui/span/E0204.stderr @@ -1,20 +1,11 @@ error[E0204]: the trait `Copy` may not be implemented for this type - --> $DIR/E0204.rs:29:10 - | -29 | #[derive(Copy)] - | ^^^^ -30 | enum EFoo2<'a> { -31 | Bar(&'a mut bool), - | ------------- this field does not implement `Copy` - -error[E0204]: the trait `Copy` may not be implemented for this type - --> $DIR/E0204.rs:17:10 + --> $DIR/E0204.rs:15:6 | -17 | #[derive(Copy)] - | ^^^^ -18 | struct Foo2<'a> { -19 | ty: &'a mut bool, - | ---------------- this field does not implement `Copy` +12 | foo: Vec, + | ------------- this field does not implement `Copy` +... +15 | impl Copy for Foo { } + | ^^^^ error[E0204]: the trait `Copy` may not be implemented for this type --> $DIR/E0204.rs:27:6 @@ -26,13 +17,22 @@ error[E0204]: the trait `Copy` may not be implemented for this type | ^^^^ error[E0204]: the trait `Copy` may not be implemented for this type - --> $DIR/E0204.rs:15:6 + --> $DIR/E0204.rs:17:10 | -12 | foo: Vec, - | ------------- this field does not implement `Copy` -... -15 | impl Copy for Foo { } - | ^^^^ +17 | #[derive(Copy)] + | ^^^^ +18 | struct Foo2<'a> { +19 | ty: &'a mut bool, + | ---------------- this field does not implement `Copy` + +error[E0204]: the trait `Copy` may not be implemented for this type + --> $DIR/E0204.rs:29:10 + | +29 | #[derive(Copy)] + | ^^^^ +30 | enum EFoo2<'a> { +31 | Bar(&'a mut bool), + | ------------- this field does not implement `Copy` error: aborting due to 4 previous errors