Properly parse '--extern-private' with name and path

Author: Aaron1011

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<CrateNum>;

src/librustc/session/config.rs

@@ -270,6 +270,7 @@ impl OutputTypes {
 #[derive(Clone, Hash)]
 pub struct Externs(BTreeMap<String, BTreeSet<Option<String>>>);
 
+
 impl Externs {
     pub fn new(data: BTreeMap<String, BTreeSet<Option<String>>>) -> Externs {
         Externs(data)
@@ -284,6 +285,21 @@ impl Externs {
     }
 }
 
+// Similar to 'Externs', but used for the '--extern-private' option
+#[derive(Clone, Hash)]
+pub struct ExternPrivates(BTreeMap<String, BTreeSet<String>>);
+
+impl ExternPrivates {
+    pub fn get(&self, key: &str) -> Option<&BTreeSet<String>> {
+        self.0.get(key)
+    }
+
+    pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, BTreeSet<String>> {
+        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]) => ({
@@ -413,9 +429,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<String> [TRACKED],
+        extern_private: ExternPrivates [UNTRACKED],
     }
 );
 
@@ -618,7 +634,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())
         }
     }
 }
@@ -2288,10 +2304,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"));
@@ -2359,7 +2390,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,
     )

src/librustc/ty/context.rs

@@ -1381,6 +1381,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() {

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<cstore::CrateMetadata>) {
         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!()
         }

src/librustc_metadata/cstore.rs

@@ -79,6 +79,10 @@ pub struct CrateMetadata {
     pub source: CrateSource,
 
     pub proc_macros: Option<Vec<(ast::Name, Lrc<SyntaxExtension>)>>,
+
+    /// 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<CrateMetadata> {
-        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<CrateMetadata>) {

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

src/librustc_privacy/lib.rs

@@ -1539,7 +1539,6 @@ struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> {
     has_pub_restricted: bool,
     has_old_errors: bool,
     in_assoc_ty: bool,
-    private_crates: FxHashSet<CrateNum>
 }
 
 impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> {
@@ -1621,7 +1620,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;
@@ -1639,7 +1638,6 @@ struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> {
     tcx: TyCtxt<'a, 'tcx, 'tcx>,
     has_pub_restricted: bool,
     old_error_set: &'a HirIdSet,
-    private_crates: FxHashSet<CrateNum>
 }
 
 impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> {
@@ -1677,7 +1675,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()
         }
     }
 
@@ -1875,17 +1872,11 @@ fn check_private_in_public<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, krate: CrateNum) {
         pub_restricted_visitor.has_pub_restricted
     };
 
-    let private_crates: FxHashSet<CrateNum> = 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));
 }

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

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<String>,
+    // A list of crates to pass '--extern-private name:PATH' flags for
+    // This should be a subset of 'aux_build'
+    pub extern_private: Vec<String>,
     // Environment settings to use for compiling
     pub rustc_env: Vec<(String, String)>,
     // Environment settings to use during execution
@@ -349,6 +352,7 @@ impl TestProps {
             run_flags: None,
             pp_exact: None,
             aux_builds: vec![],
+            extern_private: vec![],
             revisions: vec![],
             rustc_env: vec![],
             exec_env: vec![],
@@ -465,6 +469,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);
             }
@@ -606,6 +614,10 @@ impl Config {
             .map(|r| r.trim().to_string())
     }
 
+    fn parse_extern_private(&self, line: &str) -> Option<String> {
+        self.parse_name_value_directive(line, "extern-private")
+    }
+
     fn parse_compile_flags(&self, line: &str) -> Option<String> {
         self.parse_name_value_directive(line, "compile-flags")
     }

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 =