From 82e030322bd47d0ac3d1639cabf93e24dbdc1779 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 16 Apr 2017 19:06:26 +0300 Subject: [PATCH 1/3] Check privacy of trait items in all contexts --- src/librustc/ty/mod.rs | 25 ++-- src/librustc_privacy/lib.rs | 27 ----- src/librustc_typeck/astconv.rs | 14 ++- src/librustc_typeck/check/method/mod.rs | 7 -- src/librustc_typeck/check/method/probe.rs | 78 +++++-------- src/test/compile-fail/issue-28514.rs | 47 -------- .../no-method-suggested-traits.rs | 2 +- src/test/compile-fail/trait-item-privacy.rs | 110 ++++++++++++++++++ src/test/compile-fail/trait-not-accessible.rs | 28 ----- 9 files changed, 162 insertions(+), 176 deletions(-) delete mode 100644 src/test/compile-fail/issue-28514.rs create mode 100644 src/test/compile-fail/trait-item-privacy.rs delete mode 100644 src/test/compile-fail/trait-not-accessible.rs diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 352b6ab1d356f..6a00586a01f82 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 92f7e48b6be48..5ef79d6ceeb0d 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 5137ae6ff4222..e0c67c1456d02 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 7dd2699a6eaf0..72fba1ef6ecca 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 80f9372eb54c4..26e8693d3b2aa 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 3488310b12883..0000000000000 --- 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 ea8796d38f93c..a8d97d4674cbb 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 0000000000000..4e8f8d6760a75 --- /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 5feef0a24eb0e..0000000000000 --- 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); -} From 1883b8d715d91c65cb25fc8a2e7258c9fc704c16 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 16 Apr 2017 21:19:24 +0300 Subject: [PATCH 2/3] privacy: Rename and cleanup PrivacyVisitor --- src/librustc_privacy/lib.rs | 108 ++++++------------ .../privacy/union-field-privacy-1.rs | 4 +- 2 files changed, 40 insertions(+), 72 deletions(-) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 5ef79d6ceeb0d..f7155f219a828 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -13,8 +13,8 @@ #![crate_type = "dylib"] #![crate_type = "rlib"] #![doc(html_logo_url = "https://www.rust-lang.org/logos/rust-logo-128x128-blk-v2.png", - html_favicon_url = "https://doc.rust-lang.org/favicon.ico", - html_root_url = "https://doc.rust-lang.org/nightly/")] + html_favicon_url = "https://doc.rust-lang.org/favicon.ico", + html_root_url = "https://doc.rust-lang.org/nightly/")] #![deny(warnings)] #![feature(rustc_diagnostic_macros)] @@ -30,7 +30,6 @@ use rustc::hir::def::Def; use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, CrateNum, DefId}; use rustc::hir::intravisit::{self, Visitor, NestedVisitorMap}; use rustc::hir::itemlikevisit::DeepVisitor; -use rustc::hir::pat_util::EnumerateAndAdjustIterator; use rustc::lint; use rustc::middle::privacy::{AccessLevel, AccessLevels}; use rustc::ty::{self, TyCtxt, Ty, TypeFoldable}; @@ -415,30 +414,32 @@ impl<'b, 'a, 'tcx> TypeVisitor<'tcx> for ReachEverythingInTheInterfaceVisitor<'b } } -//////////////////////////////////////////////////////////////////////////////// -/// The privacy visitor, where privacy checks take place (violations reported) -//////////////////////////////////////////////////////////////////////////////// +////////////////////////////////////////////////////////////////////////////////////// +/// Name privacy visitor, checks privacy and reports violations. +/// Most of name privacy checks are performed during the main resolution phase, +/// or later in type checking when field accesses and associated items are resolved. +/// This pass performs remaining checks for fields in struct expressions and patterns. +////////////////////////////////////////////////////////////////////////////////////// -struct PrivacyVisitor<'a, 'tcx: 'a> { +struct NamePrivacyVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, - curitem: DefId, - in_foreign: bool, tables: &'a ty::TypeckTables<'tcx>, + current_item: DefId, } -impl<'a, 'tcx> PrivacyVisitor<'a, 'tcx> { - // Checks that a field is in scope. +impl<'a, 'tcx> NamePrivacyVisitor<'a, 'tcx> { + // Checks that a field is accessible. 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) { + if !def.is_enum() && !field.vis.is_accessible_from(self.current_item, self.tcx) { struct_span_err!(self.tcx.sess, span, E0451, "field `{}` of {} `{}` is private", - field.name, def.variant_descr(), self.tcx.item_path_str(def.did)) + field.name, def.variant_descr(), self.tcx.item_path_str(def.did)) .span_label(span, &format!("field `{}` is private", field.name)) .emit(); } } } -impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> { +impl<'a, 'tcx> Visitor<'tcx> for NamePrivacyVisitor<'a, 'tcx> { /// We want to visit items in the context of their containing /// module and so forth, so supply a crate for doing a deep walk. fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> { @@ -446,39 +447,36 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> { } fn visit_nested_body(&mut self, body: hir::BodyId) { - let old_tables = self.tables; - self.tables = self.tcx.body_tables(body); + let orig_tables = replace(&mut self.tables, self.tcx.body_tables(body)); let body = self.tcx.hir.body(body); self.visit_body(body); - self.tables = old_tables; + self.tables = orig_tables; } fn visit_item(&mut self, item: &'tcx hir::Item) { - let orig_curitem = replace(&mut self.curitem, self.tcx.hir.local_def_id(item.id)); + let orig_current_item = replace(&mut self.current_item, self.tcx.hir.local_def_id(item.id)); intravisit::walk_item(self, item); - self.curitem = orig_curitem; + self.current_item = orig_current_item; } fn visit_expr(&mut self, expr: &'tcx hir::Expr) { match expr.node { - hir::ExprStruct(ref qpath, ref expr_fields, _) => { + hir::ExprStruct(ref qpath, ref fields, ref base) => { let def = self.tables.qpath_def(qpath, expr.id); let adt = self.tables.expr_ty(expr).ty_adt_def().unwrap(); let variant = adt.variant_of_def(def); - // RFC 736: ensure all unmentioned fields are visible. - // Rather than computing the set of unmentioned fields - // (i.e. `all_fields - fields`), just check them all, - // unless the ADT is a union, then unmentioned fields - // are not checked. - if adt.is_union() { - for expr_field in expr_fields { - self.check_field(expr.span, adt, variant.field_named(expr_field.name.node)); + if let Some(ref base) = *base { + // If the expression uses FRU we need to make sure all the unmentioned fields + // are checked for privacy (RFC 736). Rather than computing the set of + // unmentioned fields, just check them all. + for variant_field in &variant.fields { + let field = fields.iter().find(|f| f.name.node == variant_field.name); + let span = if let Some(f) = field { f.span } else { base.span }; + self.check_field(span, adt, variant_field); } } else { - for field in &variant.fields { - let expr_field = expr_fields.iter().find(|f| f.name.node == field.name); - let span = if let Some(f) = expr_field { f.span } else { expr.span }; - self.check_field(span, adt, field); + for field in fields { + self.check_field(field.span, adt, variant.field_named(field.name.node)); } } } @@ -488,47 +486,20 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivacyVisitor<'a, 'tcx> { intravisit::walk_expr(self, expr); } - fn visit_pat(&mut self, pattern: &'tcx hir::Pat) { - // Foreign functions do not have their patterns mapped in the def_map, - // and there's nothing really relevant there anyway, so don't bother - // checking privacy. If you can name the type then you can pass it to an - // external C function anyway. - if self.in_foreign { return } - - match pattern.node { + fn visit_pat(&mut self, pat: &'tcx hir::Pat) { + match pat.node { PatKind::Struct(ref qpath, ref fields, _) => { - let def = self.tables.qpath_def(qpath, pattern.id); - let adt = self.tables.pat_ty(pattern).ty_adt_def().unwrap(); + let def = self.tables.qpath_def(qpath, pat.id); + let adt = self.tables.pat_ty(pat).ty_adt_def().unwrap(); let variant = adt.variant_of_def(def); for field in fields { self.check_field(field.span, adt, variant.field_named(field.node.name)); } } - PatKind::TupleStruct(_, ref fields, ddpos) => { - match self.tables.pat_ty(pattern).sty { - // enum fields have no privacy at this time - ty::TyAdt(def, _) if !def.is_enum() => { - let expected_len = def.struct_variant().fields.len(); - for (i, field) in fields.iter().enumerate_and_adjust(expected_len, ddpos) { - if let PatKind::Wild = field.node { - continue - } - self.check_field(field.span, def, &def.struct_variant().fields[i]); - } - } - _ => {} - } - } _ => {} } - intravisit::walk_pat(self, pattern); - } - - fn visit_foreign_item(&mut self, fi: &'tcx hir::ForeignItem) { - self.in_foreign = true; - intravisit::walk_foreign_item(self, fi); - self.in_foreign = false; + intravisit::walk_pat(self, pat); } } @@ -1206,17 +1177,14 @@ fn privacy_access_levels<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, let krate = tcx.hir.krate(); - // Use the parent map to check the privacy of everything - let mut visitor = PrivacyVisitor { - curitem: DefId::local(CRATE_DEF_INDEX), - in_foreign: false, + // Check privacy of names not checked in previous compilation stages. + let mut visitor = NamePrivacyVisitor { tcx: tcx, tables: &ty::TypeckTables::empty(), + current_item: DefId::local(CRATE_DEF_INDEX), }; intravisit::walk_crate(&mut visitor, krate); - tcx.sess.abort_if_errors(); - // Build up a set of all exported items in the AST. This is a set of all // items which are reachable from external crates based on visibility. let mut visitor = EmbargoVisitor { diff --git a/src/test/compile-fail/privacy/union-field-privacy-1.rs b/src/test/compile-fail/privacy/union-field-privacy-1.rs index bddcd391b205d..807be619f6c5f 100644 --- a/src/test/compile-fail/privacy/union-field-privacy-1.rs +++ b/src/test/compile-fail/privacy/union-field-privacy-1.rs @@ -18,7 +18,7 @@ mod m { } } -fn main() { +fn main() { unsafe { let u = m::U { a: 0 }; // OK let u = m::U { b: 0 }; // OK let u = m::U { c: 0 }; //~ ERROR field `c` of union `m::U` is private @@ -26,4 +26,4 @@ fn main() { let m::U { a } = u; // OK let m::U { b } = u; // OK let m::U { c } = u; //~ ERROR field `c` of union `m::U` is private -} +}} From 4bd417e4389e9d1c4b589d6f36911e8e05224904 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 25 Apr 2017 22:38:21 +0300 Subject: [PATCH 3/3] Fix object safety violations in the test --- src/test/compile-fail/trait-item-privacy.rs | 71 ++++++++++++++------- 1 file changed, 48 insertions(+), 23 deletions(-) diff --git a/src/test/compile-fail/trait-item-privacy.rs b/src/test/compile-fail/trait-item-privacy.rs index 4e8f8d6760a75..721d7583230a7 100644 --- a/src/test/compile-fail/trait-item-privacy.rs +++ b/src/test/compile-fail/trait-item-privacy.rs @@ -16,23 +16,37 @@ 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_const { + trait A { + const A: u8 = 0; + } + + pub trait B { + const B: u8 = 0; + } + + pub trait C: A + B { + const C: u8 = 0; + } + impl A for ::S {} + impl B for ::S {} + impl C for ::S {} } mod assoc_ty { @@ -51,27 +65,9 @@ mod assoc_ty { 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() { +fn check_method() { // A is private // B is pub, not in scope // C : A + B is pub, in scope @@ -97,6 +93,13 @@ fn main() { C::a(&S); //~ ERROR method `a` is private C::b(&S); // OK C::c(&S); // OK +} + +fn check_assoc_const() { + // A is private + // B is pub, not in scope + // C : A + B is pub, in scope + use assoc_const::C; // Associated constants // A, B, C are resolved as trait items, their traits need to be in scope @@ -105,6 +108,28 @@ fn main() { 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 + //~^ ERROR the trait `assoc_const::C` cannot be made into an object + //~| ERROR the trait bound `assoc_const::C: assoc_const::A` is not satisfied + C::B; // ERROR the trait `assoc_const::C` cannot be made into an object + //~^ ERROR the trait bound `assoc_const::C: assoc_const::B` is not satisfied C::C; // OK } + +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() {}