diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs index e4890977c9bd6..d22de6c647699 100644 --- a/src/librustc/middle/cstore.rs +++ b/src/librustc/middle/cstore.rs @@ -199,6 +199,7 @@ pub trait CrateStore { // "queries" used in resolve that aren't tracked for incremental compilation fn crate_name_untracked(&self, cnum: CrateNum) -> Symbol; + fn crate_is_private_dep_untracked(&self, cnum: CrateNum) -> bool; fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> CrateDisambiguator; fn crate_hash_untracked(&self, cnum: CrateNum) -> Svh; fn extern_mod_stmt_cnum_untracked(&self, emod_id: ast::NodeId) -> Option; diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 7c0eab26b09b6..d3a734cbc6ead 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -283,22 +283,29 @@ impl OutputTypes { // DO NOT switch BTreeMap or BTreeSet out for an unsorted container type! That // would break dependency tracking for command-line arguments. #[derive(Clone, Hash)] -pub struct Externs(BTreeMap>>); +pub struct Externs(BTreeMap); + +#[derive(Clone, Hash, Eq, PartialEq, Ord, PartialOrd, Debug, Default)] +pub struct ExternEntry { + pub locations: BTreeSet>, + pub is_private_dep: bool +} impl Externs { - pub fn new(data: BTreeMap>>) -> Externs { + pub fn new(data: BTreeMap) -> Externs { Externs(data) } - pub fn get(&self, key: &str) -> Option<&BTreeSet>> { + pub fn get(&self, key: &str) -> Option<&ExternEntry> { self.0.get(key) } - pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, BTreeSet>> { + pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, ExternEntry> { self.0.iter() } } + macro_rules! hash_option { ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, [UNTRACKED]) => ({}); ($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, [TRACKED]) => ({ @@ -427,10 +434,6 @@ top_level_options!( remap_path_prefix: Vec<(PathBuf, PathBuf)> [UNTRACKED], edition: Edition [TRACKED], - - // The list of crates to consider private when - // checking leaked private dependency types in public interfaces - extern_private: Vec [TRACKED], } ); @@ -633,7 +636,6 @@ impl Default for Options { cli_forced_thinlto_off: false, remap_path_prefix: Vec::new(), edition: DEFAULT_EDITION, - extern_private: Vec::new() } } } @@ -2315,10 +2317,14 @@ pub fn build_session_options_and_crate_config( ) } - let extern_private = matches.opt_strs("extern-private"); + // We start out with a Vec<(Option, bool)>>, + // and later convert it into a BTreeSet<(Option, bool)> + // This allows to modify entries in-place to set their correct + // 'public' value + let mut externs: BTreeMap = BTreeMap::new(); + for (arg, private) in matches.opt_strs("extern").into_iter().map(|v| (v, false)) + .chain(matches.opt_strs("extern-private").into_iter().map(|v| (v, true))) { - let mut externs: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); - for arg in matches.opt_strs("extern").into_iter().chain(matches.opt_strs("extern-private")) { let mut parts = arg.splitn(2, '='); let name = parts.next().unwrap_or_else(|| early_error(error_format, "--extern value must not be empty")); @@ -2331,10 +2337,17 @@ pub fn build_session_options_and_crate_config( ); }; - externs + let entry = externs .entry(name.to_owned()) - .or_default() - .insert(location); + .or_default(); + + + entry.locations.insert(location.clone()); + + // Crates start out being not private, + // and go to being private if we see an '--extern-private' + // flag + entry.is_private_dep |= private; } let crate_name = matches.opt_str("crate-name"); @@ -2386,7 +2399,6 @@ pub fn build_session_options_and_crate_config( cli_forced_thinlto_off: disable_thinlto, remap_path_prefix, edition, - extern_private }, cfg, ) @@ -2651,7 +2663,7 @@ mod tests { build_session_options_and_crate_config, to_crate_config }; - use crate::session::config::{LtoCli, LinkerPluginLto, PgoGenerate}; + use crate::session::config::{LtoCli, LinkerPluginLto, PgoGenerate, ExternEntry}; use crate::session::build_session; use crate::session::search_paths::SearchPath; use std::collections::{BTreeMap, BTreeSet}; @@ -2664,6 +2676,19 @@ mod tests { use syntax; use super::Options; + impl ExternEntry { + fn new_public, + I: IntoIterator>>(locations: I) -> ExternEntry { + let locations: BTreeSet<_> = locations.into_iter().map(|o| o.map(|s| s.into())) + .collect(); + + ExternEntry { + locations, + is_private_dep: false + } + } + } + fn optgroups() -> getopts::Options { let mut opts = getopts::Options::new(); for group in super::rustc_optgroups() { @@ -2676,10 +2701,6 @@ mod tests { BTreeMap::from_iter(entries.into_iter()) } - fn mk_set(entries: Vec) -> BTreeSet { - BTreeSet::from_iter(entries.into_iter()) - } - // When the user supplies --test we should implicitly supply --cfg test #[test] fn test_switch_implies_cfg_test() { @@ -2797,33 +2818,33 @@ mod tests { v1.externs = Externs::new(mk_map(vec![ ( String::from("a"), - mk_set(vec![Some(String::from("b")), Some(String::from("c"))]), + ExternEntry::new_public(vec![Some("b"), Some("c")]) ), ( String::from("d"), - mk_set(vec![Some(String::from("e")), Some(String::from("f"))]), + ExternEntry::new_public(vec![Some("e"), Some("f")]) ), ])); v2.externs = Externs::new(mk_map(vec![ ( String::from("d"), - mk_set(vec![Some(String::from("e")), Some(String::from("f"))]), + ExternEntry::new_public(vec![Some("e"), Some("f")]) ), ( String::from("a"), - mk_set(vec![Some(String::from("b")), Some(String::from("c"))]), + ExternEntry::new_public(vec![Some("b"), Some("c")]) ), ])); v3.externs = Externs::new(mk_map(vec![ ( String::from("a"), - mk_set(vec![Some(String::from("b")), Some(String::from("c"))]), + ExternEntry::new_public(vec![Some("b"), Some("c")]) ), ( String::from("d"), - mk_set(vec![Some(String::from("f")), Some(String::from("e"))]), + ExternEntry::new_public(vec![Some("f"), Some("e")]) ), ])); diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs index 7dc4dee3fbf91..8bfdd0801d40f 100644 --- a/src/librustc/ty/context.rs +++ b/src/librustc/ty/context.rs @@ -1391,6 +1391,16 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { } } + /// Returns whether or not the crate with CrateNum 'cnum' + /// is marked as a private dependency + pub fn is_private_dep(self, cnum: CrateNum) -> bool { + if cnum == LOCAL_CRATE { + false + } else { + self.cstore.crate_is_private_dep_untracked(cnum) + } + } + #[inline] pub fn def_path_hash(self, def_id: DefId) -> hir_map::DefPathHash { if def_id.is_local() { diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 66daa4518bef6..160d4c30c0bad 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -131,9 +131,9 @@ impl<'a> CrateLoader<'a> { // `source` stores paths which are normalized which may be different // from the strings on the command line. let source = &self.cstore.get_crate_data(cnum).source; - if let Some(locs) = self.sess.opts.externs.get(&*name.as_str()) { + if let Some(entry) = self.sess.opts.externs.get(&*name.as_str()) { // Only use `--extern crate_name=path` here, not `--extern crate_name`. - let found = locs.iter().filter_map(|l| l.as_ref()).any(|l| { + let found = entry.locations.iter().filter_map(|l| l.as_ref()).any(|l| { let l = fs::canonicalize(l).ok(); source.dylib.as_ref().map(|p| &p.0) == l.as_ref() || source.rlib.as_ref().map(|p| &p.0) == l.as_ref() @@ -195,12 +195,20 @@ impl<'a> CrateLoader<'a> { ident: Symbol, span: Span, lib: Library, - dep_kind: DepKind + dep_kind: DepKind, + name: Symbol ) -> (CrateNum, Lrc) { let crate_root = lib.metadata.get_root(); - info!("register crate `extern crate {} as {}`", crate_root.name, ident); self.verify_no_symbol_conflicts(span, &crate_root); + let private_dep = self.sess.opts.externs.get(&name.as_str()) + .map(|e| e.is_private_dep) + .unwrap_or(false); + + info!("register crate `extern crate {} as {}` (private_dep = {})", + crate_root.name, ident, private_dep); + + // Claim this crate number and cache it let cnum = self.cstore.alloc_new_crate_num(); @@ -272,7 +280,8 @@ impl<'a> CrateLoader<'a> { dylib, rlib, rmeta, - } + }, + private_dep }; let cmeta = Lrc::new(cmeta); @@ -390,7 +399,7 @@ impl<'a> CrateLoader<'a> { Ok((cnum, data)) } (LoadResult::Loaded(library), host_library) => { - Ok(self.register_crate(host_library, root, ident, span, library, dep_kind)) + Ok(self.register_crate(host_library, root, ident, span, library, dep_kind, name)) } _ => panic!() } diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs index d646879b4d45d..22a13f37722b8 100644 --- a/src/librustc_metadata/cstore.rs +++ b/src/librustc_metadata/cstore.rs @@ -79,6 +79,10 @@ pub struct CrateMetadata { pub source: CrateSource, pub proc_macros: Option)>>, + + /// Whether or not this crate should be consider a private dependency + /// for purposes of the 'exported_private_dependencies' lint + pub private_dep: bool } pub struct CStore { @@ -114,7 +118,8 @@ impl CStore { } pub(super) fn get_crate_data(&self, cnum: CrateNum) -> Lrc { - self.metas.borrow()[cnum].clone().unwrap() + self.metas.borrow()[cnum].clone() + .unwrap_or_else(|| panic!("Failed to get crate data for {:?}", cnum)) } pub(super) fn set_crate_data(&self, cnum: CrateNum, data: Lrc) { diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 995532a00cd6e..95fe9a70f750a 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -494,6 +494,10 @@ impl CrateStore for cstore::CStore { self.get_crate_data(cnum).name } + fn crate_is_private_dep_untracked(&self, cnum: CrateNum) -> bool { + self.get_crate_data(cnum).private_dep + } + fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> CrateDisambiguator { self.get_crate_data(cnum).root.disambiguator diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs index 81878c4f687b6..116042c53fb9e 100644 --- a/src/librustc_metadata/locator.rs +++ b/src/librustc_metadata/locator.rs @@ -442,11 +442,11 @@ impl<'a> Context<'a> { // must be loaded via -L plus some filtering. if self.hash.is_none() { self.should_match_name = false; - if let Some(s) = self.sess.opts.externs.get(&self.crate_name.as_str()) { + if let Some(entry) = self.sess.opts.externs.get(&self.crate_name.as_str()) { // Only use `--extern crate_name=path` here, not `--extern crate_name`. - if s.iter().any(|l| l.is_some()) { + if entry.locations.iter().any(|l| l.is_some()) { return self.find_commandline_library( - s.iter().filter_map(|l| l.as_ref()), + entry.locations.iter().filter_map(|l| l.as_ref()), ); } } diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 9a8970b2935e0..44621e5dc95d1 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -1540,7 +1540,6 @@ struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> { has_pub_restricted: bool, has_old_errors: bool, in_assoc_ty: bool, - private_crates: FxHashSet } impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { @@ -1622,7 +1621,7 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> { /// 2. It comes from a private crate fn leaks_private_dep(&self, item_id: DefId) -> bool { let ret = self.required_visibility == ty::Visibility::Public && - self.private_crates.contains(&item_id.krate); + self.tcx.is_private_dep(item_id.krate); log::debug!("leaks_private_dep(item_id={:?})={}", item_id, ret); return ret; @@ -1640,7 +1639,6 @@ struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, has_pub_restricted: bool, old_error_set: &'a HirIdSet, - private_crates: FxHashSet } impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { @@ -1678,7 +1676,6 @@ impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> { has_pub_restricted: self.has_pub_restricted, has_old_errors, in_assoc_ty: false, - private_crates: self.private_crates.clone() } } @@ -1876,17 +1873,11 @@ fn check_private_in_public<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, krate: CrateNum) { pub_restricted_visitor.has_pub_restricted }; - let private_crates: FxHashSet = tcx.sess.opts.extern_private.iter() - .flat_map(|c| { - tcx.crates().iter().find(|&&krate| &tcx.crate_name(krate) == c).cloned() - }).collect(); - // Check for private types and traits in public interfaces. let mut visitor = PrivateItemsInPublicInterfacesVisitor { tcx, has_pub_restricted, old_error_set: &visitor.old_error_set, - private_crates }; krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); } diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index f2682e00430d0..a4d2a3be86330 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -1,4 +1,4 @@ -use std::collections::{BTreeMap, BTreeSet}; +use std::collections::BTreeMap; use std::fmt; use std::path::PathBuf; @@ -9,7 +9,7 @@ use rustc::lint::Level; use rustc::session::early_error; use rustc::session::config::{CodegenOptions, DebuggingOptions, ErrorOutputType, Externs}; use rustc::session::config::{nightly_options, build_codegen_options, build_debugging_options, - get_cmd_lint_options}; + get_cmd_lint_options, ExternEntry}; use rustc::session::search_paths::SearchPath; use rustc_driver; use rustc_target::spec::TargetTriple; @@ -578,7 +578,7 @@ fn parse_extern_html_roots( /// error message. // FIXME(eddyb) This shouldn't be duplicated with `rustc::session`. fn parse_externs(matches: &getopts::Matches) -> Result { - let mut externs: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); + let mut externs: BTreeMap<_, ExternEntry> = BTreeMap::new(); for arg in &matches.opt_strs("extern") { let mut parts = arg.splitn(2, '='); let name = parts.next().ok_or("--extern value must not be empty".to_string())?; @@ -588,7 +588,10 @@ fn parse_externs(matches: &getopts::Matches) -> Result { enable `--extern crate_name` without `=path`".to_string()); } let name = name.to_string(); - externs.entry(name).or_default().insert(location); + // For Rustdoc purposes, we can treat all externs as public + externs.entry(name) + .or_default() + .locations.insert(location.clone()); } Ok(Externs::new(externs)) } diff --git a/src/test/ui/privacy/pub-priv-dep/pub-priv1.rs b/src/test/ui/privacy/pub-priv-dep/pub-priv1.rs index 9ebc96017fe9c..784615354a95c 100644 --- a/src/test/ui/privacy/pub-priv-dep/pub-priv1.rs +++ b/src/test/ui/privacy/pub-priv-dep/pub-priv1.rs @@ -1,6 +1,6 @@ // aux-build:priv_dep.rs // aux-build:pub_dep.rs - // compile-flags: --extern-private priv_dep + // extern-private:priv_dep #![deny(exported_private_dependencies)] // This crate is a private dependency diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 2fe837e99d33f..64882c603bad3 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -286,6 +286,10 @@ pub struct TestProps { // directory as the test, but for backwards compatibility reasons // we also check the auxiliary directory) pub aux_builds: Vec, + // A list of crates to pass '--extern-private name:PATH' flags for + // This should be a subset of 'aux_build' + // FIXME: Replace this with a better solution: https://github.com/rust-lang/rust/pull/54020 + pub extern_private: Vec, // Environment settings to use for compiling pub rustc_env: Vec<(String, String)>, // Environment settings to use during execution @@ -353,6 +357,7 @@ impl TestProps { run_flags: None, pp_exact: None, aux_builds: vec![], + extern_private: vec![], revisions: vec![], rustc_env: vec![], exec_env: vec![], @@ -469,6 +474,10 @@ impl TestProps { self.aux_builds.push(ab); } + if let Some(ep) = config.parse_extern_private(ln) { + self.extern_private.push(ep); + } + if let Some(ee) = config.parse_env(ln, "exec-env") { self.exec_env.push(ee); } @@ -610,6 +619,10 @@ impl Config { .map(|r| r.trim().to_string()) } + fn parse_extern_private(&self, line: &str) -> Option { + self.parse_name_value_directive(line, "extern-private") + } + fn parse_compile_flags(&self, line: &str) -> Option { self.parse_name_value_directive(line, "compile-flags") } diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index dfc023da9736b..9e3c49119deaf 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -1,5 +1,6 @@ #![crate_name = "compiletest"] #![feature(test)] +#![feature(vec_remove_item)] #![deny(warnings, rust_2018_idioms)] #[cfg(unix)] diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 2021dd513aa62..79868a781fdd1 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -74,6 +74,25 @@ pub fn dylib_env_var() -> &'static str { } } +/// The platform-specific library name +pub fn get_lib_name(lib: &str, dylib: bool) -> String { + // In some casess (e.g. MUSL), we build a static + // library, rather than a dynamic library. + // In this case, the only path we can pass + // with '--extern-meta' is the '.lib' file + if !dylib { + return format!("lib{}.rlib", lib); + } + + if cfg!(windows) { + format!("{}.dll", lib) + } else if cfg!(target_os = "macos") { + format!("lib{}.dylib", lib) + } else { + format!("lib{}.so", lib) + } +} + #[derive(Debug, PartialEq)] pub enum DiffLine { Context(String), @@ -1585,6 +1604,16 @@ impl<'test> TestCx<'test> { create_dir_all(&aux_dir).unwrap(); } + // Use a Vec instead of a HashMap to preserve original order + let mut extern_priv = self.props.extern_private.clone(); + + let mut add_extern_priv = |priv_dep: &str, dylib: bool| { + let lib_name = get_lib_name(priv_dep, dylib); + rustc + .arg("--extern-private") + .arg(format!("{}={}", priv_dep, aux_dir.join(lib_name).to_str().unwrap())); + }; + for rel_ab in &self.props.aux_builds { let aux_testpaths = self.compute_aux_test_paths(rel_ab); let aux_props = @@ -1601,8 +1630,8 @@ impl<'test> TestCx<'test> { create_dir_all(aux_cx.output_base_dir()).unwrap(); let mut aux_rustc = aux_cx.make_compile_args(&aux_testpaths.file, aux_output); - let crate_type = if aux_props.no_prefer_dynamic { - None + let (dylib, crate_type) = if aux_props.no_prefer_dynamic { + (true, None) } else if self.config.target.contains("cloudabi") || self.config.target.contains("emscripten") || (self.config.target.contains("musl") && !aux_props.force_host) @@ -1618,11 +1647,20 @@ impl<'test> TestCx<'test> { // dynamic libraries so we just go back to building a normal library. Note, // however, that for MUSL if the library is built with `force_host` then // it's ok to be a dylib as the host should always support dylibs. - Some("lib") + (false, Some("lib")) } else { - Some("dylib") + (true, Some("dylib")) }; + let trimmed = rel_ab.trim_end_matches(".rs").to_string(); + + // Normally, every 'extern-private' has a correspodning 'aux-build' + // entry. If so, we remove it from our list of private crates, + // and add an '--extern-private' flag to rustc + if extern_priv.remove_item(&trimmed).is_some() { + add_extern_priv(&trimmed, dylib); + } + if let Some(crate_type) = crate_type { aux_rustc.args(&["--crate-type", crate_type]); } @@ -1646,6 +1684,12 @@ impl<'test> TestCx<'test> { } } + // Add any '--extern-private' entries without a matching + // 'aux-build' + for private_lib in extern_priv { + add_extern_priv(&private_lib, true); + } + rustc.envs(self.props.rustc_env.clone()); self.compose_and_run( rustc,