diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 352b6ab1d356..6a00586a01f8 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2156,6 +2156,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { fn associated_item_from_trait_item_ref(self, parent_def_id: DefId, + parent_vis: &hir::Visibility, trait_item_ref: &hir::TraitItemRef) -> AssociatedItem { let def_id = self.hir.local_def_id(trait_item_ref.id.node_id); @@ -2170,7 +2171,8 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { AssociatedItem { name: trait_item_ref.name, kind: kind, - vis: Visibility::from_hir(&hir::Inherited, trait_item_ref.id.node_id, self), + // Visibility of trait items is inherited from their traits. + vis: Visibility::from_hir(parent_vis, trait_item_ref.id.node_id, self), defaultness: trait_item_ref.defaultness, def_id: def_id, container: TraitContainer(parent_def_id), @@ -2180,7 +2182,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { fn associated_item_from_impl_item_ref(self, parent_def_id: DefId, - from_trait_impl: bool, impl_item_ref: &hir::ImplItemRef) -> AssociatedItem { let def_id = self.hir.local_def_id(impl_item_ref.id.node_id); @@ -2192,14 +2193,11 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { hir::AssociatedItemKind::Type => (ty::AssociatedKind::Type, false), }; - // Trait impl items are always public. - let public = hir::Public; - let vis = if from_trait_impl { &public } else { &impl_item_ref.vis }; - ty::AssociatedItem { name: impl_item_ref.name, kind: kind, - vis: ty::Visibility::from_hir(vis, impl_item_ref.id.node_id, self), + // Visibility of trait impl items doesn't matter. + vis: ty::Visibility::from_hir(&impl_item_ref.vis, impl_item_ref.id.node_id, self), defaultness: impl_item_ref.defaultness, def_id: def_id, container: ImplContainer(parent_def_id), @@ -2639,12 +2637,10 @@ fn associated_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) let parent_def_id = tcx.hir.local_def_id(parent_id); let parent_item = tcx.hir.expect_item(parent_id); match parent_item.node { - hir::ItemImpl(.., ref impl_trait_ref, _, ref impl_item_refs) => { + hir::ItemImpl(.., ref impl_item_refs) => { if let Some(impl_item_ref) = impl_item_refs.iter().find(|i| i.id.node_id == id) { - let assoc_item = - tcx.associated_item_from_impl_item_ref(parent_def_id, - impl_trait_ref.is_some(), - impl_item_ref); + let assoc_item = tcx.associated_item_from_impl_item_ref(parent_def_id, + impl_item_ref); debug_assert_eq!(assoc_item.def_id, def_id); return assoc_item; } @@ -2652,8 +2648,9 @@ fn associated_item<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) hir::ItemTrait(.., ref trait_item_refs) => { if let Some(trait_item_ref) = trait_item_refs.iter().find(|i| i.id.node_id == id) { - let assoc_item = - tcx.associated_item_from_trait_item_ref(parent_def_id, trait_item_ref); + let assoc_item = tcx.associated_item_from_trait_item_ref(parent_def_id, + &parent_item.vis, + trait_item_ref); debug_assert_eq!(assoc_item.def_id, def_id); return assoc_item; } diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 92f7e48b6be4..5ef79d6ceeb0 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -427,14 +427,6 @@ struct PrivacyVisitor<'a, 'tcx: 'a> { } impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { - fn item_is_accessible(&self, did: DefId) -> bool { - match self.tcx.hir.as_local_node_id(did) { - Some(node_id) => - ty::Visibility::from_hir(&self.tcx.hir.expect_item(node_id).vis, node_id, self.tcx), - None => self.tcx.sess.cstore.visibility(did), - }.is_accessible_from(self.curitem, self.tcx) - } - // Checks that a field is in scope. fn check_field(&mut self, span: Span, def: &'tcx ty::AdtDef, field: &'tcx ty::FieldDef) { if !def.is_enum() && !field.vis.is_accessible_from(self.curitem, self.tcx) { @@ -444,20 +436,6 @@ impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { .emit(); } } - - // Checks that a method is in scope. - fn check_method(&mut self, span: Span, method_def_id: DefId) { - match self.tcx.associated_item(method_def_id).container { - // Trait methods are always all public. The only controlling factor - // is whether the trait itself is accessible or not. - ty::TraitContainer(trait_def_id) if !self.item_is_accessible(trait_def_id) => { - let msg = format!("source trait `{}` is private", - self.tcx.item_path_str(trait_def_id)); - self.tcx.sess.span_err(span, &msg); - } - _ => {} - } - } } impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> { @@ -483,11 +461,6 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> { fn visit_expr(&mut self, expr: &'tcx hir::Expr) { match expr.node { - hir::ExprMethodCall(..) => { - let method_call = ty::MethodCall::expr(expr.id); - let method = self.tables.method_map[&method_call]; - self.check_method(expr.span, method.def_id); - } hir::ExprStruct(ref qpath, ref expr_fields, _) => { let def = self.tables.qpath_def(qpath, expr.id); let adt = self.tables.expr_ty(expr).ty_adt_def().unwrap(); diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 5137ae6ff422..e0c67c1456d0 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -903,10 +903,16 @@ impl<'o, 'gcx: 'tcx, 'tcx> AstConv<'gcx, 'tcx>+'o { let ty = self.projected_ty_from_poly_trait_ref(span, bound, assoc_name); let ty = self.normalize_ty(span, ty); - let item = tcx.associated_items(trait_did).find(|i| i.name == assoc_name); - let def_id = item.expect("missing associated type").def_id; - tcx.check_stability(def_id, ref_id, span); - (ty, Def::AssociatedTy(def_id)) + let item = tcx.associated_items(trait_did).find(|i| i.name == assoc_name) + .expect("missing associated type"); + let def = Def::AssociatedTy(item.def_id); + if !tcx.vis_is_accessible_from(item.vis, ref_id) { + let msg = format!("{} `{}` is private", def.kind_name(), assoc_name); + tcx.sess.span_err(span, &msg); + } + tcx.check_stability(item.def_id, ref_id, span); + + (ty, def) } fn qpath_to_ty(&self, diff --git a/src/librustc_typeck/check/method/mod.rs b/src/librustc_typeck/check/method/mod.rs index 7dd2699a6eaf..72fba1ef6ecc 100644 --- a/src/librustc_typeck/check/method/mod.rs +++ b/src/librustc_typeck/check/method/mod.rs @@ -349,15 +349,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } let def = pick.item.def(); - self.tcx.check_stability(def.def_id(), expr_id, span); - if let probe::InherentImplPick = pick.kind { - if !self.tcx.vis_is_accessible_from(pick.item.vis, self.body_id) { - let msg = format!("{} `{}` is private", def.kind_name(), method_name); - self.tcx.sess.span_err(span, &msg); - } - } Ok(def) } diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 80f9372eb54c..26e8693d3b2a 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -369,6 +369,24 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { /////////////////////////////////////////////////////////////////////////// // CANDIDATE ASSEMBLY + fn push_inherent_candidate(&mut self, xform_self_ty: Ty<'tcx>, item: ty::AssociatedItem, + kind: CandidateKind<'tcx>, import_id: Option) { + if self.tcx.vis_is_accessible_from(item.vis, self.body_id) { + self.inherent_candidates.push(Candidate { xform_self_ty, item, kind, import_id }); + } else if self.private_candidate.is_none() { + self.private_candidate = Some(item.def()); + } + } + + fn push_extension_candidate(&mut self, xform_self_ty: Ty<'tcx>, item: ty::AssociatedItem, + kind: CandidateKind<'tcx>, import_id: Option) { + if self.tcx.vis_is_accessible_from(item.vis, self.body_id) { + self.extension_candidates.push(Candidate { xform_self_ty, item, kind, import_id }); + } else if self.private_candidate.is_none() { + self.private_candidate = Some(item.def()); + } + } + fn assemble_inherent_candidates(&mut self) { let steps = self.steps.clone(); for step in steps.iter() { @@ -499,11 +517,6 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { continue } - if !self.tcx.vis_is_accessible_from(item.vis, self.body_id) { - self.private_candidate = Some(item.def()); - continue - } - let (impl_ty, impl_substs) = self.impl_ty_and_substs(impl_def_id); let impl_ty = impl_ty.subst(self.tcx, impl_substs); @@ -519,12 +532,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { debug!("assemble_inherent_impl_probe: xform_self_ty = {:?}", xform_self_ty); - self.inherent_candidates.push(Candidate { - xform_self_ty: xform_self_ty, - item: item, - kind: InherentImplCandidate(impl_substs, obligations), - import_id: None, - }); + self.push_inherent_candidate(xform_self_ty, item, + InherentImplCandidate(impl_substs, obligations), None); } } @@ -548,12 +557,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { let xform_self_ty = this.xform_self_ty(&item, new_trait_ref.self_ty(), new_trait_ref.substs); - this.inherent_candidates.push(Candidate { - xform_self_ty: xform_self_ty, - item: item, - kind: ObjectCandidate, - import_id: None, - }); + this.push_inherent_candidate(xform_self_ty, item, ObjectCandidate, None); }); } @@ -599,12 +603,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { // `WhereClausePick`. assert!(!trait_ref.substs.needs_infer()); - this.inherent_candidates.push(Candidate { - xform_self_ty: xform_self_ty, - item: item, - kind: WhereClauseCandidate(poly_trait_ref), - import_id: None, - }); + this.push_inherent_candidate(xform_self_ty, item, + WhereClauseCandidate(poly_trait_ref), None); }); } @@ -743,12 +743,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { debug!("xform_self_ty={:?}", xform_self_ty); - self.extension_candidates.push(Candidate { - xform_self_ty: xform_self_ty, - item: item.clone(), - kind: ExtensionImplCandidate(impl_def_id, impl_substs, obligations), - import_id: import_id, - }); + self.push_extension_candidate(xform_self_ty, item, + ExtensionImplCandidate(impl_def_id, impl_substs, obligations), import_id); }); } @@ -833,12 +829,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { }); let xform_self_ty = self.xform_self_ty(&item, step.self_ty, substs); - self.inherent_candidates.push(Candidate { - xform_self_ty: xform_self_ty, - item: item.clone(), - kind: TraitCandidate, - import_id: import_id, - }); + self.push_inherent_candidate(xform_self_ty, item, TraitCandidate, import_id); } Ok(()) @@ -854,7 +845,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { trait_def_id, item); - for step in self.steps.iter() { + for step in Rc::clone(&self.steps).iter() { debug!("assemble_projection_candidates: step={:?}", step); let (def_id, substs) = match step.self_ty.sty { @@ -889,12 +880,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { bound, xform_self_ty); - self.extension_candidates.push(Candidate { - xform_self_ty: xform_self_ty, - item: item.clone(), - kind: TraitCandidate, - import_id: import_id, - }); + self.push_extension_candidate(xform_self_ty, item, TraitCandidate, import_id); } } } @@ -918,12 +904,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { bound, xform_self_ty); - self.extension_candidates.push(Candidate { - xform_self_ty: xform_self_ty, - item: item.clone(), - kind: WhereClauseCandidate(poly_bound), - import_id: import_id, - }); + self.push_extension_candidate(xform_self_ty, item, + WhereClauseCandidate(poly_bound), import_id); } } diff --git a/src/test/compile-fail/issue-28514.rs b/src/test/compile-fail/issue-28514.rs deleted file mode 100644 index 3488310b1288..000000000000 --- a/src/test/compile-fail/issue-28514.rs +++ /dev/null @@ -1,47 +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. - -#![deny(private_in_public)] - -pub use inner::C; - -mod inner { - trait A { - fn a(&self) { } - } - - pub trait B { - fn b(&self) { } - } - - pub trait C: A + B { //~ ERROR private trait `inner::A` in public interface - //~^ WARN will become a hard error - fn c(&self) { } - } - - impl A for i32 {} - impl B for i32 {} - impl C for i32 {} - -} - -fn main() { - // A is private - // B is pub, not reexported - // C : A + B is pub, reexported - - // 0.a(); // can't call - // 0.b(); // can't call - 0.c(); // ok - - C::a(&0); // can call - C::b(&0); // can call - C::c(&0); // ok -} diff --git a/src/test/compile-fail/no-method-suggested-traits.rs b/src/test/compile-fail/no-method-suggested-traits.rs index ea8796d38f93..a8d97d4674cb 100644 --- a/src/test/compile-fail/no-method-suggested-traits.rs +++ b/src/test/compile-fail/no-method-suggested-traits.rs @@ -16,7 +16,7 @@ struct Foo; enum Bar { X } mod foo { - trait Bar { + pub trait Bar { fn method(&self) {} fn method2(&self) {} diff --git a/src/test/compile-fail/trait-item-privacy.rs b/src/test/compile-fail/trait-item-privacy.rs new file mode 100644 index 000000000000..4e8f8d6760a7 --- /dev/null +++ b/src/test/compile-fail/trait-item-privacy.rs @@ -0,0 +1,110 @@ +// 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. + +#![feature(associated_consts)] +#![feature(associated_type_defaults)] + +struct S; + +mod method { + trait A { + fn a(&self) { } + const A: u8 = 0; + } + + pub trait B { + fn b(&self) { } + const B: u8 = 0; + } + + pub trait C: A + B { + fn c(&self) { } + const C: u8 = 0; + } + + impl A for ::S {} + impl B for ::S {} + impl C for ::S {} + +} + +mod assoc_ty { + trait A { + type A = u8; + } + + pub trait B { + type B = u8; + } + + pub trait C: A + B { + type C = u8; + } + + impl A for ::S {} + impl B for ::S {} + impl C for ::S {} + +} + +fn check_assoc_ty() { + // A is private + // B is pub, not in scope + // C : A + B is pub, in scope + use assoc_ty::C; + + // Associated types + // A, B, C are resolved as trait items, their traits need to be in scope, not implemented yet + let _: S::A; //~ ERROR ambiguous associated type + let _: S::B; //~ ERROR ambiguous associated type + let _: S::C; //~ ERROR ambiguous associated type + // A, B, C are resolved as inherent items, their traits don't need to be in scope + let _: T::A; //~ ERROR associated type `A` is private + let _: T::B; // OK + let _: T::C; // OK +} + +fn main() { + // A is private + // B is pub, not in scope + // C : A + B is pub, in scope + use method::C; + + // Methods, method call + // a, b, c are resolved as trait items, their traits need to be in scope + S.a(); //~ ERROR no method named `a` found for type `S` in the current scope + S.b(); //~ ERROR no method named `b` found for type `S` in the current scope + S.c(); // OK + // a, b, c are resolved as inherent items, their traits don't need to be in scope + let c = &S as &C; + c.a(); //~ ERROR method `a` is private + c.b(); // OK + c.c(); // OK + + // Methods, UFCS + // a, b, c are resolved as trait items, their traits need to be in scope + S::a(&S); //~ ERROR no associated item named `a` found for type `S` in the current scope + S::b(&S); //~ ERROR no associated item named `b` found for type `S` in the current scope + S::c(&S); // OK + // a, b, c are resolved as inherent items, their traits don't need to be in scope + C::a(&S); //~ ERROR method `a` is private + C::b(&S); // OK + C::c(&S); // OK + + // Associated constants + // A, B, C are resolved as trait items, their traits need to be in scope + S::A; //~ ERROR no associated item named `A` found for type `S` in the current scope + S::B; //~ ERROR no associated item named `B` found for type `S` in the current scope + S::C; // OK + // A, B, C are resolved as inherent items, their traits don't need to be in scope + C::A; //~ ERROR associated constant `A` is private + C::B; // OK + C::C; // OK +} diff --git a/src/test/compile-fail/trait-not-accessible.rs b/src/test/compile-fail/trait-not-accessible.rs deleted file mode 100644 index 5feef0a24eb0..000000000000 --- a/src/test/compile-fail/trait-not-accessible.rs +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright 2015 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. - -mod m { - trait Priv { - fn f(&self) {} - } - impl Priv for super::S {} - pub trait Pub: Priv {} -} - -struct S; -impl m::Pub for S {} - -fn g(arg: T) { - arg.f(); //~ ERROR: source trait `m::Priv` is private -} - -fn main() { - g(S); -}