Skip to content

Commit 3899f8d

Browse files
borsbors
committed
auto merge of #10217 : alexcrichton/rust/less-reachable, r=pcwalton
Previously, all functions called by a reachable function were considered reachable, but this is only the case if the original function was possibly inlineable (if it's type generic or #[inline]-flagged).
2 parents 22dfdc9 + 681fda0 commit 3899f8d

File tree

4 files changed

+130
-54
lines changed

4 files changed

+130
-54
lines changed

‎src/librustc/middle/reachable.rs

+68-50
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ fn item_might_be_inlined(item: @ast::item) -> bool {
5050
}
5151

5252
match item.node {
53+
ast::item_impl(ref generics, _, _, _) |
5354
ast::item_fn(_, _, _, ref generics, _) => {
5455
generics_require_inlining(generics)
5556
}
@@ -64,6 +65,25 @@ fn ty_method_might_be_inlined(ty_method: &ast::TypeMethod) -> bool {
6465
generics_require_inlining(&ty_method.generics)
6566
}
6667

68+
fn method_might_be_inlined(tcx: ty::ctxt, method: &ast::method,
69+
impl_src: ast::DefId) -> bool {
70+
if attributes_specify_inlining(method.attrs) ||
71+
generics_require_inlining(&method.generics) {
72+
return true
73+
}
74+
if is_local(impl_src) {
75+
match tcx.items.find(&impl_src.node) {
76+
Some(&ast_map::node_item(item, _)) => item_might_be_inlined(item),
77+
Some(*) | None => {
78+
tcx.sess.span_bug(method.span, "impl did is not an item")
79+
}
80+
}
81+
} else {
82+
tcx.sess.span_bug(method.span, "found a foreign impl as a parent of a \
83+
local method")
84+
}
85+
}
86+
6787
// Returns true if the given trait method must be inlined because it may be
6888
// monomorphized or it was marked with `#[inline]`.
6989
fn trait_method_might_be_inlined(trait_method: &ast::trait_method) -> bool {
@@ -100,50 +120,54 @@ impl Visitor<()> for MarkSymbolVisitor {
100120

101121
fn visit_expr(&mut self, expr:@ast::Expr, _:()) {
102122

103-
match expr.node {
104-
ast::ExprPath(_) => {
105-
let def = match self.tcx.def_map.find(&expr.id) {
106-
Some(&def) => def,
107-
None => {
108-
self.tcx.sess.span_bug(expr.span,
109-
"def ID not in def map?!")
110-
}
111-
};
123+
match expr.node {
124+
ast::ExprPath(_) => {
125+
let def = match self.tcx.def_map.find(&expr.id) {
126+
Some(&def) => def,
127+
None => {
128+
self.tcx.sess.span_bug(expr.span,
129+
"def ID not in def map?!")
130+
}
131+
};
112132

113-
let def_id = def_id_of_def(def);
133+
let def_id = def_id_of_def(def);
134+
if ReachableContext::
135+
def_id_represents_local_inlined_item(self.tcx, def_id) {
136+
self.worklist.push(def_id.node)
137+
}
138+
self.reachable_symbols.insert(def_id.node);
139+
}
140+
ast::ExprMethodCall(*) => {
141+
match self.method_map.find(&expr.id) {
142+
Some(&typeck::method_map_entry {
143+
origin: typeck::method_static(def_id),
144+
_
145+
}) => {
114146
if ReachableContext::
115-
def_id_represents_local_inlined_item(self.tcx,
116-
def_id) {
117-
self.worklist.push(def_id.node)
118-
}
147+
def_id_represents_local_inlined_item(
148+
self.tcx,
149+
def_id) {
150+
self.worklist.push(def_id.node)
151+
}
119152
self.reachable_symbols.insert(def_id.node);
120153
}
121-
ast::ExprMethodCall(*) => {
122-
match self.method_map.find(&expr.id) {
123-
Some(&typeck::method_map_entry {
124-
origin: typeck::method_static(def_id),
125-
_
126-
}) => {
127-
if ReachableContext::
128-
def_id_represents_local_inlined_item(
129-
self.tcx,
130-
def_id) {
131-
self.worklist.push(def_id.node)
132-
}
133-
self.reachable_symbols.insert(def_id.node);
134-
}
135-
Some(_) => {}
136-
None => {
137-
self.tcx.sess.span_bug(expr.span,
138-
"method call expression \
154+
Some(_) => {}
155+
None => {
156+
self.tcx.sess.span_bug(expr.span,
157+
"method call expression \
139158
not in method map?!")
140-
}
141-
}
142159
}
143-
_ => {}
144160
}
161+
}
162+
_ => {}
163+
}
145164

146-
visit::walk_expr(self, expr, ())
165+
visit::walk_expr(self, expr, ())
166+
}
167+
168+
fn visit_item(&mut self, _item: @ast::item, _: ()) {
169+
// Do not recurse into items. These items will be added to the worklist
170+
// and recursed into manually if necessary.
147171
}
148172
}
149173

@@ -263,8 +287,11 @@ impl ReachableContext {
263287
Some(&ast_map::node_item(item, _)) => {
264288
match item.node {
265289
ast::item_fn(_, _, _, _, ref search_block) => {
266-
visit::walk_block(&mut visitor, search_block, ())
290+
if item_might_be_inlined(item) {
291+
visit::walk_block(&mut visitor, search_block, ())
292+
}
267293
}
294+
268295
// Our recursion into modules involves looking up their
269296
// public reexports and the destinations of those
270297
// exports. Privacy will put them in the worklist, but
@@ -331,8 +358,10 @@ impl ReachableContext {
331358
}
332359
}
333360
}
334-
Some(&ast_map::node_method(ref method, _, _)) => {
335-
visit::walk_block(&mut visitor, &method.body, ())
361+
Some(&ast_map::node_method(method, did, _)) => {
362+
if method_might_be_inlined(self.tcx, method, did) {
363+
visit::walk_block(&mut visitor, &method.body, ())
364+
}
336365
}
337366
// Nothing to recurse on for these
338367
Some(&ast_map::node_foreign_item(*)) |
@@ -378,17 +407,6 @@ pub fn find_reachable(tcx: ty::ctxt,
378407
exp_map2: resolve::ExportMap2,
379408
exported_items: &privacy::ExportedItems)
380409
-> @mut HashSet<ast::NodeId> {
381-
// XXX(pcwalton): We only need to mark symbols that are exported. But this
382-
// is more complicated than just looking at whether the symbol is `pub`,
383-
// because it might be the target of a `pub use` somewhere. For now, I
384-
// think we are fine, because you can't `pub use` something that wasn't
385-
// exported due to the bug whereby `use` only looks through public
386-
// modules even if you're inside the module the `use` appears in. When
387-
// this bug is fixed, however, this code will need to be updated. Probably
388-
// the easiest way to fix this (although a conservative overapproximation)
389-
// is to have the name resolution pass mark all targets of a `pub use` as
390-
// "must be reachable".
391-
392410
let reachable_context = ReachableContext::new(tcx, method_map, exp_map2);
393411

394412
// Step 1: Seed the worklist with all nodes which were found to be public as

‎src/librustc/middle/trans/base.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -2471,7 +2471,7 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
24712471
match val {
24722472
Some(v) => v,
24732473
None => {
2474-
let mut exprt = false;
2474+
let mut foreign = false;
24752475
let item = ccx.tcx.items.get_copy(&id);
24762476
let val = match item {
24772477
ast_map::node_item(i, pth) => {
@@ -2584,7 +2584,6 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
25842584
get_item_val()");
25852585
}
25862586
ast::provided(m) => {
2587-
exprt = true;
25882587
register_method(ccx, id, pth, m)
25892588
}
25902589
}
@@ -2596,7 +2595,7 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
25962595

25972596
ast_map::node_foreign_item(ni, abis, _, pth) => {
25982597
let ty = ty::node_id_to_type(ccx.tcx, ni.id);
2599-
exprt = true;
2598+
foreign = true;
26002599

26012600
match ni.node {
26022601
ast::foreign_item_fn(*) => {
@@ -2690,7 +2689,10 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
26902689
}
26912690
};
26922691

2693-
if !exprt && !ccx.reachable.contains(&id) {
2692+
// foreign items (extern fns and extern statics) don't have internal
2693+
// linkage b/c that doesn't quite make sense. Otherwise items can
2694+
// have internal linkage if they're not reachable.
2695+
if !foreign && !ccx.reachable.contains(&id) {
26942696
lib::llvm::SetLinkage(val, lib::llvm::InternalLinkage);
26952697
}
26962698

‎src/test/auxiliary/linkage-visibility.rs

+36
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
use std::unstable::dynamic_lib::DynamicLibrary;
12+
13+
#[no_mangle]
14+
pub fn foo() { bar(); }
15+
16+
pub fn foo2<T>() {
17+
fn bar2() {
18+
bar();
19+
}
20+
bar2();
21+
}
22+
23+
#[no_mangle]
24+
fn bar() { }
25+
26+
#[no_mangle]
27+
fn baz() { }
28+
29+
pub fn test() {
30+
let lib = DynamicLibrary::open(None).unwrap();
31+
unsafe {
32+
assert!(lib.symbol::<int>("foo").is_ok());
33+
assert!(lib.symbol::<int>("baz").is_err());
34+
assert!(lib.symbol::<int>("bar").is_err());
35+
}
36+
}

‎src/test/run-pass/linkage-visibility.rs

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// aux-build:linkage-visibility.rs
12+
// xfail-fast windows doesn't like aux-build
13+
14+
extern mod foo(name = "linkage-visibility");
15+
16+
fn main() {
17+
foo::test();
18+
foo::foo2::<int>();
19+
foo::foo();
20+
}

0 commit comments

Comments
 (0)