From e6f0f7d52d69d0c4f290ac32c6eebf92bbf6dd81 Mon Sep 17 00:00:00 2001 From: arcnmx Date: Sat, 7 Nov 2015 12:22:04 -0500 Subject: [PATCH 1/4] Only retain external static symbols across LTO --- src/librustc/middle/cstore.rs | 6 ++--- src/librustc/middle/reachable.rs | 18 +++++++-------- src/librustc_metadata/csearch.rs | 11 ++------- src/librustc_metadata/decoder.rs | 39 ++++++++++++++++---------------- src/librustc_trans/trans/base.rs | 3 +-- src/libsyntax/attr.rs | 7 ++++++ 6 files changed, 41 insertions(+), 43 deletions(-) diff --git a/src/librustc/middle/cstore.rs b/src/librustc/middle/cstore.rs index 1e21d6aabe97b..c2c23f6a03250 100644 --- a/src/librustc/middle/cstore.rs +++ b/src/librustc/middle/cstore.rs @@ -187,8 +187,7 @@ pub trait CrateStore<'tcx> : Any { fn is_defaulted_trait(&self, did: DefId) -> bool; fn is_impl(&self, did: DefId) -> bool; fn is_default_impl(&self, impl_did: DefId) -> bool; - fn is_extern_fn(&self, tcx: &ty::ctxt<'tcx>, did: DefId) -> bool; - fn is_static(&self, did: DefId) -> bool; + fn is_extern_item(&self, tcx: &ty::ctxt<'tcx>, did: DefId) -> bool; fn is_static_method(&self, did: DefId) -> bool; fn is_statically_included_foreign_item(&self, id: ast::NodeId) -> bool; fn is_typedef(&self, did: DefId) -> bool; @@ -357,8 +356,7 @@ impl<'tcx> CrateStore<'tcx> for DummyCrateStore { fn is_defaulted_trait(&self, did: DefId) -> bool { unimplemented!() } fn is_impl(&self, did: DefId) -> bool { unimplemented!() } fn is_default_impl(&self, impl_did: DefId) -> bool { unimplemented!() } - fn is_extern_fn(&self, tcx: &ty::ctxt<'tcx>, did: DefId) -> bool { unimplemented!() } - fn is_static(&self, did: DefId) -> bool { unimplemented!() } + fn is_extern_item(&self, tcx: &ty::ctxt<'tcx>, did: DefId) -> bool { unimplemented!() } fn is_static_method(&self, did: DefId) -> bool { unimplemented!() } fn is_statically_included_foreign_item(&self, id: ast::NodeId) -> bool { false } fn is_typedef(&self, did: DefId) -> bool { unimplemented!() } diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs index d34c3b742179c..5486172b830d2 100644 --- a/src/librustc/middle/reachable.rs +++ b/src/librustc/middle/reachable.rs @@ -229,16 +229,16 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> { fn propagate_node(&mut self, node: &ast_map::Node, search_item: ast::NodeId) { if !self.any_library { - // If we are building an executable, then there's no need to flag - // anything as external except for `extern fn` types. These - // functions may still participate in some form of native interface, - // but all other rust-only interfaces can be private (they will not - // participate in linkage after this product is produced) + // If we are building an executable, only explicitly extern + // types need to be exported. if let ast_map::NodeItem(item) = *node { - if let hir::ItemFn(_, _, _, abi, _, _) = item.node { - if abi != Abi::Rust { - self.reachable_symbols.insert(search_item); - } + let reachable = if let hir::ItemFn(_, _, _, abi, _, _) = item.node { + abi != Abi::Rust + } else { + false + }; + if reachable || attr::contains_extern_indicator(&item.attrs) { + self.reachable_symbols.insert(search_item); } } } else { diff --git a/src/librustc_metadata/csearch.rs b/src/librustc_metadata/csearch.rs index 06a2727ea28ae..5eade3bc0a54e 100644 --- a/src/librustc_metadata/csearch.rs +++ b/src/librustc_metadata/csearch.rs @@ -260,16 +260,9 @@ impl<'tcx> CrateStore<'tcx> for cstore::CStore { decoder::is_default_impl(&*cdata, impl_did.index) } - fn is_extern_fn(&self, tcx: &ty::ctxt<'tcx>, did: DefId) -> bool - { - let cdata = self.get_crate_data(did.krate); - decoder::is_extern_fn(&*cdata, did.index, tcx) - } - - fn is_static(&self, did: DefId) -> bool - { + fn is_extern_item(&self, tcx: &ty::ctxt<'tcx>, did: DefId) -> bool { let cdata = self.get_crate_data(did.krate); - decoder::is_static(&*cdata, did.index) + decoder::is_extern_item(&*cdata, did.index, tcx) } fn is_static_method(&self, def: DefId) -> bool diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 5ba9e566e0090..3ec32635a4bd4 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -1567,11 +1567,28 @@ pub fn is_const_fn(cdata: Cmd, id: DefIndex) -> bool { } } -pub fn is_static(cdata: Cmd, id: DefIndex) -> bool { - let item_doc = cdata.lookup_item(id); - match item_family(item_doc) { +pub fn is_extern_item(cdata: Cmd, id: DefIndex, tcx: &ty::ctxt) -> bool { + let item_doc = match cdata.get_item(id) { + Some(doc) => doc, + None => return false, + }; + let applicable = match item_family(item_doc) { ImmStatic | MutStatic => true, + Fn => { + let ty::TypeScheme { generics, ty } = get_type(cdata, id, tcx); + let no_generics = generics.types.is_empty(); + match ty.sty { + ty::TyBareFn(_, fn_ty) if fn_ty.abi != Abi::Rust => return no_generics, + _ => no_generics, + } + }, _ => false, + }; + + if applicable { + attr::contains_extern_indicator(&get_attributes(item_doc)) + } else { + false } } @@ -1693,22 +1710,6 @@ pub fn get_imported_filemaps(metadata: &[u8]) -> Vec { }).collect() } -pub fn is_extern_fn(cdata: Cmd, id: DefIndex, tcx: &ty::ctxt) -> bool { - let item_doc = match cdata.get_item(id) { - Some(doc) => doc, - None => return false, - }; - if let Fn = item_family(item_doc) { - let ty::TypeScheme { generics, ty } = get_type(cdata, id, tcx); - generics.types.is_empty() && match ty.sty { - ty::TyBareFn(_, fn_ty) => fn_ty.abi != Abi::Rust, - _ => false, - } - } else { - false - } -} - pub fn closure_kind(cdata: Cmd, closure_id: DefIndex) -> ty::ClosureKind { let closure_doc = cdata.lookup_item(closure_id); let closure_kind_doc = reader::get_doc(closure_doc, tag_items_closure_kind); diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index c2d962b032f01..4748c62921de6 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -3281,8 +3281,7 @@ pub fn trans_crate<'tcx>(tcx: &ty::ctxt<'tcx>, for cnum in sess.cstore.crates() { let syms = sess.cstore.reachable_ids(cnum); reachable_symbols.extend(syms.into_iter().filter(|did| { - sess.cstore.is_extern_fn(shared_ccx.tcx(), *did) || - sess.cstore.is_static(*did) + sess.cstore.is_extern_item(shared_ccx.tcx(), *did) }).map(|did| { sess.cstore.item_symbol(did) })); diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index cc5f30e21849f..e265437573646 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -316,6 +316,13 @@ pub fn find_export_name_attr(diag: &Handler, attrs: &[Attribute]) -> Option bool { + contains_name(attrs, "no_mangle") || + contains_name(attrs, "link_section") || + contains_name(attrs, "linkage") || + contains_name(attrs, "export_name") +} + #[derive(Copy, Clone, PartialEq)] pub enum InlineAttr { None, From 32328ac6ff13cf55b61dbfcf0c79aba1b7a8f429 Mon Sep 17 00:00:00 2001 From: arcnmx Date: Tue, 12 Jan 2016 13:07:04 -0500 Subject: [PATCH 2/4] Remove link_section and linkage as extern indicators --- src/libsyntax/attr.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index e265437573646..c3e3042eff08b 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -318,8 +318,6 @@ pub fn find_export_name_attr(diag: &Handler, attrs: &[Attribute]) -> Option bool { contains_name(attrs, "no_mangle") || - contains_name(attrs, "link_section") || - contains_name(attrs, "linkage") || contains_name(attrs, "export_name") } From a141c52816b606f01a3eb432ea1c5e1131513d1c Mon Sep 17 00:00:00 2001 From: arcnmx Date: Tue, 12 Jan 2016 15:55:59 -0500 Subject: [PATCH 3/4] Use find_export_name_attr instead of string literal --- src/librustc_trans/trans/base.rs | 2 +- src/libsyntax/attr.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 4748c62921de6..5a3c5ced8045f 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -2727,7 +2727,7 @@ fn exported_name<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, None => {} } - match attr::find_export_name_attr(ccx.sess().diagnostic(), attrs) { + match attr::find_export_name_attr(Some(ccx.sess().diagnostic()), attrs) { // Use provided name Some(name) => name.to_string(), _ => { diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index c3e3042eff08b..fca659e63b582 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -298,16 +298,16 @@ pub fn find_crate_name(attrs: &[Attribute]) -> Option { } /// Find the value of #[export_name=*] attribute and check its validity. -pub fn find_export_name_attr(diag: &Handler, attrs: &[Attribute]) -> Option { +pub fn find_export_name_attr(diag: Option<&Handler>, attrs: &[Attribute]) -> Option { attrs.iter().fold(None, |ia,attr| { if attr.check_name("export_name") { if let s@Some(_) = attr.value_str() { s } else { - diag.struct_span_err(attr.span, + diag.map(|d| d.struct_span_err(attr.span, "export_name attribute has invalid format") .help("use #[export_name=\"*\"]") - .emit(); + .emit()); None } } else { @@ -318,7 +318,7 @@ pub fn find_export_name_attr(diag: &Handler, attrs: &[Attribute]) -> Option bool { contains_name(attrs, "no_mangle") || - contains_name(attrs, "export_name") + find_export_name_attr(None, attrs).is_some() } #[derive(Copy, Clone, PartialEq)] From 0ff055ad6640e26a4ddf783682545320de0d1c03 Mon Sep 17 00:00:00 2001 From: arcnmx Date: Fri, 15 Jan 2016 14:43:14 -0500 Subject: [PATCH 4/4] Pass through diagnostic handler instead --- src/librustc/middle/reachable.rs | 4 +++- src/librustc_metadata/decoder.rs | 3 ++- src/librustc_trans/trans/base.rs | 2 +- src/libsyntax/attr.rs | 10 +++++----- 4 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs index 5486172b830d2..d2e1b3edc3a68 100644 --- a/src/librustc/middle/reachable.rs +++ b/src/librustc/middle/reachable.rs @@ -237,7 +237,9 @@ impl<'a, 'tcx> ReachableContext<'a, 'tcx> { } else { false }; - if reachable || attr::contains_extern_indicator(&item.attrs) { + let is_extern = attr::contains_extern_indicator(&self.tcx.sess.diagnostic(), + &item.attrs); + if reachable || is_extern { self.reachable_symbols.insert(search_item); } } diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index 3ec32635a4bd4..8409a0c74b869 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -1586,7 +1586,8 @@ pub fn is_extern_item(cdata: Cmd, id: DefIndex, tcx: &ty::ctxt) -> bool { }; if applicable { - attr::contains_extern_indicator(&get_attributes(item_doc)) + attr::contains_extern_indicator(tcx.sess.diagnostic(), + &get_attributes(item_doc)) } else { false } diff --git a/src/librustc_trans/trans/base.rs b/src/librustc_trans/trans/base.rs index 5a3c5ced8045f..4748c62921de6 100644 --- a/src/librustc_trans/trans/base.rs +++ b/src/librustc_trans/trans/base.rs @@ -2727,7 +2727,7 @@ fn exported_name<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, None => {} } - match attr::find_export_name_attr(Some(ccx.sess().diagnostic()), attrs) { + match attr::find_export_name_attr(ccx.sess().diagnostic(), attrs) { // Use provided name Some(name) => name.to_string(), _ => { diff --git a/src/libsyntax/attr.rs b/src/libsyntax/attr.rs index fca659e63b582..9953947eb52b7 100644 --- a/src/libsyntax/attr.rs +++ b/src/libsyntax/attr.rs @@ -298,16 +298,16 @@ pub fn find_crate_name(attrs: &[Attribute]) -> Option { } /// Find the value of #[export_name=*] attribute and check its validity. -pub fn find_export_name_attr(diag: Option<&Handler>, attrs: &[Attribute]) -> Option { +pub fn find_export_name_attr(diag: &Handler, attrs: &[Attribute]) -> Option { attrs.iter().fold(None, |ia,attr| { if attr.check_name("export_name") { if let s@Some(_) = attr.value_str() { s } else { - diag.map(|d| d.struct_span_err(attr.span, + diag.struct_span_err(attr.span, "export_name attribute has invalid format") .help("use #[export_name=\"*\"]") - .emit()); + .emit(); None } } else { @@ -316,9 +316,9 @@ pub fn find_export_name_attr(diag: Option<&Handler>, attrs: &[Attribute]) -> Opt }) } -pub fn contains_extern_indicator(attrs: &[Attribute]) -> bool { +pub fn contains_extern_indicator(diag: &Handler, attrs: &[Attribute]) -> bool { contains_name(attrs, "no_mangle") || - find_export_name_attr(None, attrs).is_some() + find_export_name_attr(diag, attrs).is_some() } #[derive(Copy, Clone, PartialEq)]