From aca5b5dd5260d4e2c1bf584fc6ba36b5fba239cb Mon Sep 17 00:00:00 2001 From: Trevor Gross Date: Tue, 28 Jan 2025 23:20:36 +0000 Subject: [PATCH] If the parent dependency is private, treat dependents as private Currently, marking a dependency private does not automatically make all its child dependencies private. Resolve this by making its children private by default as well. This also resolves some FIXMEs for tests that are intended to fail but previously passed. [1]: https://github.com/rust-lang/rust/pull/135501#issuecomment-2620242419 --- compiler/rustc_metadata/src/creader.rs | 31 ++++++++++++++++--- .../pub-priv-dep/reexport_from_priv.rs | 3 +- .../pub-priv-dep/reexport_from_priv.stderr | 14 +++++++++ .../privacy/pub-priv-dep/shared_indirect.rs | 3 +- .../pub-priv-dep/shared_indirect.stderr | 14 +++++++++ 5 files changed, 56 insertions(+), 9 deletions(-) create mode 100644 tests/ui/privacy/pub-priv-dep/reexport_from_priv.stderr create mode 100644 tests/ui/privacy/pub-priv-dep/shared_indirect.stderr diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index b94e32e17d41..58ff2425748f 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -168,7 +168,10 @@ impl<'a> std::fmt::Debug for CrateDump<'a> { enum CrateOrigin<'a> { /// This crate was a dependency of another crate. 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, /// Dependency info about this crate. dep: &'a CrateDep, }, @@ -194,6 +197,17 @@ 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 { @@ -585,7 +599,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, private_dep)?; let raw_proc_macros = if crate_root.is_proc_macro_crate() { let temp_root; @@ -722,7 +737,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 = dep.map(|d| d.is_private); + let private_dep = origin.private_dep(); let result = if let Some(cnum) = self.existing_match(name, hash, path_kind) { (LoadResult::Previous(cnum), None) @@ -819,6 +834,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { metadata: &MetadataBlob, krate: CrateNum, dep_kind: CrateDepKind, + parent_is_private: bool, ) -> Result { debug!( "resolving deps of external crate `{}` with dep root `{}`", @@ -837,11 +853,12 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { crate_num_map.push(krate); for dep in deps { info!( - "resolving dep `{}`->`{}` hash: `{}` extra filename: `{}`", + "resolving dep `{}`->`{}` hash: `{}` extra filename: `{}` private {}", crate_root.name(), dep.name, dep.hash, - dep.extra_filename + dep.extra_filename, + dep.is_private, ); let dep_kind = match dep_kind { CrateDepKind::MacrosOnly => CrateDepKind::MacrosOnly, @@ -850,7 +867,11 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> { let cnum = self.maybe_resolve_crate( dep.name, dep_kind, - CrateOrigin::IndirectDependency { dep_root, dep: &dep }, + CrateOrigin::IndirectDependency { + dep_root, + parent_private: parent_is_private, + dep: &dep, + }, )?; crate_num_map.push(cnum); } diff --git a/tests/ui/privacy/pub-priv-dep/reexport_from_priv.rs b/tests/ui/privacy/pub-priv-dep/reexport_from_priv.rs index 3c6e9825e728..136e0ceeb5be 100644 --- a/tests/ui/privacy/pub-priv-dep/reexport_from_priv.rs +++ b/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 } diff --git a/tests/ui/privacy/pub-priv-dep/reexport_from_priv.stderr b/tests/ui/privacy/pub-priv-dep/reexport_from_priv.stderr new file mode 100644 index 000000000000..f1573283ff2e --- /dev/null +++ b/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 + diff --git a/tests/ui/privacy/pub-priv-dep/shared_indirect.rs b/tests/ui/privacy/pub-priv-dep/shared_indirect.rs index 34b624b4a1a4..2fe66ae80cf4 100644 --- a/tests/ui/privacy/pub-priv-dep/shared_indirect.rs +++ b/tests/ui/privacy/pub-priv-dep/shared_indirect.rs @@ -1,7 +1,6 @@ //@ aux-crate:priv:shared=shared.rs //@ aux-crate:priv:indirect1=indirect1.rs //@ compile-flags: -Zunstable-options -//@ check-pass // A shared dependency, where it is only indirectly public. // @@ -23,7 +22,7 @@ extern crate shared; extern crate indirect1; -// FIXME: This should trigger. pub fn leaks_priv() -> shared::Shared { + //~^ ERROR type `Shared` from private dependency 'shared' in public interface shared::Shared } diff --git a/tests/ui/privacy/pub-priv-dep/shared_indirect.stderr b/tests/ui/privacy/pub-priv-dep/shared_indirect.stderr new file mode 100644 index 000000000000..dbc534713d16 --- /dev/null +++ b/tests/ui/privacy/pub-priv-dep/shared_indirect.stderr @@ -0,0 +1,14 @@ +error: type `Shared` from private dependency 'shared' in public interface + --> $DIR/shared_indirect.rs:25:1 + | +LL | pub fn leaks_priv() -> shared::Shared { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: the lint level is defined here + --> $DIR/shared_indirect.rs:20:9 + | +LL | #![deny(exported_private_dependencies)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +error: aborting due to 1 previous error +