From 21491dc7012ae50ecdc694b5ee664ecc502ae2c5 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Wed, 20 Mar 2019 23:27:08 -0400 Subject: [PATCH 01/13] Properly parse '--extern-private' with name and path --- src/librustc/middle/cstore.rs | 1 + src/librustc/session/config.rs | 43 ++++++++++++++++--- src/librustc/ty/context.rs | 10 +++++ src/librustc_metadata/creader.rs | 26 +++++++++-- src/librustc_metadata/cstore.rs | 7 ++- src/librustc_metadata/cstore_impl.rs | 5 +++ src/librustc_privacy/lib.rs | 11 +---- src/test/ui/privacy/pub-priv-dep/pub-priv1.rs | 2 +- src/tools/compiletest/src/header.rs | 12 ++++++ src/tools/compiletest/src/runtest.rs | 18 ++++++++ 10 files changed, 113 insertions(+), 22 deletions(-) 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..92f9346ef6e47 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -285,6 +285,7 @@ impl OutputTypes { #[derive(Clone, Hash)] pub struct Externs(BTreeMap>>); + impl Externs { pub fn new(data: BTreeMap>>) -> Externs { Externs(data) @@ -299,6 +300,21 @@ impl Externs { } } +// Similar to 'Externs', but used for the '--extern-private' option +#[derive(Clone, Hash)] +pub struct ExternPrivates(BTreeMap>); + +impl ExternPrivates { + pub fn get(&self, key: &str) -> Option<&BTreeSet> { + self.0.get(key) + } + + pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, BTreeSet> { + 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]) => ({ @@ -428,9 +444,9 @@ top_level_options!( edition: Edition [TRACKED], - // The list of crates to consider private when + // The crates to consider private when // checking leaked private dependency types in public interfaces - extern_private: Vec [TRACKED], + extern_private: ExternPrivates [UNTRACKED], } ); @@ -633,7 +649,7 @@ impl Default for Options { cli_forced_thinlto_off: false, remap_path_prefix: Vec::new(), edition: DEFAULT_EDITION, - extern_private: Vec::new() + extern_private: ExternPrivates(BTreeMap::new()) } } } @@ -2315,10 +2331,25 @@ pub fn build_session_options_and_crate_config( ) } - let extern_private = matches.opt_strs("extern-private"); + let mut extern_private: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); + + for arg in matches.opt_strs("extern-private").into_iter() { + let mut parts = arg.splitn(2, '='); + let name = parts.next().unwrap_or_else(|| + early_error(error_format, "--extern-private value must not be empty")); + let location = parts.next().map(|s| s.to_string()).unwrap_or_else(|| + early_error(error_format, "--extern-private value must include a location")); + + + extern_private + .entry(name.to_owned()) + .or_default() + .insert(location); + + } let mut externs: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); - for arg in matches.opt_strs("extern").into_iter().chain(matches.opt_strs("extern-private")) { + for arg in matches.opt_strs("extern").into_iter() { let mut parts = arg.splitn(2, '='); let name = parts.next().unwrap_or_else(|| early_error(error_format, "--extern value must not be empty")); @@ -2386,7 +2417,7 @@ pub fn build_session_options_and_crate_config( cli_forced_thinlto_off: disable_thinlto, remap_path_prefix, edition, - extern_private + extern_private: ExternPrivates(extern_private) }, cfg, ) 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..53348e75aa932 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -195,12 +195,29 @@ 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 mut private_dep = false; + if let Some(s) = self.sess.opts.extern_private.get(&name.as_str()) { + for path in s { + let p = Some(path.as_str()); + if p == lib.dylib.as_ref().and_then(|r| r.0.to_str()) || + p == lib.rlib.as_ref().and_then(|r| r.0.to_str()) { + + private_dep = true; + } + } + } + + + 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 +289,8 @@ impl<'a> CrateLoader<'a> { dylib, rlib, rmeta, - } + }, + private_dep }; let cmeta = Lrc::new(cmeta); @@ -390,7 +408,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..75671facf9446 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -399,6 +399,7 @@ impl cstore::CStore { r } + pub fn crate_edition_untracked(&self, cnum: CrateNum) -> Edition { self.get_crate_data(cnum).root.edition } @@ -494,6 +495,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_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/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..c548b1efa75cb 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -286,6 +286,9 @@ 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' + pub extern_private: Vec, // Environment settings to use for compiling pub rustc_env: Vec<(String, String)>, // Environment settings to use during execution @@ -353,6 +356,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 +473,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 +618,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/runtest.rs b/src/tools/compiletest/src/runtest.rs index 2021dd513aa62..cec1d83eb0262 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -74,6 +74,17 @@ pub fn dylib_env_var() -> &'static str { } } +/// The platform-specific library file extension +pub fn lib_extension() -> &'static str { + if cfg!(windows) { + ".dll" + } else if cfg!(target_os = "macos") { + ".dylib" + } else { + ".so" + } +} + #[derive(Debug, PartialEq)] pub enum DiffLine { Context(String), @@ -1585,6 +1596,13 @@ impl<'test> TestCx<'test> { create_dir_all(&aux_dir).unwrap(); } + for priv_dep in &self.props.extern_private { + let lib_name = format!("lib{}{}", priv_dep, lib_extension()); + 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 = From 7cc3ce3bbd59f0b5876a5d3c0b02aa8f893a2bdd Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 24 Mar 2019 23:06:32 -0400 Subject: [PATCH 02/13] Combine 'Extern' and 'ExternPrivate' --- src/librustc/session/config.rs | 77 +++++++++++++++++++++----------- src/librustc_metadata/creader.rs | 11 ++--- src/librustc_metadata/locator.rs | 4 +- src/librustdoc/config.rs | 5 ++- 4 files changed, 61 insertions(+), 36 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 92f9346ef6e47..2a4ff69cbd7ca 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -283,33 +283,24 @@ 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)] +pub struct ExternEntry { + pub location: Option, + pub public: 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>> { - self.0.get(key) - } - - pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, BTreeSet>> { - self.0.iter() - } -} - -// Similar to 'Externs', but used for the '--extern-private' option -#[derive(Clone, Hash)] -pub struct ExternPrivates(BTreeMap>); - -impl ExternPrivates { - pub fn get(&self, key: &str) -> Option<&BTreeSet> { + pub fn get(&self, key: &str) -> Option<&BTreeSet> { self.0.get(key) } - pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, BTreeSet> { + pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, BTreeSet> { self.0.iter() } } @@ -446,7 +437,7 @@ top_level_options!( // The crates to consider private when // checking leaked private dependency types in public interfaces - extern_private: ExternPrivates [UNTRACKED], + //extern_private: ExternPrivates [UNTRACKED], } ); @@ -649,7 +640,7 @@ impl Default for Options { cli_forced_thinlto_off: false, remap_path_prefix: Vec::new(), edition: DEFAULT_EDITION, - extern_private: ExternPrivates(BTreeMap::new()) + //extern_private: ExternPrivates(BTreeMap::new()) } } } @@ -2331,7 +2322,7 @@ pub fn build_session_options_and_crate_config( ) } - let mut extern_private: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); + /*let mut extern_private: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); for arg in matches.opt_strs("extern-private").into_iter() { let mut parts = arg.splitn(2, '='); @@ -2346,10 +2337,16 @@ pub fn build_session_options_and_crate_config( .or_default() .insert(location); - } + }*/ + + // 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, bool>> = BTreeMap::new(); + for (arg, public) in matches.opt_strs("extern").into_iter().map(|v| (v, true)) + .chain(matches.opt_strs("extern-private").into_iter().map(|v| (v, false))) { - let mut externs: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); - for arg in matches.opt_strs("extern").into_iter() { let mut parts = arg.splitn(2, '='); let name = parts.next().unwrap_or_else(|| early_error(error_format, "--extern value must not be empty")); @@ -2362,11 +2359,37 @@ pub fn build_session_options_and_crate_config( ); }; + + // Externsl crates start out public, + // and become private if we later see + // an '--extern-private' key. They never + // go back to being public once we've seen + // '--extern-private', so we logical-AND + // their current and new 'public' value together + externs .entry(name.to_owned()) .or_default() - .insert(location); - } + .entry(location) + .and_modify(|e| *e &= public) + .or_insert(public); + } + + // Now that we've determined the 'public' status of each extern, + // collect them into a set of ExternEntry + let externs: BTreeMap> = externs.into_iter() + .map(|(k, v)| { + let values =v.into_iter().map(|(location, public)| { + ExternEntry { + location, + public + } + }).collect::>(); + (k, values) + }) + .collect(); + + let crate_name = matches.opt_str("crate-name"); @@ -2417,7 +2440,7 @@ pub fn build_session_options_and_crate_config( cli_forced_thinlto_off: disable_thinlto, remap_path_prefix, edition, - extern_private: ExternPrivates(extern_private) + //extern_private: ExternPrivates(extern_private) }, cfg, ) diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 53348e75aa932..7c5b5dc7113ae 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -133,7 +133,7 @@ impl<'a> CrateLoader<'a> { let source = &self.cstore.get_crate_data(cnum).source; if let Some(locs) = 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 = locs.iter().filter_map(|l| l.location.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() @@ -202,13 +202,14 @@ impl<'a> CrateLoader<'a> { self.verify_no_symbol_conflicts(span, &crate_root); let mut private_dep = false; - if let Some(s) = self.sess.opts.extern_private.get(&name.as_str()) { - for path in s { - let p = Some(path.as_str()); + if let Some(s) = self.sess.opts.externs.get(&name.as_str()) { + for entry in s { + let p = entry.location.as_ref().map(|s| s.as_str()); if p == lib.dylib.as_ref().and_then(|r| r.0.to_str()) || p == lib.rlib.as_ref().and_then(|r| r.0.to_str()) { - private_dep = true; + private_dep = !entry.public; + break; } } } diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs index 81878c4f687b6..f56ca5af76e8f 100644 --- a/src/librustc_metadata/locator.rs +++ b/src/librustc_metadata/locator.rs @@ -444,9 +444,9 @@ impl<'a> Context<'a> { self.should_match_name = false; if let Some(s) = 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 s.iter().any(|l| l.location.is_some()) { return self.find_commandline_library( - s.iter().filter_map(|l| l.as_ref()), + s.iter().filter_map(|l| l.location.as_ref()), ); } } diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index f2682e00430d0..80c0911fee1ea 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -7,7 +7,7 @@ use errors::emitter::ColorConfig; use getopts; use rustc::lint::Level; use rustc::session::early_error; -use rustc::session::config::{CodegenOptions, DebuggingOptions, ErrorOutputType, Externs}; +use rustc::session::config::{CodegenOptions, DebuggingOptions, ErrorOutputType, Externs, ExternEntry}; use rustc::session::config::{nightly_options, build_codegen_options, build_debugging_options, get_cmd_lint_options}; use rustc::session::search_paths::SearchPath; @@ -588,7 +588,8 @@ 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().insert(ExternEntry { location, public: true }); } Ok(Externs::new(externs)) } From a6ae8abdd656c34430504a51044cb70434214bc5 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 25 Mar 2019 00:11:25 -0400 Subject: [PATCH 03/13] Fix tidy --- src/librustc/session/config.rs | 2 +- src/librustdoc/config.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 2a4ff69cbd7ca..5e6f1e8fc4558 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -2388,7 +2388,7 @@ pub fn build_session_options_and_crate_config( (k, values) }) .collect(); - + let crate_name = matches.opt_str("crate-name"); diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 80c0911fee1ea..fae07be101780 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -7,9 +7,9 @@ use errors::emitter::ColorConfig; use getopts; use rustc::lint::Level; use rustc::session::early_error; -use rustc::session::config::{CodegenOptions, DebuggingOptions, ErrorOutputType, Externs, ExternEntry}; +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; From 87734b1f2585ccfa4ceb425389a729e432419981 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 1 Apr 2019 15:13:08 -0400 Subject: [PATCH 04/13] Fix tests --- src/librustc/session/config.rs | 35 +++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 5e6f1e8fc4558..e07ef35c5d519 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -291,6 +291,15 @@ pub struct ExternEntry { pub public: bool } +impl ExternEntry { + pub fn new_public(location: Option) -> ExternEntry { + ExternEntry { + location, + public: true + } + } +} + impl Externs { pub fn new(data: BTreeMap>) -> Externs { Externs(data) @@ -2705,7 +2714,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}; @@ -2851,33 +2860,45 @@ mod tests { v1.externs = Externs::new(mk_map(vec![ ( String::from("a"), - mk_set(vec![Some(String::from("b")), Some(String::from("c"))]), + mk_set(vec![ExternEntry::new_public(Some(String::from("b"))), + ExternEntry::new_public(Some(String::from("c"))) + ]), ), ( String::from("d"), - mk_set(vec![Some(String::from("e")), Some(String::from("f"))]), + mk_set(vec![ExternEntry::new_public(Some(String::from("e"))), + ExternEntry::new_public(Some(String::from("f"))) + ]), ), ])); v2.externs = Externs::new(mk_map(vec![ ( String::from("d"), - mk_set(vec![Some(String::from("e")), Some(String::from("f"))]), + mk_set(vec![ExternEntry::new_public(Some(String::from("e"))), + ExternEntry::new_public(Some(String::from("f"))) + ]), ), ( String::from("a"), - mk_set(vec![Some(String::from("b")), Some(String::from("c"))]), + mk_set(vec![ExternEntry::new_public(Some(String::from("b"))), + ExternEntry::new_public(Some(String::from("c"))) + ]), ), ])); v3.externs = Externs::new(mk_map(vec![ ( String::from("a"), - mk_set(vec![Some(String::from("b")), Some(String::from("c"))]), + mk_set(vec![ExternEntry::new_public(Some(String::from("b"))), + ExternEntry::new_public(Some(String::from("c"))) + ]), ), ( String::from("d"), - mk_set(vec![Some(String::from("f")), Some(String::from("e"))]), + mk_set(vec![ExternEntry::new_public(Some(String::from("f"))), + ExternEntry::new_public(Some(String::from("e"))) + ]), ), ])); From 0f88106940b0d09425a498ce5eef3bcad975a4a2 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 2 Apr 2019 16:06:43 -0400 Subject: [PATCH 05/13] Improve formatting --- src/librustc/session/config.rs | 25 +------------------------ src/librustc_metadata/cstore_impl.rs | 1 - 2 files changed, 1 insertion(+), 25 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index e07ef35c5d519..bf0e4a7777775 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -443,10 +443,6 @@ top_level_options!( remap_path_prefix: Vec<(PathBuf, PathBuf)> [UNTRACKED], edition: Edition [TRACKED], - - // The crates to consider private when - // checking leaked private dependency types in public interfaces - //extern_private: ExternPrivates [UNTRACKED], } ); @@ -649,7 +645,6 @@ impl Default for Options { cli_forced_thinlto_off: false, remap_path_prefix: Vec::new(), edition: DEFAULT_EDITION, - //extern_private: ExternPrivates(BTreeMap::new()) } } } @@ -2331,23 +2326,6 @@ pub fn build_session_options_and_crate_config( ) } - /*let mut extern_private: BTreeMap<_, BTreeSet<_>> = BTreeMap::new(); - - for arg in matches.opt_strs("extern-private").into_iter() { - let mut parts = arg.splitn(2, '='); - let name = parts.next().unwrap_or_else(|| - early_error(error_format, "--extern-private value must not be empty")); - let location = parts.next().map(|s| s.to_string()).unwrap_or_else(|| - early_error(error_format, "--extern-private value must include a location")); - - - extern_private - .entry(name.to_owned()) - .or_default() - .insert(location); - - }*/ - // 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 @@ -2369,7 +2347,7 @@ pub fn build_session_options_and_crate_config( }; - // Externsl crates start out public, + // Extern crates start out public, // and become private if we later see // an '--extern-private' key. They never // go back to being public once we've seen @@ -2449,7 +2427,6 @@ pub fn build_session_options_and_crate_config( cli_forced_thinlto_off: disable_thinlto, remap_path_prefix, edition, - //extern_private: ExternPrivates(extern_private) }, cfg, ) diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 75671facf9446..95fe9a70f750a 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -399,7 +399,6 @@ impl cstore::CStore { r } - pub fn crate_edition_untracked(&self, cnum: CrateNum) -> Edition { self.get_crate_data(cnum).root.edition } From b23a8830f97f70aad44c63c813fd1d9083900537 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 6 Apr 2019 20:39:12 -0400 Subject: [PATCH 06/13] Move new_public to 'tess' module --- src/librustc/session/config.rs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index bf0e4a7777775..474848fbaf345 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -291,14 +291,7 @@ pub struct ExternEntry { pub public: bool } -impl ExternEntry { - pub fn new_public(location: Option) -> ExternEntry { - ExternEntry { - location, - public: true - } - } -} + impl Externs { pub fn new(data: BTreeMap>) -> Externs { @@ -2704,6 +2697,15 @@ mod tests { use syntax; use super::Options; + impl ExternEntry { + fn new_public(location: Option) -> ExternEntry { + ExternEntry { + location, + public: true + } + } + } + fn optgroups() -> getopts::Options { let mut opts = getopts::Options::new(); for group in super::rustc_optgroups() { From 482b77a1470681da08748f1461c598eb8e48615b Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 7 Apr 2019 18:48:40 -0400 Subject: [PATCH 07/13] Refactor structure of ExternEntry --- src/librustc/session/config.rs | 66 ++++++++++++----------------- src/librustc_metadata/creader.rs | 20 +++------ src/librustc_metadata/locator.rs | 6 +-- src/librustc_typeck/lib.rs | 6 +-- src/tools/compiletest/src/header.rs | 1 + 5 files changed, 40 insertions(+), 59 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 474848fbaf345..9bc9c7cbbe3f0 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -283,26 +283,24 @@ 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)] pub struct ExternEntry { - pub location: Option, - pub public: bool + 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() } } @@ -2323,9 +2321,9 @@ pub fn build_session_options_and_crate_config( // 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, bool>> = BTreeMap::new(); - for (arg, public) in matches.opt_strs("extern").into_iter().map(|v| (v, true)) - .chain(matches.opt_strs("extern-private").into_iter().map(|v| (v, false))) { + 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 parts = arg.splitn(2, '='); let name = parts.next().unwrap_or_else(|| @@ -2340,36 +2338,26 @@ pub fn build_session_options_and_crate_config( }; - // Extern crates start out public, - // and become private if we later see - // an '--extern-private' key. They never - // go back to being public once we've seen - // '--extern-private', so we logical-AND - // their current and new 'public' value together - externs .entry(name.to_owned()) - .or_default() - .entry(location) - .and_modify(|e| *e &= public) - .or_insert(public); - } + .and_modify(|e| { + e.locations.insert(location.clone()); + + // Crates start out being not private, + // and go to being private if we see an '--extern-private' + // flag + e.is_private_dep |= private; + }) + .or_insert_with(|| { + let mut locations = BTreeSet::new(); + locations.insert(location); - // Now that we've determined the 'public' status of each extern, - // collect them into a set of ExternEntry - let externs: BTreeMap> = externs.into_iter() - .map(|(k, v)| { - let values =v.into_iter().map(|(location, public)| { ExternEntry { - location, - public + locations: locations, + is_private_dep: private } - }).collect::>(); - (k, values) - }) - .collect(); - - + }); + } let crate_name = matches.opt_str("crate-name"); @@ -2699,9 +2687,11 @@ mod tests { impl ExternEntry { fn new_public(location: Option) -> ExternEntry { + let mut locations = BTreeSet::new(); + locations.insert(location); ExternEntry { - location, - public: true + locations, + is_private_dep: false } } } diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 7c5b5dc7113ae..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.location.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() @@ -201,19 +201,9 @@ impl<'a> CrateLoader<'a> { let crate_root = lib.metadata.get_root(); self.verify_no_symbol_conflicts(span, &crate_root); - let mut private_dep = false; - if let Some(s) = self.sess.opts.externs.get(&name.as_str()) { - for entry in s { - let p = entry.location.as_ref().map(|s| s.as_str()); - if p == lib.dylib.as_ref().and_then(|r| r.0.to_str()) || - p == lib.rlib.as_ref().and_then(|r| r.0.to_str()) { - - private_dep = !entry.public; - break; - } - } - } - + 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); diff --git a/src/librustc_metadata/locator.rs b/src/librustc_metadata/locator.rs index f56ca5af76e8f..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.location.is_some()) { + if entry.locations.iter().any(|l| l.is_some()) { return self.find_commandline_library( - s.iter().filter_map(|l| l.location.as_ref()), + entry.locations.iter().filter_map(|l| l.as_ref()), ); } } diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 21d1af229ddc2..401d7457517e6 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -176,7 +176,7 @@ fn require_same_types<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }) } -fn check_main_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, main_def_id: DefId) { +pub fn check_main_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, main_def_id: DefId) { let main_id = tcx.hir().as_local_hir_id(main_def_id).unwrap(); let main_span = tcx.def_span(main_def_id); let main_t = tcx.type_of(main_def_id); @@ -241,7 +241,7 @@ fn check_main_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, main_def_id: DefId) { } } -fn check_start_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, start_def_id: DefId) { +pub fn check_start_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, start_def_id: DefId) { let start_id = tcx.hir().as_local_hir_id(start_def_id).unwrap(); let start_span = tcx.def_span(start_def_id); let start_t = tcx.type_of(start_def_id); @@ -298,7 +298,7 @@ fn check_start_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, start_def_id: DefId) } } -fn check_for_entry_fn<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { +pub fn check_for_entry_fn<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { match tcx.entry_fn(LOCAL_CRATE) { Some((def_id, EntryFnType::Main)) => check_main_fn_ty(tcx, def_id), Some((def_id, EntryFnType::Start)) => check_start_fn_ty(tcx, def_id), diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index c548b1efa75cb..64882c603bad3 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -288,6 +288,7 @@ pub struct TestProps { 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)>, From 724f6a11b3b9ea7c267c8f009f2e6958e6286e5f Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sun, 7 Apr 2019 19:15:32 -0400 Subject: [PATCH 08/13] Update rustdoc to new ExternEntry format --- src/librustdoc/config.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index fae07be101780..8a669cad514b5 100644 --- a/src/librustdoc/config.rs +++ b/src/librustdoc/config.rs @@ -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())?; @@ -589,7 +589,13 @@ fn parse_externs(matches: &getopts::Matches) -> Result { } let name = name.to_string(); // For Rustdoc purposes, we can treat all externs as public - externs.entry(name).or_default().insert(ExternEntry { location, public: true }); + externs.entry(name) + .and_modify(|e| { e.locations.insert(location.clone()); } ) + .or_insert_with(|| { + let mut locations = BTreeSet::new(); + locations.insert(location); + ExternEntry { locations, is_private_dep: false } + }); } Ok(Externs::new(externs)) } From 872c20d41569ab42eda5741d6e2a463a2b385c56 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Mon, 8 Apr 2019 23:22:22 -0400 Subject: [PATCH 09/13] Fix ExternEntry test --- src/librustc/session/config.rs | 36 +++++++++++----------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 9bc9c7cbbe3f0..00dfe9aca3940 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -2686,9 +2686,11 @@ mod tests { use super::Options; impl ExternEntry { - fn new_public(location: Option) -> ExternEntry { - let mut locations = BTreeSet::new(); - locations.insert(location); + 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 @@ -2708,10 +2710,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() { @@ -2829,45 +2827,33 @@ mod tests { v1.externs = Externs::new(mk_map(vec![ ( String::from("a"), - mk_set(vec![ExternEntry::new_public(Some(String::from("b"))), - ExternEntry::new_public(Some(String::from("c"))) - ]), + ExternEntry::new_public(vec![Some("b"), Some("c")]) ), ( String::from("d"), - mk_set(vec![ExternEntry::new_public(Some(String::from("e"))), - ExternEntry::new_public(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![ExternEntry::new_public(Some(String::from("e"))), - ExternEntry::new_public(Some(String::from("f"))) - ]), + ExternEntry::new_public(vec![Some("e"), Some("f")]) ), ( String::from("a"), - mk_set(vec![ExternEntry::new_public(Some(String::from("b"))), - ExternEntry::new_public(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![ExternEntry::new_public(Some(String::from("b"))), - ExternEntry::new_public(Some(String::from("c"))) - ]), + ExternEntry::new_public(vec![Some("b"), Some("c")]) ), ( String::from("d"), - mk_set(vec![ExternEntry::new_public(Some(String::from("f"))), - ExternEntry::new_public(Some(String::from("e"))) - ]), + ExternEntry::new_public(vec![Some("f"), Some("e")]) ), ])); From 4dfce34912f866d6fef52907f1e5c56117becf27 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Tue, 9 Apr 2019 17:24:24 -0400 Subject: [PATCH 10/13] Derive Default for ExternEntry --- src/librustc/session/config.rs | 31 +++++++++++-------------------- src/librustc_typeck/lib.rs | 6 +++--- src/librustdoc/config.rs | 10 +++------- 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index 00dfe9aca3940..d3a734cbc6ead 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -285,7 +285,7 @@ impl OutputTypes { #[derive(Clone, Hash)] pub struct Externs(BTreeMap); -#[derive(Clone, Hash, Eq, PartialEq, Ord, PartialOrd, Debug)] +#[derive(Clone, Hash, Eq, PartialEq, Ord, PartialOrd, Debug, Default)] pub struct ExternEntry { pub locations: BTreeSet>, pub is_private_dep: bool @@ -2337,26 +2337,17 @@ pub fn build_session_options_and_crate_config( ); }; - - externs + let entry = externs .entry(name.to_owned()) - .and_modify(|e| { - e.locations.insert(location.clone()); - - // Crates start out being not private, - // and go to being private if we see an '--extern-private' - // flag - e.is_private_dep |= private; - }) - .or_insert_with(|| { - let mut locations = BTreeSet::new(); - locations.insert(location); - - ExternEntry { - locations: locations, - is_private_dep: private - } - }); + .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"); diff --git a/src/librustc_typeck/lib.rs b/src/librustc_typeck/lib.rs index 401d7457517e6..21d1af229ddc2 100644 --- a/src/librustc_typeck/lib.rs +++ b/src/librustc_typeck/lib.rs @@ -176,7 +176,7 @@ fn require_same_types<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }) } -pub fn check_main_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, main_def_id: DefId) { +fn check_main_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, main_def_id: DefId) { let main_id = tcx.hir().as_local_hir_id(main_def_id).unwrap(); let main_span = tcx.def_span(main_def_id); let main_t = tcx.type_of(main_def_id); @@ -241,7 +241,7 @@ pub fn check_main_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, main_def_id: DefI } } -pub fn check_start_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, start_def_id: DefId) { +fn check_start_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, start_def_id: DefId) { let start_id = tcx.hir().as_local_hir_id(start_def_id).unwrap(); let start_span = tcx.def_span(start_def_id); let start_t = tcx.type_of(start_def_id); @@ -298,7 +298,7 @@ pub fn check_start_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, start_def_id: De } } -pub fn check_for_entry_fn<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { +fn check_for_entry_fn<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) { match tcx.entry_fn(LOCAL_CRATE) { Some((def_id, EntryFnType::Main)) => check_main_fn_ty(tcx, def_id), Some((def_id, EntryFnType::Start)) => check_start_fn_ty(tcx, def_id), diff --git a/src/librustdoc/config.rs b/src/librustdoc/config.rs index 8a669cad514b5..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; @@ -590,12 +590,8 @@ fn parse_externs(matches: &getopts::Matches) -> Result { let name = name.to_string(); // For Rustdoc purposes, we can treat all externs as public externs.entry(name) - .and_modify(|e| { e.locations.insert(location.clone()); } ) - .or_insert_with(|| { - let mut locations = BTreeSet::new(); - locations.insert(location); - ExternEntry { locations, is_private_dep: false } - }); + .or_default() + .locations.insert(location.clone()); } Ok(Externs::new(externs)) } From 3c7c2c6ca7de0317b5852c0e77aa39a2493f2dee Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Fri, 12 Apr 2019 20:14:51 -0400 Subject: [PATCH 11/13] Handle --extern-private properly on musl On musl (and some other platforms), compiletest ends up creating a static rlib (instead of a dylib) when building 'aux-build' crates. This commit changes the '--extern-private' path computed by compiletest to properly take this into account --- src/tools/compiletest/src/main.rs | 1 + src/tools/compiletest/src/runtest.rs | 42 ++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 8 deletions(-) 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 cec1d83eb0262..e6e937f5db445 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -75,7 +75,15 @@ pub fn dylib_env_var() -> &'static str { } /// The platform-specific library file extension -pub fn lib_extension() -> &'static str { +pub fn lib_extension(dylib: bool) -> &'static str { + // 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 ".rlib" + } + if cfg!(windows) { ".dll" } else if cfg!(target_os = "macos") { @@ -1596,12 +1604,15 @@ impl<'test> TestCx<'test> { create_dir_all(&aux_dir).unwrap(); } - for priv_dep in &self.props.extern_private { - let lib_name = format!("lib{}{}", priv_dep, lib_extension()); + // 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 = format!("lib{}{}", priv_dep, lib_extension(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); @@ -1619,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) @@ -1636,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]); } @@ -1664,6 +1684,12 @@ impl<'test> TestCx<'test> { } } + // Add any '--extenr-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, From 7ba8a9e685470b6aef3a230ab6eeb25c3152e7f1 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 13 Apr 2019 18:49:01 -0400 Subject: [PATCH 12/13] Fix Windows dll name format --- src/tools/compiletest/src/runtest.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index e6e937f5db445..11e9f1c39785d 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -74,22 +74,22 @@ pub fn dylib_env_var() -> &'static str { } } -/// The platform-specific library file extension -pub fn lib_extension(dylib: bool) -> &'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 ".rlib" + return format!("lib{}.rlib", lib); } if cfg!(windows) { - ".dll" + format!("{}.dll", lib) } else if cfg!(target_os = "macos") { - ".dylib" + format!("lib{}.dylib", lib) } else { - ".so" + format!("lib{}.so", lib) } } @@ -1608,7 +1608,7 @@ impl<'test> TestCx<'test> { let mut extern_priv = self.props.extern_private.clone(); let mut add_extern_priv = |priv_dep: &str, dylib: bool| { - let lib_name = format!("lib{}{}", priv_dep, lib_extension(dylib)); + 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())); From 5cd51b1d8416ce5670a2411a22886bd683bb2766 Mon Sep 17 00:00:00 2001 From: Aaron Hill Date: Sat, 13 Apr 2019 22:25:27 -0400 Subject: [PATCH 13/13] Fix typo in comment --- src/tools/compiletest/src/runtest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 11e9f1c39785d..79868a781fdd1 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -1684,7 +1684,7 @@ impl<'test> TestCx<'test> { } } - // Add any '--extenr-private' entries without a matching + // Add any '--extern-private' entries without a matching // 'aux-build' for private_lib in extern_priv { add_extern_priv(&private_lib, true);