Skip to content

Commit 2f9f15b

Browse files
cramertjcramertj
authored
Rollup merge of rust-lang#59335 - Aaron1011:fix/extern-priv-final, r=petrochenkov
Properly parse '--extern-private' with name and path It turns out that rust-lang#57586 didn't properly parse `--extern-private name=path`. This PR properly implements the `--extern-private` option. I've added a new `extern-private` option to `compiletest`, which causes an `--extern-private` option to be passed to the compiler with the proper path. Part of rust-lang#44663
2 parents fa3b1c3 + eb15d2f commit 2f9f15b

File tree

12 files changed

+127
-52
lines changed

12 files changed

+127
-52
lines changed

‎src/librustc/middle/cstore.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,7 @@ pub trait CrateStore {
199199

200200
// "queries" used in resolve that aren't tracked for incremental compilation
201201
fn crate_name_untracked(&self, cnum: CrateNum) -> Symbol;
202+
fn crate_is_private_dep_untracked(&self, cnum: CrateNum) -> bool;
202203
fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> CrateDisambiguator;
203204
fn crate_hash_untracked(&self, cnum: CrateNum) -> Svh;
204205
fn extern_mod_stmt_cnum_untracked(&self, emod_id: ast::NodeId) -> Option<CrateNum>;

‎src/librustc/session/config.rs

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -268,22 +268,29 @@ impl OutputTypes {
268268
// DO NOT switch BTreeMap or BTreeSet out for an unsorted container type! That
269269
// would break dependency tracking for command-line arguments.
270270
#[derive(Clone, Hash)]
271-
pub struct Externs(BTreeMap<String, BTreeSet<Option<String>>>);
271+
pub struct Externs(BTreeMap<String, ExternEntry>);
272+
273+
#[derive(Clone, Hash, Eq, PartialEq, Ord, PartialOrd, Debug, Default)]
274+
pub struct ExternEntry {
275+
pub locations: BTreeSet<Option<String>>,
276+
pub is_private_dep: bool
277+
}
272278

273279
impl Externs {
274-
pub fn new(data: BTreeMap<String, BTreeSet<Option<String>>>) -> Externs {
280+
pub fn new(data: BTreeMap<String, ExternEntry>) -> Externs {
275281
Externs(data)
276282
}
277283

278-
pub fn get(&self, key: &str) -> Option<&BTreeSet<Option<String>>> {
284+
pub fn get(&self, key: &str) -> Option<&ExternEntry> {
279285
self.0.get(key)
280286
}
281287

282-
pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, BTreeSet<Option<String>>> {
288+
pub fn iter<'a>(&'a self) -> BTreeMapIter<'a, String, ExternEntry> {
283289
self.0.iter()
284290
}
285291
}
286292

293+
287294
macro_rules! hash_option {
288295
($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, [UNTRACKED]) => ({});
289296
($opt_name:ident, $opt_expr:expr, $sub_hashes:expr, [TRACKED]) => ({
@@ -412,10 +419,6 @@ top_level_options!(
412419
remap_path_prefix: Vec<(PathBuf, PathBuf)> [UNTRACKED],
413420

414421
edition: Edition [TRACKED],
415-
416-
// The list of crates to consider private when
417-
// checking leaked private dependency types in public interfaces
418-
extern_private: Vec<String> [TRACKED],
419422
}
420423
);
421424

@@ -618,7 +621,6 @@ impl Default for Options {
618621
cli_forced_thinlto_off: false,
619622
remap_path_prefix: Vec::new(),
620623
edition: DEFAULT_EDITION,
621-
extern_private: Vec::new()
622624
}
623625
}
624626
}
@@ -2290,10 +2292,14 @@ pub fn build_session_options_and_crate_config(
22902292
)
22912293
}
22922294

2293-
let extern_private = matches.opt_strs("extern-private");
2295+
// We start out with a Vec<(Option<String>, bool)>>,
2296+
// and later convert it into a BTreeSet<(Option<String>, bool)>
2297+
// This allows to modify entries in-place to set their correct
2298+
// 'public' value
2299+
let mut externs: BTreeMap<String, ExternEntry> = BTreeMap::new();
2300+
for (arg, private) in matches.opt_strs("extern").into_iter().map(|v| (v, false))
2301+
.chain(matches.opt_strs("extern-private").into_iter().map(|v| (v, true))) {
22942302

2295-
let mut externs: BTreeMap<_, BTreeSet<_>> = BTreeMap::new();
2296-
for arg in matches.opt_strs("extern").into_iter().chain(matches.opt_strs("extern-private")) {
22972303
let mut parts = arg.splitn(2, '=');
22982304
let name = parts.next().unwrap_or_else(||
22992305
early_error(error_format, "--extern value must not be empty"));
@@ -2306,10 +2312,17 @@ pub fn build_session_options_and_crate_config(
23062312
);
23072313
};
23082314

2309-
externs
2315+
let entry = externs
23102316
.entry(name.to_owned())
2311-
.or_default()
2312-
.insert(location);
2317+
.or_default();
2318+
2319+
2320+
entry.locations.insert(location.clone());
2321+
2322+
// Crates start out being not private,
2323+
// and go to being private if we see an '--extern-private'
2324+
// flag
2325+
entry.is_private_dep |= private;
23132326
}
23142327

23152328
let crate_name = matches.opt_str("crate-name");
@@ -2361,7 +2374,6 @@ pub fn build_session_options_and_crate_config(
23612374
cli_forced_thinlto_off: disable_thinlto,
23622375
remap_path_prefix,
23632376
edition,
2364-
extern_private
23652377
},
23662378
cfg,
23672379
)
@@ -2625,7 +2637,7 @@ mod tests {
26252637
build_session_options_and_crate_config,
26262638
to_crate_config
26272639
};
2628-
use crate::session::config::{LtoCli, LinkerPluginLto};
2640+
use crate::session::config::{LtoCli, LinkerPluginLto, ExternEntry};
26292641
use crate::session::build_session;
26302642
use crate::session::search_paths::SearchPath;
26312643
use std::collections::{BTreeMap, BTreeSet};
@@ -2638,6 +2650,19 @@ mod tests {
26382650
use syntax;
26392651
use super::Options;
26402652

2653+
impl ExternEntry {
2654+
fn new_public<S: Into<String>,
2655+
I: IntoIterator<Item = Option<S>>>(locations: I) -> ExternEntry {
2656+
let locations: BTreeSet<_> = locations.into_iter().map(|o| o.map(|s| s.into()))
2657+
.collect();
2658+
2659+
ExternEntry {
2660+
locations,
2661+
is_private_dep: false
2662+
}
2663+
}
2664+
}
2665+
26412666
fn optgroups() -> getopts::Options {
26422667
let mut opts = getopts::Options::new();
26432668
for group in super::rustc_optgroups() {
@@ -2650,10 +2675,6 @@ mod tests {
26502675
BTreeMap::from_iter(entries.into_iter())
26512676
}
26522677

2653-
fn mk_set<V: Ord>(entries: Vec<V>) -> BTreeSet<V> {
2654-
BTreeSet::from_iter(entries.into_iter())
2655-
}
2656-
26572678
// When the user supplies --test we should implicitly supply --cfg test
26582679
#[test]
26592680
fn test_switch_implies_cfg_test() {
@@ -2771,33 +2792,33 @@ mod tests {
27712792
v1.externs = Externs::new(mk_map(vec![
27722793
(
27732794
String::from("a"),
2774-
mk_set(vec![Some(String::from("b")), Some(String::from("c"))]),
2795+
ExternEntry::new_public(vec![Some("b"), Some("c")])
27752796
),
27762797
(
27772798
String::from("d"),
2778-
mk_set(vec![Some(String::from("e")), Some(String::from("f"))]),
2799+
ExternEntry::new_public(vec![Some("e"), Some("f")])
27792800
),
27802801
]));
27812802

27822803
v2.externs = Externs::new(mk_map(vec![
27832804
(
27842805
String::from("d"),
2785-
mk_set(vec![Some(String::from("e")), Some(String::from("f"))]),
2806+
ExternEntry::new_public(vec![Some("e"), Some("f")])
27862807
),
27872808
(
27882809
String::from("a"),
2789-
mk_set(vec![Some(String::from("b")), Some(String::from("c"))]),
2810+
ExternEntry::new_public(vec![Some("b"), Some("c")])
27902811
),
27912812
]));
27922813

27932814
v3.externs = Externs::new(mk_map(vec![
27942815
(
27952816
String::from("a"),
2796-
mk_set(vec![Some(String::from("b")), Some(String::from("c"))]),
2817+
ExternEntry::new_public(vec![Some("b"), Some("c")])
27972818
),
27982819
(
27992820
String::from("d"),
2800-
mk_set(vec![Some(String::from("f")), Some(String::from("e"))]),
2821+
ExternEntry::new_public(vec![Some("f"), Some("e")])
28012822
),
28022823
]));
28032824

‎src/librustc/ty/context.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1388,6 +1388,16 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
13881388
}
13891389
}
13901390

1391+
/// Returns whether or not the crate with CrateNum 'cnum'
1392+
/// is marked as a private dependency
1393+
pub fn is_private_dep(self, cnum: CrateNum) -> bool {
1394+
if cnum == LOCAL_CRATE {
1395+
false
1396+
} else {
1397+
self.cstore.crate_is_private_dep_untracked(cnum)
1398+
}
1399+
}
1400+
13911401
#[inline]
13921402
pub fn def_path_hash(self, def_id: DefId) -> hir_map::DefPathHash {
13931403
if def_id.is_local() {

‎src/librustc_metadata/creader.rs

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,9 @@ impl<'a> CrateLoader<'a> {
131131
// `source` stores paths which are normalized which may be different
132132
// from the strings on the command line.
133133
let source = &self.cstore.get_crate_data(cnum).source;
134-
if let Some(locs) = self.sess.opts.externs.get(&*name.as_str()) {
134+
if let Some(entry) = self.sess.opts.externs.get(&*name.as_str()) {
135135
// Only use `--extern crate_name=path` here, not `--extern crate_name`.
136-
let found = locs.iter().filter_map(|l| l.as_ref()).any(|l| {
136+
let found = entry.locations.iter().filter_map(|l| l.as_ref()).any(|l| {
137137
let l = fs::canonicalize(l).ok();
138138
source.dylib.as_ref().map(|p| &p.0) == l.as_ref() ||
139139
source.rlib.as_ref().map(|p| &p.0) == l.as_ref()
@@ -195,12 +195,20 @@ impl<'a> CrateLoader<'a> {
195195
ident: Symbol,
196196
span: Span,
197197
lib: Library,
198-
dep_kind: DepKind
198+
dep_kind: DepKind,
199+
name: Symbol
199200
) -> (CrateNum, Lrc<cstore::CrateMetadata>) {
200201
let crate_root = lib.metadata.get_root();
201-
info!("register crate `extern crate {} as {}`", crate_root.name, ident);
202202
self.verify_no_symbol_conflicts(span, &crate_root);
203203

204+
let private_dep = self.sess.opts.externs.get(&name.as_str())
205+
.map(|e| e.is_private_dep)
206+
.unwrap_or(false);
207+
208+
info!("register crate `extern crate {} as {}` (private_dep = {})",
209+
crate_root.name, ident, private_dep);
210+
211+
204212
// Claim this crate number and cache it
205213
let cnum = self.cstore.alloc_new_crate_num();
206214

@@ -272,7 +280,8 @@ impl<'a> CrateLoader<'a> {
272280
dylib,
273281
rlib,
274282
rmeta,
275-
}
283+
},
284+
private_dep
276285
};
277286

278287
let cmeta = Lrc::new(cmeta);
@@ -390,7 +399,7 @@ impl<'a> CrateLoader<'a> {
390399
Ok((cnum, data))
391400
}
392401
(LoadResult::Loaded(library), host_library) => {
393-
Ok(self.register_crate(host_library, root, ident, span, library, dep_kind))
402+
Ok(self.register_crate(host_library, root, ident, span, library, dep_kind, name))
394403
}
395404
_ => panic!()
396405
}

‎src/librustc_metadata/cstore.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,10 @@ pub struct CrateMetadata {
7979
pub source: CrateSource,
8080

8181
pub proc_macros: Option<Vec<(ast::Name, Lrc<SyntaxExtension>)>>,
82+
83+
/// Whether or not this crate should be consider a private dependency
84+
/// for purposes of the 'exported_private_dependencies' lint
85+
pub private_dep: bool
8286
}
8387

8488
pub struct CStore {
@@ -114,7 +118,8 @@ impl CStore {
114118
}
115119

116120
pub(super) fn get_crate_data(&self, cnum: CrateNum) -> Lrc<CrateMetadata> {
117-
self.metas.borrow()[cnum].clone().unwrap()
121+
self.metas.borrow()[cnum].clone()
122+
.unwrap_or_else(|| panic!("Failed to get crate data for {:?}", cnum))
118123
}
119124

120125
pub(super) fn set_crate_data(&self, cnum: CrateNum, data: Lrc<CrateMetadata>) {

‎src/librustc_metadata/cstore_impl.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -494,6 +494,10 @@ impl CrateStore for cstore::CStore {
494494
self.get_crate_data(cnum).name
495495
}
496496

497+
fn crate_is_private_dep_untracked(&self, cnum: CrateNum) -> bool {
498+
self.get_crate_data(cnum).private_dep
499+
}
500+
497501
fn crate_disambiguator_untracked(&self, cnum: CrateNum) -> CrateDisambiguator
498502
{
499503
self.get_crate_data(cnum).root.disambiguator

‎src/librustc_metadata/locator.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -442,11 +442,11 @@ impl<'a> Context<'a> {
442442
// must be loaded via -L plus some filtering.
443443
if self.hash.is_none() {
444444
self.should_match_name = false;
445-
if let Some(s) = self.sess.opts.externs.get(&self.crate_name.as_str()) {
445+
if let Some(entry) = self.sess.opts.externs.get(&self.crate_name.as_str()) {
446446
// Only use `--extern crate_name=path` here, not `--extern crate_name`.
447-
if s.iter().any(|l| l.is_some()) {
447+
if entry.locations.iter().any(|l| l.is_some()) {
448448
return self.find_commandline_library(
449-
s.iter().filter_map(|l| l.as_ref()),
449+
entry.locations.iter().filter_map(|l| l.as_ref()),
450450
);
451451
}
452452
}

‎src/librustc_privacy/lib.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1540,7 +1540,6 @@ struct SearchInterfaceForPrivateItemsVisitor<'a, 'tcx: 'a> {
15401540
has_pub_restricted: bool,
15411541
has_old_errors: bool,
15421542
in_assoc_ty: bool,
1543-
private_crates: FxHashSet<CrateNum>
15441543
}
15451544

15461545
impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> {
@@ -1622,7 +1621,7 @@ impl<'a, 'tcx: 'a> SearchInterfaceForPrivateItemsVisitor<'a, 'tcx> {
16221621
/// 2. It comes from a private crate
16231622
fn leaks_private_dep(&self, item_id: DefId) -> bool {
16241623
let ret = self.required_visibility == ty::Visibility::Public &&
1625-
self.private_crates.contains(&item_id.krate);
1624+
self.tcx.is_private_dep(item_id.krate);
16261625

16271626
log::debug!("leaks_private_dep(item_id={:?})={}", item_id, ret);
16281627
return ret;
@@ -1640,7 +1639,6 @@ struct PrivateItemsInPublicInterfacesVisitor<'a, 'tcx: 'a> {
16401639
tcx: TyCtxt<'a, 'tcx, 'tcx>,
16411640
has_pub_restricted: bool,
16421641
old_error_set: &'a HirIdSet,
1643-
private_crates: FxHashSet<CrateNum>
16441642
}
16451643

16461644
impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> {
@@ -1678,7 +1676,6 @@ impl<'a, 'tcx> PrivateItemsInPublicInterfacesVisitor<'a, 'tcx> {
16781676
has_pub_restricted: self.has_pub_restricted,
16791677
has_old_errors,
16801678
in_assoc_ty: false,
1681-
private_crates: self.private_crates.clone()
16821679
}
16831680
}
16841681

@@ -1876,17 +1873,11 @@ fn check_private_in_public<'tcx>(tcx: TyCtxt<'_, 'tcx, 'tcx>, krate: CrateNum) {
18761873
pub_restricted_visitor.has_pub_restricted
18771874
};
18781875

1879-
let private_crates: FxHashSet<CrateNum> = tcx.sess.opts.extern_private.iter()
1880-
.flat_map(|c| {
1881-
tcx.crates().iter().find(|&&krate| &tcx.crate_name(krate) == c).cloned()
1882-
}).collect();
1883-
18841876
// Check for private types and traits in public interfaces.
18851877
let mut visitor = PrivateItemsInPublicInterfacesVisitor {
18861878
tcx,
18871879
has_pub_restricted,
18881880
old_error_set: &visitor.old_error_set,
1889-
private_crates
18901881
};
18911882
krate.visit_all_item_likes(&mut DeepVisitor::new(&mut visitor));
18921883
}

‎src/librustdoc/config.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use std::collections::{BTreeMap, BTreeSet};
1+
use std::collections::BTreeMap;
22
use std::fmt;
33
use std::path::PathBuf;
44

@@ -9,7 +9,7 @@ use rustc::lint::Level;
99
use rustc::session::early_error;
1010
use rustc::session::config::{CodegenOptions, DebuggingOptions, ErrorOutputType, Externs};
1111
use rustc::session::config::{nightly_options, build_codegen_options, build_debugging_options,
12-
get_cmd_lint_options};
12+
get_cmd_lint_options, ExternEntry};
1313
use rustc::session::search_paths::SearchPath;
1414
use rustc_driver;
1515
use rustc_target::spec::TargetTriple;
@@ -578,7 +578,7 @@ fn parse_extern_html_roots(
578578
/// error message.
579579
// FIXME(eddyb) This shouldn't be duplicated with `rustc::session`.
580580
fn parse_externs(matches: &getopts::Matches) -> Result<Externs, String> {
581-
let mut externs: BTreeMap<_, BTreeSet<_>> = BTreeMap::new();
581+
let mut externs: BTreeMap<_, ExternEntry> = BTreeMap::new();
582582
for arg in &matches.opt_strs("extern") {
583583
let mut parts = arg.splitn(2, '=');
584584
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<Externs, String> {
588588
enable `--extern crate_name` without `=path`".to_string());
589589
}
590590
let name = name.to_string();
591-
externs.entry(name).or_default().insert(location);
591+
// For Rustdoc purposes, we can treat all externs as public
592+
externs.entry(name)
593+
.or_default()
594+
.locations.insert(location.clone());
592595
}
593596
Ok(Externs::new(externs))
594597
}

0 commit comments

Comments
 (0)