From 1883b8d715d91c65cb25fc8a2e7258c9fc704c16 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Sun, 16 Apr 2017 21:19:24 +0300 Subject: [PATCH] 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 5ef79d6ceeb0..f7155f219a82 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 bddcd391b205..807be619f6c5 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 -} +}}