fix: Calculate privacy of dependents wrt current local crate

Author: ShoyuVanilla

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.
@@ -162,6 +162,14 @@ impl<'a> std::fmt::Debug for CrateDump<'a> {
     }
 }
 
+#[derive(Clone, Copy)]
+struct DepOf<'b> {
+    dep_root: &'b CratePaths,
+    parent: CrateNum,
+    privacy_of_parent: DepPrivacy,
+    dep: &'b CrateDep,
+}
+
 impl CStore {
     pub fn from_tcx(tcx: TyCtxt<'_>) -> FreezeReadGuard<'_, CStore> {
         FreezeReadGuard::map(tcx.untracked().cstore.read(), |cstore| {
@@ -493,36 +501,41 @@ 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<bool>,
-        dep_root: Option<&CratePaths>,
-    ) -> bool {
+    fn privacy_of_dep(&self, name: Symbol, dep_of: Option<DepOf<'_>>) -> DepPrivacy {
         // Standard library crates are never private.
         if STDLIB_STABLE_CRATES.contains(&name) {
             tracing::info!("returning false for {name} is private");
-            return false;
+            return DepPrivacy::Direct(false);
         }
 
         let extern_private = self.sess.opts.externs.get(name.as_str()).map(|e| e.is_private_dep);
 
         // Any descendants of `std` should be private. These crates are usually not marked
         // private in metadata, so we ignore that field.
         if extern_private.is_none()
-            && let Some(dep) = dep_root
+            && let Some(dep) = dep_of.map(|d| d.dep_root)
             && STDLIB_STABLE_CRATES.contains(&dep.name)
         {
-            return true;
+            return DepPrivacy::Direct(true);
         }
 
-        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 = if let Some(dep_of) = dep_of {
+            let is_private = dep_of.dep.is_private;
+            if dep_of.parent == LOCAL_CRATE {
+                DepPrivacy::Direct(is_private)
+            } else {
+                match dep_of.privacy_of_parent {
+                    DepPrivacy::Direct(false) if !is_private => DepPrivacy::TransitivePublic,
+                    DepPrivacy::TransitivePublic if !is_private => DepPrivacy::TransitivePublic,
+                    DepPrivacy::ExternCrate if !is_private => DepPrivacy::TransitivePublic,
+                    _ => DepPrivacy::TransitivePrivate,
+                }
+            }
+        } else {
+            DepPrivacy::ExternCrate
+        };
+
+        extern_private.map_or(dep_privacy, |private| dep_privacy.merge(DepPrivacy::Direct(private)))
     }
 
     fn register_crate(
@@ -532,25 +545,24 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         lib: Library,
         dep_kind: CrateDepKind,
         name: Symbol,
-        private_dep: Option<bool>,
+        dep_privacy: DepPrivacy,
     ) -> Result<CrateNum, CrateError> {
         let _prof_timer =
             self.sess.prof.generic_activity_with_arg("metadata_register_crate", name.as_str());
 
         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, dep_root);
 
         // 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.
@@ -563,7 +575,8 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
             &crate_paths
         };
 
-        let cnum_map = self.resolve_crate_deps(dep_root, &crate_root, &metadata, cnum, dep_kind)?;
+        let cnum_map =
+            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;
@@ -590,7 +603,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
             cnum_map,
             dep_kind,
             source,
-            private_dep,
+            dep_privacy,
             host_hash,
         );
 
@@ -681,24 +694,24 @@ 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,
-        dep_of: Option<(&'b CratePaths, &'b CrateDep)>,
+        dep_of: Option<DepOf<'_>>,
     ) -> Result<CrateNum, CrateError> {
         info!("resolving crate `{}`", name);
         if !name.as_str().is_ascii() {
             return Err(CrateError::NonAsciiName(name));
         }
 
-        let dep_root = dep_of.map(|d| d.0);
-        let dep = dep_of.map(|d| d.1);
+        let dep_root = dep_of.map(|d| d.dep_root);
+        let dep = dep_of.map(|d| d.dep);
         let hash = dep.map(|d| d.hash);
         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 = dep.map(|d| d.is_private);
+        let dep_privacy = self.privacy_of_dep(name, dep_of);
 
         let result = if let Some(cnum) = self.existing_match(name, hash, path_kind) {
             (LoadResult::Previous(cnum), None)
@@ -732,22 +745,22 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         match result {
             (LoadResult::Previous(cnum), None) => {
                 info!("library for `{}` was loaded previously", name);
-                // When `private_dep` is none, it indicates the directly dependent crate. If it is
+                // When `dep_of` is none, it indicates the directly dependent crate. If it is
                 // 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, dep_root);
                 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, dep_root, library, dep_kind, name, private_dep)
+
+                self.register_crate(host_library, dep_root, library, dep_kind, name, dep_privacy)
             }
             _ => panic!(),
         }
@@ -795,6 +808,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
         metadata: &MetadataBlob,
         krate: CrateNum,
         dep_kind: CrateDepKind,
+        crate_root_privacy: DepPrivacy,
     ) -> Result<CrateNumMap, CrateError> {
         debug!(
             "resolving deps of external crate `{}` with dep root `{}`",
@@ -823,7 +837,9 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
                 CrateDepKind::MacrosOnly => CrateDepKind::MacrosOnly,
                 _ => dep.kind,
             };
-            let cnum = self.maybe_resolve_crate(dep.name, dep_kind, Some((dep_root, &dep)))?;
+            let dep_of =
+                DepOf { dep_root, parent: krate, privacy_of_parent: crate_root_privacy, dep: &dep };
+            let cnum = self.maybe_resolve_crate(dep.name, dep_kind, Some(dep_of))?;
             crate_num_map.push(cnum);
         }
 

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<Svh>,
     /// The crate was used non-speculatively.
@@ -153,6 +153,50 @@ struct ImportedSourceFile {
     translated_source_file: Arc<rustc_span::SourceFile>,
 }
 
+#[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<CrateMetadataRef<'a>>,
@@ -1837,7 +1881,7 @@ impl CrateMetadata {
         cnum_map: CrateNumMap,
         dep_kind: CrateDepKind,
         source: CrateSource,
-        private_dep: bool,
+        dep_privacy: DepPrivacy,
         host_hash: Option<Svh>,
     ) -> 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 {

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 }

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;

tests/ui/privacy/pub-priv-dep/reexport_from_priv.rs

@@ -1,6 +1,5 @@
 //@ aux-crate:priv:reexport=reexport.rs
 //@ compile-flags: -Zunstable-options
-//@ check-pass
 
 // Checks the behavior of a reexported item from a private dependency.
 
@@ -9,7 +8,7 @@
 
 extern crate reexport;
 
-// FIXME: This should trigger.
 pub fn leaks_priv() -> reexport::Shared {
+//~^ ERROR type `Shared` from private dependency 'shared' in public interface
     reexport::Shared
 }

tests/ui/privacy/pub-priv-dep/reexport_from_priv.stderr

@@ -0,0 +1,14 @@
+error: type `Shared` from private dependency 'shared' in public interface
+  --> $DIR/reexport_from_priv.rs:11:1
+   |
+LL | pub fn leaks_priv() -> reexport::Shared {
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: the lint level is defined here
+  --> $DIR/reexport_from_priv.rs:7:9
+   |
+LL | #![deny(exported_private_dependencies)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+error: aborting due to 1 previous error
+

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
 }

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
+