From d5bb71c9f1042c0c931c7dddecc418233de6e30f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Sat, 23 Feb 2019 16:40:15 +0100 Subject: [PATCH] Split up privacy checking so privacy_access_levels only does computations required for AccessLevels --- src/librustc/dep_graph/dep_node.rs | 1 + src/librustc/ty/query/config.rs | 6 ++ src/librustc/ty/query/mod.rs | 3 +- src/librustc/ty/query/plumbing.rs | 1 + src/librustc_interface/passes.rs | 29 ++++-- src/librustc_privacy/lib.rs | 88 +++++++++---------- .../ui/privacy/private-inferred-type.stderr | 36 ++++---- 7 files changed, 89 insertions(+), 75 deletions(-) diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index c607eb5906e..a46ffffe94c 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -456,6 +456,7 @@ define_dep_nodes!( <'tcx> [eval_always] CoherenceInherentImplOverlapCheck, [] CoherenceCheckTrait(DefId), [eval_always] PrivacyAccessLevels(CrateNum), + [eval_always] CheckPrivacy(CrateNum), [eval_always] Analysis(CrateNum), // Represents the MIR for a fn; also used as the task node for diff --git a/src/librustc/ty/query/config.rs b/src/librustc/ty/query/config.rs index feca0f7170e..0fef90c945e 100644 --- a/src/librustc/ty/query/config.rs +++ b/src/librustc/ty/query/config.rs @@ -369,6 +369,12 @@ impl<'tcx> QueryDescription<'tcx> for queries::privacy_access_levels<'tcx> { } } +impl<'tcx> QueryDescription<'tcx> for queries::check_privacy<'tcx> { + fn describe(_: TyCtxt<'_, '_, '_>, _: CrateNum) -> Cow<'static, str> { + "privacy checking".into() + } +} + impl<'tcx> QueryDescription<'tcx> for queries::typeck_item_bodies<'tcx> { fn describe(_: TyCtxt<'_, '_, '_>, _: CrateNum) -> Cow<'static, str> { "type-checking all item bodies".into() diff --git a/src/librustc/ty/query/mod.rs b/src/librustc/ty/query/mod.rs index 197b9a71b0a..cb72ad6fe8b 100644 --- a/src/librustc/ty/query/mod.rs +++ b/src/librustc/ty/query/mod.rs @@ -350,8 +350,9 @@ define_queries! { <'tcx> [] fn check_match: CheckMatch(DefId) -> Result<(), ErrorReported>, - /// Performs the privacy check and computes "access levels". + /// Performs part of the privacy check and computes "access levels". [] fn privacy_access_levels: PrivacyAccessLevels(CrateNum) -> Lrc, + [] fn check_privacy: CheckPrivacy(CrateNum) -> (), }, Other { diff --git a/src/librustc/ty/query/plumbing.rs b/src/librustc/ty/query/plumbing.rs index ebaa31d703f..ff2639da136 100644 --- a/src/librustc/ty/query/plumbing.rs +++ b/src/librustc/ty/query/plumbing.rs @@ -1251,6 +1251,7 @@ pub fn force_from_dep_node<'a, 'gcx, 'lcx>(tcx: TyCtxt<'a, 'gcx, 'lcx>, force!(crate_inherent_impls_overlap_check, LOCAL_CRATE) }, DepKind::PrivacyAccessLevels => { force!(privacy_access_levels, LOCAL_CRATE); } + DepKind::CheckPrivacy => { force!(check_privacy, LOCAL_CRATE); } DepKind::MirBuilt => { force!(mir_built, def_id!()); } DepKind::MirConstQualif => { force!(mir_const_qualif, def_id!()); } DepKind::MirConst => { force!(mir_const, def_id!()); } diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 16ced695638..591583a1526 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -21,7 +21,7 @@ use rustc_borrowck as borrowck; use rustc_codegen_utils::codegen_backend::CodegenBackend; use rustc_data_structures::fingerprint::Fingerprint; use rustc_data_structures::stable_hasher::StableHasher; -use rustc_data_structures::sync::Lrc; +use rustc_data_structures::sync::{Lrc, ParallelIterator, par_iter}; use rustc_incremental; use rustc_metadata::creader::CrateLoader; use rustc_metadata::cstore::{self, CStore}; @@ -278,17 +278,28 @@ fn analysis<'tcx>( time(sess, "misc checking", || { parallel!({ - time(sess, "privacy checking", || { - rustc_privacy::check_crate(tcx) + time(sess, "privacy access levels", || { + tcx.ensure().privacy_access_levels(LOCAL_CRATE); + }); + parallel!({ + time(sess, "privacy checking", || { + tcx.ensure().check_privacy(LOCAL_CRATE); + }); + }, { + time(sess, "death checking", || middle::dead::check_crate(tcx)); + }, { + time(sess, "unused lib feature checking", || { + stability::check_unused_or_stable_features(tcx) + }); + }, { + time(sess, "lint checking", || lint::check_crate(tcx)); }); }, { - time(sess, "death checking", || middle::dead::check_crate(tcx)); - }, { - time(sess, "unused lib feature checking", || { - stability::check_unused_or_stable_features(tcx) + time(sess, "privacy checking modules", || { + par_iter(&tcx.hir().krate().modules).for_each(|(&module, _)| { + tcx.ensure().check_mod_privacy(tcx.hir().local_def_id(module)); + }); }); - }, { - time(sess, "lint checking", || lint::check_crate(tcx)); }); }); diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 550b333700b..372a9230694 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1760,19 +1760,15 @@ impl<'a, 'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> pub fn provide(providers: &mut Providers<'_>) { *providers = Providers { privacy_access_levels, + check_privacy, check_mod_privacy, ..*providers }; } -pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Lrc { - tcx.privacy_access_levels(LOCAL_CRATE) -} - fn check_mod_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, module_def_id: DefId) { let empty_tables = ty::TypeckTables::empty(None); - // Check privacy of names not checked in previous compilation stages. let mut visitor = NamePrivacyVisitor { tcx, @@ -1803,18 +1799,6 @@ fn privacy_access_levels<'tcx>( ) -> Lrc { assert_eq!(krate, LOCAL_CRATE); - let krate = tcx.hir().krate(); - - for &module in krate.modules.keys() { - tcx.ensure().check_mod_privacy(tcx.hir().local_def_id(module)); - } - - let private_crates: FxHashSet = tcx.sess.opts.extern_private.iter() - .flat_map(|c| { - tcx.crates().iter().find(|&&krate| &tcx.crate_name(krate) == c).cloned() - }).collect(); - - // 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 { @@ -1824,7 +1808,7 @@ fn privacy_access_levels<'tcx>( changed: false, }; loop { - intravisit::walk_crate(&mut visitor, krate); + intravisit::walk_crate(&mut visitor, tcx.hir().krate()); if visitor.changed { visitor.changed = false; } else { @@ -1833,36 +1817,46 @@ fn privacy_access_levels<'tcx>( } visitor.update(hir::CRATE_HIR_ID, Some(AccessLevel::Public)); - { - let mut visitor = ObsoleteVisiblePrivateTypesVisitor { - tcx, - access_levels: &visitor.access_levels, - in_variant: false, - old_error_set: Default::default(), - }; - intravisit::walk_crate(&mut visitor, krate); - - - let has_pub_restricted = { - let mut pub_restricted_visitor = PubRestrictedVisitor { - tcx, - has_pub_restricted: false - }; - intravisit::walk_crate(&mut pub_restricted_visitor, krate); - pub_restricted_visitor.has_pub_restricted - }; - - // Check for private types and traits in public interfaces. - let mut visitor = PrivateItemsInPublicInterfacesVisitor { - tcx, - has_pub_restricted, - old_error_set: &visitor.old_error_set, - private_crates - }; - krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); - } - Lrc::new(visitor.access_levels) } +fn check_privacy<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, krate: CrateNum) { + assert_eq!(krate, LOCAL_CRATE); + + let access_levels = tcx.privacy_access_levels(LOCAL_CRATE); + + let krate = tcx.hir().krate(); + + let mut visitor = ObsoleteVisiblePrivateTypesVisitor { + tcx, + access_levels: &access_levels, + in_variant: false, + old_error_set: Default::default(), + }; + intravisit::walk_crate(&mut visitor, krate); + + let has_pub_restricted = { + let mut pub_restricted_visitor = PubRestrictedVisitor { + tcx, + has_pub_restricted: false + }; + intravisit::walk_crate(&mut pub_restricted_visitor, krate); + pub_restricted_visitor.has_pub_restricted + }; + + let private_crates: FxHashSet = tcx.sess.opts.extern_private.iter() + .flat_map(|c| { + tcx.crates().iter().find(|&&krate| &tcx.crate_name(krate) == c).cloned() + }).collect(); + + // Check for private types and traits in public interfaces. + let mut visitor = PrivateItemsInPublicInterfacesVisitor { + tcx, + has_pub_restricted, + old_error_set: &visitor.old_error_set, + private_crates + }; + krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); +} + __build_diagnostic_array! { librustc_privacy, DIAGNOSTICS } diff --git a/src/test/ui/privacy/private-inferred-type.stderr b/src/test/ui/privacy/private-inferred-type.stderr index 80a475f7dce..568d4dadc8c 100644 --- a/src/test/ui/privacy/private-inferred-type.stderr +++ b/src/test/ui/privacy/private-inferred-type.stderr @@ -1,3 +1,21 @@ +error[E0446]: private type `m::Priv` in public interface + --> $DIR/private-inferred-type.rs:61:36 + | +LL | struct Priv; + | - `m::Priv` declared as private +... +LL | impl TraitWithAssocTy for u8 { type AssocTy = Priv; } + | ^^^^^^^^^^^^^^^^^^^^ can't leak private type + +error[E0446]: private type `adjust::S2` in public interface + --> $DIR/private-inferred-type.rs:83:9 + | +LL | struct S2; + | - `adjust::S2` declared as private +... +LL | type Target = S2Alias; //~ ERROR private type `adjust::S2` in public interface + | ^^^^^^^^^^^^^^^^^^^^^^ can't leak private type + error: type `m::Priv` is private --> $DIR/private-inferred-type.rs:97:9 | @@ -202,24 +220,6 @@ error: type `m::Priv` is private LL | match a { //~ ERROR type `m::Priv` is private | ^ -error[E0446]: private type `m::Priv` in public interface - --> $DIR/private-inferred-type.rs:61:36 - | -LL | struct Priv; - | - `m::Priv` declared as private -... -LL | impl TraitWithAssocTy for u8 { type AssocTy = Priv; } - | ^^^^^^^^^^^^^^^^^^^^ can't leak private type - -error[E0446]: private type `adjust::S2` in public interface - --> $DIR/private-inferred-type.rs:83:9 - | -LL | struct S2; - | - `adjust::S2` declared as private -... -LL | type Target = S2Alias; //~ ERROR private type `adjust::S2` in public interface - | ^^^^^^^^^^^^^^^^^^^^^^ can't leak private type - error: aborting due to 33 previous errors For more information about this error, try `rustc --explain E0446`.