diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index 12503ffd1a67f..71d3debd99d33 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -39,7 +39,7 @@ use tracing::{debug, info, trace}; use crate::errors; use crate::locator::{CrateError, CrateLocator, CratePaths}; use crate::rmeta::{ - CrateDep, CrateMetadata, CrateNumMap, CrateRoot, MetadataBlob, TargetModifiers, + CrateDep, CrateMetadata, CrateNumMap, CrateRoot, DepPrivacy, MetadataBlob, TargetModifiers, }; /// The backend's way to give the crate store access to the metadata in a library. @@ -170,8 +170,10 @@ enum CrateOrigin<'a> { IndirectDependency { /// Where this dependency was included from. dep_root: &'a CratePaths, - /// True if the parent is private, meaning the dependent should also be private. - parent_private: bool, + /// Crate number of parent dependency. + parent_crate: CrateNum, + /// Privacy of the parent dependency. + parent_privacy: DepPrivacy, /// Dependency info about this crate. dep: &'a CrateDep, }, @@ -197,17 +199,6 @@ impl<'a> CrateOrigin<'a> { _ => None, } } - - /// `Some(true)` if the dependency is private or its parent is private, `Some(false)` if the - /// dependency is not private, `None` if it could not be determined. - fn private_dep(&self) -> Option { - match self { - CrateOrigin::IndirectDependency { parent_private, dep, .. } => { - Some(dep.is_private || *parent_private) - } - _ => None, - } - } } impl CStore { @@ -541,24 +532,30 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { /// Sometimes the directly dependent crate is not specified by `--extern`, in this case, /// `private-dep` is none during loading. This is equivalent to the scenario where the /// command parameter is set to `public-dependency` - fn is_private_dep( - &self, - name: Symbol, - private_dep: Option, - origin: CrateOrigin<'_>, - ) -> bool { + fn privacy_of_dep(&self, name: Symbol, origin: CrateOrigin<'_>) -> DepPrivacy { if matches!(origin, CrateOrigin::Injected) { - return true; + return DepPrivacy::Direct(true); } let extern_private = self.sess.opts.externs.get(name.as_str()).map(|e| e.is_private_dep); - match (extern_private, private_dep) { - // Explicit non-private via `--extern`, explicit non-private from metadata, or - // unspecified with default to public. - (Some(false), _) | (_, Some(false)) | (None, None) => false, - // Marked private via `--extern priv:mycrate` or in metadata. - (Some(true) | None, Some(true) | None) => true, - } + + let dep_privacy = match origin { + CrateOrigin::IndirectDependency { dep_root: _, parent_crate, parent_privacy, dep } => { + let is_private = dep.is_private; + match parent_privacy { + _ if parent_crate == LOCAL_CRATE => DepPrivacy::Direct(is_private), + DepPrivacy::Direct(false) if !is_private => DepPrivacy::TransitivePublic, + DepPrivacy::TransitivePublic if !is_private => DepPrivacy::TransitivePublic, + DepPrivacy::ExternCrate if !is_private => DepPrivacy::TransitivePublic, + _ => DepPrivacy::TransitivePrivate, + } + } + // Short-circuited from the very beginning of this function. + CrateOrigin::Injected => unreachable!(), + CrateOrigin::Extern => DepPrivacy::ExternCrate, + }; + + extern_private.map_or(dep_privacy, |private| dep_privacy.merge(DepPrivacy::Direct(private))) } fn register_crate( @@ -568,7 +565,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { lib: Library, dep_kind: CrateDepKind, name: Symbol, - private_dep: Option, + dep_privacy: DepPrivacy, ) -> Result { let _prof_timer = self.sess.prof.generic_activity_with_arg("metadata_register_crate", name.as_str()); @@ -576,17 +573,16 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { let Library { source, metadata } = lib; let crate_root = metadata.get_root(); let host_hash = host_lib.as_ref().map(|lib| lib.metadata.get_root().hash()); - let private_dep = self.is_private_dep(name, private_dep, origin); // Claim this crate number and cache it let feed = self.cstore.intern_stable_crate_id(&crate_root, self.tcx)?; let cnum = feed.key(); info!( - "register crate `{}` (cnum = {}. private_dep = {})", + "register crate `{}` (cnum = {}. dep_privacy = {:?})", crate_root.name(), cnum, - private_dep + dep_privacy ); // Maintain a reference to the top most crate. @@ -600,7 +596,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { }; let cnum_map = - self.resolve_crate_deps(dep_root, &crate_root, &metadata, cnum, dep_kind, private_dep)?; + self.resolve_crate_deps(dep_root, &crate_root, &metadata, cnum, dep_kind, dep_privacy)?; let raw_proc_macros = if crate_root.is_proc_macro_crate() { let temp_root; @@ -627,7 +623,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { cnum_map, dep_kind, source, - private_dep, + dep_privacy, host_hash, ); @@ -720,11 +716,11 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { } } - fn maybe_resolve_crate<'b>( - &'b mut self, + fn maybe_resolve_crate( + &mut self, name: Symbol, mut dep_kind: CrateDepKind, - origin: CrateOrigin<'b>, + origin: CrateOrigin<'_>, ) -> Result { info!("resolving crate `{}`", name); if !name.as_str().is_ascii() { @@ -737,7 +733,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { let host_hash = dep.map(|d| d.host_hash).flatten(); let extra_filename = dep.map(|d| &d.extra_filename[..]); let path_kind = if dep.is_some() { PathKind::Dependency } else { PathKind::Crate }; - let private_dep = origin.private_dep(); + let dep_privacy = self.privacy_of_dep(name, origin); let result = if let Some(cnum) = self.existing_match(name, hash, path_kind) { (LoadResult::Previous(cnum), None) @@ -775,18 +771,17 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { // not specified by `--extern` on command line parameters, it may be // `private-dependency` when `register_crate` is called for the first time. Then it must be updated to // `public-dependency` here. - let private_dep = self.is_private_dep(name, private_dep, origin); let data = self.cstore.get_crate_data_mut(cnum); if data.is_proc_macro_crate() { dep_kind = CrateDepKind::MacrosOnly; } data.set_dep_kind(cmp::max(data.dep_kind(), dep_kind)); - data.update_and_private_dep(private_dep); + data.update_merge_dep_privacy(dep_privacy); Ok(cnum) } (LoadResult::Loaded(library), host_library) => { info!("register newly loaded library for `{}`", name); - self.register_crate(host_library, origin, library, dep_kind, name, private_dep) + self.register_crate(host_library, origin, library, dep_kind, name, dep_privacy) } _ => panic!(), } @@ -822,7 +817,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { metadata: &MetadataBlob, krate: CrateNum, dep_kind: CrateDepKind, - parent_is_private: bool, + crate_root_privacy: DepPrivacy, ) -> Result { debug!( "resolving deps of external crate `{}` with dep root `{}`", @@ -857,7 +852,8 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { dep_kind, CrateOrigin::IndirectDependency { dep_root, - parent_private: parent_is_private, + parent_crate: krate, + parent_privacy: crate_root_privacy, dep: &dep, }, )?; diff --git a/compiler/rustc_metadata/src/rmeta/decoder.rs b/compiler/rustc_metadata/src/rmeta/decoder.rs index ef0267eb3d908..0e0c51ad29793 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder.rs @@ -122,7 +122,7 @@ pub(crate) struct CrateMetadata { /// Whether or not this crate should be consider a private dependency. /// Used by the 'exported_private_dependencies' lint, and for determining /// whether to emit suggestions that reference this crate. - private_dep: bool, + dep_privacy: DepPrivacy, /// The hash for the host proc macro. Used to support `-Z dual-proc-macro`. host_hash: Option, /// The crate was used non-speculatively. @@ -153,6 +153,50 @@ struct ImportedSourceFile { translated_source_file: Arc, } +#[derive(Clone, Copy, Debug)] +pub(crate) enum DepPrivacy { + /// Direct dependency, with privacy of + Direct(bool), + /// This is a transitive dependency and the dep graph from `LOCAL_CRATE` to this is + /// all public. + TransitivePublic, + /// This is a transitive dependency and one or more edge of dep graph from `LOCAL_CRATE` + /// to this is private. + TransitivePrivate, + /// From `extern crate`. This is considered as public dependency unless + /// `--extern priv:` is set + ExternCrate, +} + +impl DepPrivacy { + pub(crate) fn is_private(self) -> bool { + let res = match self { + Self::Direct(private) => private, + Self::TransitivePublic => false, + Self::TransitivePrivate => true, + Self::ExternCrate => false, + }; + res + } + + pub(crate) fn merge(self, other: Self) -> Self { + use DepPrivacy::*; + + match (self, other) { + (Direct(a), Direct(b)) => Direct(a && b), + // If a dependency is directly private but is re-exported as a public dependency + // in a "chain of transitive public dependencies," treat it as public. + (Direct(true), TransitivePublic) | (TransitivePublic, Direct(true)) => TransitivePublic, + // Otherwise, prioritize direct privacy + (Direct(a), _) | (_, Direct(a)) => Direct(a), + // `extern crate` are public unless they are explicitly private + (ExternCrate, _) | (_, ExternCrate) => ExternCrate, + (TransitivePublic, _) | (_, TransitivePublic) => TransitivePublic, + _ => other, + } + } +} + pub(super) struct DecodeContext<'a, 'tcx> { opaque: MemDecoder<'a>, cdata: Option>, @@ -1837,7 +1881,7 @@ impl CrateMetadata { cnum_map: CrateNumMap, dep_kind: CrateDepKind, source: CrateSource, - private_dep: bool, + dep_privacy: DepPrivacy, host_hash: Option, ) -> CrateMetadata { let trait_impls = root @@ -1868,7 +1912,7 @@ impl CrateMetadata { dependencies, dep_kind, source: Arc::new(source), - private_dep, + dep_privacy, host_hash, used: false, extern_crate: None, @@ -1920,8 +1964,8 @@ impl CrateMetadata { self.dep_kind = dep_kind; } - pub(crate) fn update_and_private_dep(&mut self, private_dep: bool) { - self.private_dep &= private_dep; + pub(crate) fn update_merge_dep_privacy(&mut self, dep_privacy: DepPrivacy) { + self.dep_privacy = self.dep_privacy.merge(dep_privacy); } pub(crate) fn used(&self) -> bool { @@ -1937,7 +1981,7 @@ impl CrateMetadata { } pub(crate) fn is_private_dep(&self) -> bool { - self.private_dep + self.dep_privacy.is_private() } pub(crate) fn is_panic_runtime(&self) -> bool { diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 776b081a4630f..104cf9ea5f15f 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -349,7 +349,7 @@ provide! { tcx, def_id, other, cdata, cross_crate_inlinable => { table_direct } dylib_dependency_formats => { cdata.get_dylib_dependency_formats(tcx) } - is_private_dep => { cdata.private_dep } + is_private_dep => { cdata.dep_privacy.is_private() } is_panic_runtime => { cdata.root.panic_runtime } is_compiler_builtins => { cdata.root.compiler_builtins } has_global_allocator => { cdata.root.has_global_allocator } diff --git a/compiler/rustc_metadata/src/rmeta/mod.rs b/compiler/rustc_metadata/src/rmeta/mod.rs index 7b34e605c5307..44a86577a07a0 100644 --- a/compiler/rustc_metadata/src/rmeta/mod.rs +++ b/compiler/rustc_metadata/src/rmeta/mod.rs @@ -1,7 +1,7 @@ use std::marker::PhantomData; use std::num::NonZero; -pub(crate) use decoder::{CrateMetadata, CrateNumMap, MetadataBlob, TargetModifiers}; +pub(crate) use decoder::{CrateMetadata, CrateNumMap, DepPrivacy, MetadataBlob, TargetModifiers}; use decoder::{DecodeContext, Metadata}; use def_path_hash_map::DefPathHashMapRef; use encoder::EncodeContext; diff --git a/tests/ui/privacy/pub-priv-dep/shared_both_private.rs b/tests/ui/privacy/pub-priv-dep/shared_both_private.rs index 20a4b85c01e8d..ded0d351c2006 100644 --- a/tests/ui/privacy/pub-priv-dep/shared_both_private.rs +++ b/tests/ui/privacy/pub-priv-dep/shared_both_private.rs @@ -1,7 +1,6 @@ //@ aux-crate:priv:shared=shared.rs -//@ aux-crate:reexport=reexport.rs +//@ aux-crate:priv:reexport=reexport.rs //@ compile-flags: -Zunstable-options -//@ check-pass // A shared dependency, where a private dependency reexports a public dependency. // @@ -21,12 +20,12 @@ extern crate shared; extern crate reexport; -// FIXME: This should trigger. pub fn leaks_priv() -> shared::Shared { +//~^ ERROR type `Shared` from private dependency 'shared' in public interface shared::Shared } -// FIXME: This should trigger. pub fn leaks_priv_reexport() -> reexport::Shared { +//~^ ERROR type `Shared` from private dependency 'shared' in public interface reexport::Shared } diff --git a/tests/ui/privacy/pub-priv-dep/shared_both_private.stderr b/tests/ui/privacy/pub-priv-dep/shared_both_private.stderr new file mode 100644 index 0000000000000..b26e026b2d67b --- /dev/null +++ b/tests/ui/privacy/pub-priv-dep/shared_both_private.stderr @@ -0,0 +1,20 @@ +error: type `Shared` from private dependency 'shared' in public interface + --> $DIR/shared_both_private.rs:23:1 + | +LL | pub fn leaks_priv() -> shared::Shared { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/shared_both_private.rs:18:9 + | +LL | #![deny(exported_private_dependencies)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: type `Shared` from private dependency 'shared' in public interface + --> $DIR/shared_both_private.rs:28:1 + | +LL | pub fn leaks_priv_reexport() -> reexport::Shared { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 2 previous errors +