Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 13 commits into from
Apr 14, 2019
1 change: 1 addition & 0 deletions src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down
75 changes: 48 additions & 27 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, BTreeSet<Option<String>>>);
pub struct Externs(BTreeMap<String, ExternEntry>);

#[derive(Clone, Hash, Eq, PartialEq, Ord, PartialOrd, Debug, Default)]
pub struct ExternEntry {
pub locations: BTreeSet<Option<String>>,
pub is_private_dep: bool
}

impl Externs {
pub fn new(data: BTreeMap<String, BTreeSet<Option<String>>>) -> Externs {
pub fn new(data: BTreeMap<String, ExternEntry>) -> Externs {
Externs(data)
}

pub fn get(&self, key: &str) -> Option<&BTreeSet<Option<String>>> {
pub fn get(&self, key: &str) -> Option<&ExternEntry> {
self.0.get(key)
}

pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, BTreeSet<Option<String>>> {
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]) => ({
Expand Down Expand Up @@ -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<String> [TRACKED],
}
);

Expand Down Expand Up @@ -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()
}
}
}
Expand Down Expand Up @@ -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<String>, bool)>>,
// and later convert it into a BTreeSet<(Option<String>, bool)>
// This allows to modify entries in-place to set their correct
// 'public' value
let mut externs: BTreeMap<String, ExternEntry> = 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"));
Expand All @@ -2331,10 +2337,17 @@ pub fn build_session_options_and_crate_config(
);
};

externs
let entry = externs
.entry(name.to_owned())
.or_default()
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
.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");
Expand Down Expand Up @@ -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,
)
Expand Down Expand Up @@ -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};
Expand All @@ -2664,6 +2676,19 @@ mod tests {
use syntax;
use super::Options;

impl ExternEntry {
fn new_public<S: Into<String>,
I: IntoIterator<Item = Option<S>>>(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() {
Expand All @@ -2676,10 +2701,6 @@ mod tests {
BTreeMap::from_iter(entries.into_iter())
}

fn mk_set<V: Ord>(entries: Vec<V>) -> BTreeSet<V> {
BTreeSet::from_iter(entries.into_iter())
}

// When the user supplies --test we should implicitly supply --cfg test
#[test]
fn test_switch_implies_cfg_test() {
Expand Down Expand Up @@ -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")])
),
]));

Expand Down
10 changes: 10 additions & 0 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
21 changes: 15 additions & 6 deletions src/librustc_metadata/creader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -195,12 +195,20 @@ 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 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();

Expand Down Expand Up @@ -272,7 +280,8 @@ impl<'a> CrateLoader<'a> {
dylib,
rlib,
rmeta,
}
},
private_dep
};

let cmeta = Lrc::new(cmeta);
Expand Down Expand Up @@ -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!()
}
Expand Down
7 changes: 6 additions & 1 deletion src/librustc_metadata/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
}

pub struct CStore {
Expand Down Expand Up @@ -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>) {
Expand Down
4 changes: 4 additions & 0 deletions src/librustc_metadata/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
petrochenkov marked this conversation as resolved.
Show resolved Hide resolved
}

fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> CrateDisambiguator
{
self.get_crate_data(cnum).root.disambiguator
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_metadata/locator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
);
}
}
Expand Down
11 changes: 1 addition & 10 deletions src/librustc_privacy/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1540,7 +1540,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> {
Expand Down Expand Up @@ -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;
Expand All @@ -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<CrateNum>
}

impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> {
Expand Down Expand Up @@ -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()
}
}

Expand Down Expand Up @@ -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<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));
}
Expand Down
11 changes: 7 additions & 4 deletions src/librustdoc/config.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::{BTreeMap, BTreeSet};
use std::collections::BTreeMap;
use std::fmt;
use std::path::PathBuf;

Expand All @@ -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;
Expand Down Expand Up @@ -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<Externs, String> {
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())?;
Expand All @@ -588,7 +588,10 @@ fn parse_externs(matches: &getopts::Matches) -> Result<Externs, String> {
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))
}
Loading