Skip to content

Commit aca5b5d

Browse files
committed
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]: rust-lang#135501 (comment)
1 parent 8c1b49d commit aca5b5d

File tree

5 files changed

+56
-9
lines changed

5 files changed

+56
-9
lines changed

compiler/rustc_metadata/src/creader.rs

+26-5
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,10 @@ impl<'a> std::fmt::Debug for CrateDump<'a> {
168168
enum CrateOrigin<'a> {
169169
/// This crate was a dependency of another crate.
170170
IndirectDependency {
171+
/// Where this dependency was included from.
171172
dep_root: &'a CratePaths,
173+
/// True if the parent is private, meaning the dependent should also be private.
174+
parent_private: bool,
172175
/// Dependency info about this crate.
173176
dep: &'a CrateDep,
174177
},
@@ -194,6 +197,17 @@ impl<'a> CrateOrigin<'a> {
194197
_ => None,
195198
}
196199
}
200+
201+
/// `Some(true)` if the dependency is private or its parent is private, `Some(false)` if the
202+
/// dependency is not private, `None` if it could not be determined.
203+
fn private_dep(&self) -> Option<bool> {
204+
match self {
205+
CrateOrigin::IndirectDependency { parent_private, dep, .. } => {
206+
Some(dep.is_private || *parent_private)
207+
}
208+
_ => None,
209+
}
210+
}
197211
}
198212

199213
impl CStore {
@@ -585,7 +599,8 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
585599
&crate_paths
586600
};
587601

588-
let cnum_map = self.resolve_crate_deps(dep_root, &crate_root, &metadata, cnum, dep_kind)?;
602+
let cnum_map =
603+
self.resolve_crate_deps(dep_root, &crate_root, &metadata, cnum, dep_kind, private_dep)?;
589604

590605
let raw_proc_macros = if crate_root.is_proc_macro_crate() {
591606
let temp_root;
@@ -722,7 +737,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
722737
let host_hash = dep.map(|d| d.host_hash).flatten();
723738
let extra_filename = dep.map(|d| &d.extra_filename[..]);
724739
let path_kind = if dep.is_some() { PathKind::Dependency } else { PathKind::Crate };
725-
let private_dep = dep.map(|d| d.is_private);
740+
let private_dep = origin.private_dep();
726741

727742
let result = if let Some(cnum) = self.existing_match(name, hash, path_kind) {
728743
(LoadResult::Previous(cnum), None)
@@ -819,6 +834,7 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
819834
metadata: &MetadataBlob,
820835
krate: CrateNum,
821836
dep_kind: CrateDepKind,
837+
parent_is_private: bool,
822838
) -> Result<CrateNumMap, CrateError> {
823839
debug!(
824840
"resolving deps of external crate `{}` with dep root `{}`",
@@ -837,11 +853,12 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
837853
crate_num_map.push(krate);
838854
for dep in deps {
839855
info!(
840-
"resolving dep `{}`->`{}` hash: `{}` extra filename: `{}`",
856+
"resolving dep `{}`->`{}` hash: `{}` extra filename: `{}` private {}",
841857
crate_root.name(),
842858
dep.name,
843859
dep.hash,
844-
dep.extra_filename
860+
dep.extra_filename,
861+
dep.is_private,
845862
);
846863
let dep_kind = match dep_kind {
847864
CrateDepKind::MacrosOnly => CrateDepKind::MacrosOnly,
@@ -850,7 +867,11 @@ impl<'a, 'tcx> CrateLoader<'a, 'tcx> {
850867
let cnum = self.maybe_resolve_crate(
851868
dep.name,
852869
dep_kind,
853-
CrateOrigin::IndirectDependency { dep_root, dep: &dep },
870+
CrateOrigin::IndirectDependency {
871+
dep_root,
872+
parent_private: parent_is_private,
873+
dep: &dep,
874+
},
854875
)?;
855876
crate_num_map.push(cnum);
856877
}
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
//@ aux-crate:priv:reexport=reexport.rs
22
//@ compile-flags: -Zunstable-options
3-
//@ check-pass
43

54
// Checks the behavior of a reexported item from a private dependency.
65

@@ -9,7 +8,7 @@
98

109
extern crate reexport;
1110

12-
// FIXME: This should trigger.
1311
pub fn leaks_priv() -> reexport::Shared {
12+
//~^ ERROR type `Shared` from private dependency 'shared' in public interface
1413
reexport::Shared
1514
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: type `Shared` from private dependency 'shared' in public interface
2+
--> $DIR/reexport_from_priv.rs:11:1
3+
|
4+
LL | pub fn leaks_priv() -> reexport::Shared {
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/reexport_from_priv.rs:7:9
9+
|
10+
LL | #![deny(exported_private_dependencies)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: aborting due to 1 previous error
14+

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

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
//@ aux-crate:priv:shared=shared.rs
22
//@ aux-crate:priv:indirect1=indirect1.rs
33
//@ compile-flags: -Zunstable-options
4-
//@ check-pass
54

65
// A shared dependency, where it is only indirectly public.
76
//
@@ -23,7 +22,7 @@
2322
extern crate shared;
2423
extern crate indirect1;
2524

26-
// FIXME: This should trigger.
2725
pub fn leaks_priv() -> shared::Shared {
26+
//~^ ERROR type `Shared` from private dependency 'shared' in public interface
2827
shared::Shared
2928
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: type `Shared` from private dependency 'shared' in public interface
2+
--> $DIR/shared_indirect.rs:25:1
3+
|
4+
LL | pub fn leaks_priv() -> shared::Shared {
5+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/shared_indirect.rs:20:9
9+
|
10+
LL | #![deny(exported_private_dependencies)]
11+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
12+
13+
error: aborting due to 1 previous error
14+

0 commit comments

Comments
 (0)