fix: Calculate privacy of dependents wrt current local crate

ShoyuVanilla · ShoyuVanilla · commit a34b7fd9f590 · 2025-02-23T17:26:44.000+09:00
diff --git 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<bool> {
-        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<bool>,
-        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,25 +565,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, 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<CrateNum, CrateError> {
         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<CrateNumMap, CrateError> {
         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
@@ -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 {
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
+
