Reduce the aggressiveness of reachability

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).

diff --git a/src/librustc/middle/reachable.rs b/src/librustc/middle/reachable.rs
index f65eeee49d510bf9a3c360f0c46d9ec2fefb82c4..840e3081656c097d637aa7b5e8270c5513cc0270 100644 (file)
--- a/src/librustc/middle/reachable.rs
+++ b/src/librustc/middle/reachable.rs
@@ -50,6 +50,7 @@ fn item_might_be_inlined(item: @ast::item) -> bool {
     }
 
     match item.node {
+        ast::item_impl(ref generics, _, _, _) |
         ast::item_fn(_, _, _, ref generics, _) => {
             generics_require_inlining(generics)
         }
@@ -64,6 +65,25 @@ fn ty_method_might_be_inlined(ty_method: &ast::TypeMethod) -> bool {
         generics_require_inlining(&ty_method.generics)
 }
 
+fn method_might_be_inlined(tcx: ty::ctxt, method: &ast::method,
+                           impl_src: ast::DefId) -> bool {
+    if attributes_specify_inlining(method.attrs) ||
+        generics_require_inlining(&method.generics) {
+        return true
+    }
+    if is_local(impl_src) {
+        match tcx.items.find(&impl_src.node) {
+            Some(&ast_map::node_item(item, _)) => item_might_be_inlined(item),
+            Some(*) | None => {
+                tcx.sess.span_bug(method.span, "impl did is not an item")
+            }
+        }
+    } else {
+        tcx.sess.span_bug(method.span, "found a foreign impl as a parent of a \
+                                        local method")
+    }
+}
+
 // Returns true if the given trait method must be inlined because it may be
 // monomorphized or it was marked with `#[inline]`.
 fn trait_method_might_be_inlined(trait_method: &ast::trait_method) -> bool {
@@ -100,50 +120,54 @@ impl Visitor<()> for MarkSymbolVisitor {
 
     fn visit_expr(&mut self, expr:@ast::Expr, _:()) {
 
-                match expr.node {
-                    ast::ExprPath(_) => {
-                        let def = match self.tcx.def_map.find(&expr.id) {
-                            Some(&def) => def,
-                            None => {
-                                self.tcx.sess.span_bug(expr.span,
-                                                  "def ID not in def map?!")
-                            }
-                        };
+        match expr.node {
+            ast::ExprPath(_) => {
+                let def = match self.tcx.def_map.find(&expr.id) {
+                    Some(&def) => def,
+                    None => {
+                        self.tcx.sess.span_bug(expr.span,
+                                               "def ID not in def map?!")
+                    }
+                };
 
-                        let def_id = def_id_of_def(def);
+                let def_id = def_id_of_def(def);
+                if ReachableContext::
+                    def_id_represents_local_inlined_item(self.tcx, def_id) {
+                        self.worklist.push(def_id.node)
+                    }
+                self.reachable_symbols.insert(def_id.node);
+            }
+            ast::ExprMethodCall(*) => {
+                match self.method_map.find(&expr.id) {
+                    Some(&typeck::method_map_entry {
+                        origin: typeck::method_static(def_id),
+                        _
+                    }) => {
                         if ReachableContext::
-                                def_id_represents_local_inlined_item(self.tcx,
-                                                                     def_id) {
-                            self.worklist.push(def_id.node)
-                        }
+                            def_id_represents_local_inlined_item(
+                                self.tcx,
+                                def_id) {
+                                self.worklist.push(def_id.node)
+                            }
                         self.reachable_symbols.insert(def_id.node);
                     }
-                    ast::ExprMethodCall(*) => {
-                        match self.method_map.find(&expr.id) {
-                            Some(&typeck::method_map_entry {
-                                origin: typeck::method_static(def_id),
-                                _
-                            }) => {
-                                if ReachableContext::
-                                    def_id_represents_local_inlined_item(
-                                        self.tcx,
-                                        def_id) {
-                                    self.worklist.push(def_id.node)
-                                }
-                                self.reachable_symbols.insert(def_id.node);
-                            }
-                            Some(_) => {}
-                            None => {
-                                self.tcx.sess.span_bug(expr.span,
-                                                  "method call expression \
+                    Some(_) => {}
+                    None => {
+                        self.tcx.sess.span_bug(expr.span,
+                        "method call expression \
                                                    not in method map?!")
-                            }
-                        }
                     }
-                    _ => {}
                 }
+            }
+            _ => {}
+        }
 
-                visit::walk_expr(self, expr, ())
+        visit::walk_expr(self, expr, ())
+    }
+
+    fn visit_item(&mut self, _item: @ast::item, _: ()) {
+        // Do not recurse into items. These items will be added to the worklist
+        // and recursed into manually if necessary.
     }
 }
 
@@ -263,8 +287,11 @@ fn propagate(&self) {
                 Some(&ast_map::node_item(item, _)) => {
                     match item.node {
                         ast::item_fn(_, _, _, _, ref search_block) => {
-                            visit::walk_block(&mut visitor, search_block, ())
+                            if item_might_be_inlined(item) {
+                                visit::walk_block(&mut visitor, search_block, ())
+                            }
                         }
+
                         // Our recursion into modules involves looking up their
                         // public reexports and the destinations of those
                         // exports. Privacy will put them in the worklist, but
@@ -331,8 +358,10 @@ fn propagate(&self) {
                         }
                     }
                 }
-                Some(&ast_map::node_method(ref method, _, _)) => {
-                    visit::walk_block(&mut visitor, &method.body, ())
+                Some(&ast_map::node_method(method, did, _)) => {
+                    if method_might_be_inlined(self.tcx, method, did) {
+                        visit::walk_block(&mut visitor, &method.body, ())
+                    }
                 }
                 // Nothing to recurse on for these
                 Some(&ast_map::node_foreign_item(*)) |
@@ -378,17 +407,6 @@ pub fn find_reachable(tcx: ty::ctxt,
                       exp_map2: resolve::ExportMap2,
                       exported_items: &privacy::ExportedItems)
                       -> @mut HashSet<ast::NodeId> {
-    // XXX(pcwalton): We only need to mark symbols that are exported. But this
-    // is more complicated than just looking at whether the symbol is `pub`,
-    // because it might be the target of a `pub use` somewhere. For now, I
-    // think we are fine, because you can't `pub use` something that wasn't
-    // exported due to the bug whereby `use` only looks through public
-    // modules even if you're inside the module the `use` appears in. When
-    // this bug is fixed, however, this code will need to be updated. Probably
-    // the easiest way to fix this (although a conservative overapproximation)
-    // is to have the name resolution pass mark all targets of a `pub use` as
-    // "must be reachable".
-
     let reachable_context = ReachableContext::new(tcx, method_map, exp_map2);
 
     // Step 1: Seed the worklist with all nodes which were found to be public as

diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs
index 2b7ed59b489c6652f91a7b1652f841b591696129..2482a7d2fcc26fc62af859288be6787ea609c6c6 100644 (file)
--- a/src/librustc/middle/trans/base.rs
+++ b/src/librustc/middle/trans/base.rs
@@ -2469,7 +2469,7 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
     match val {
         Some(v) => v,
         None => {
-            let mut exprt = false;
+            let mut foreign = false;
             let item = ccx.tcx.items.get_copy(&id);
             let val = match item {
                 ast_map::node_item(i, pth) => {
@@ -2582,7 +2582,6 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
                                          get_item_val()");
                         }
                         ast::provided(m) => {
-                            exprt = true;
                             register_method(ccx, id, pth, m)
                         }
                     }
@@ -2594,7 +2593,7 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
 
                 ast_map::node_foreign_item(ni, abis, _, pth) => {
                     let ty = ty::node_id_to_type(ccx.tcx, ni.id);
-                    exprt = true;
+                    foreign = true;
 
                     match ni.node {
                         ast::foreign_item_fn(*) => {
@@ -2688,7 +2687,10 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
                 }
             };
 
-            if !exprt && !ccx.reachable.contains(&id) {
+            // foreign items (extern fns and extern statics) don't have internal
+            // linkage b/c that doesn't quite make sense. Otherwise items can
+            // have internal linkage if they're not reachable.
+            if !foreign && !ccx.reachable.contains(&id) {
                 lib::llvm::SetLinkage(val, lib::llvm::InternalLinkage);
             }
 

diff --git a/src/test/auxiliary/linkage-visibility.rs b/src/test/auxiliary/linkage-visibility.rs
new file mode 100644 (file)
index 0000000..ab3539e
--- /dev/null
+++ b/src/test/auxiliary/linkage-visibility.rs
@@ -0,0 +1,36 @@
+// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+use std::unstable::dynamic_lib::DynamicLibrary;
+
+#[no_mangle]
+pub fn foo() { bar(); }
+
+pub fn foo2<T>() {
+    fn bar2() {
+        bar();
+    }
+    bar2();
+}
+
+#[no_mangle]
+fn bar() { }
+
+#[no_mangle]
+fn baz() { }
+
+pub fn test() {
+    let lib = DynamicLibrary::open(None).unwrap();
+    unsafe {
+        assert!(lib.symbol::<int>("foo").is_ok());
+        assert!(lib.symbol::<int>("baz").is_err());
+        assert!(lib.symbol::<int>("bar").is_err());
+    }
+}

diff --git a/src/test/run-pass/linkage-visibility.rs b/src/test/run-pass/linkage-visibility.rs
new file mode 100644 (file)
index 0000000..dff45a2
--- /dev/null
+++ b/src/test/run-pass/linkage-visibility.rs
@@ -0,0 +1,20 @@
+// Copyright 2013 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+// aux-build:linkage-visibility.rs
+// xfail-fast windows doesn't like aux-build
+
+extern mod foo(name = "linkage-visibility");
+
+fn main() {
+    foo::test();
+    foo::foo2::<int>();
+    foo::foo();
+}