Don't mark reachable extern fns as internal

If a function is marked as external, then it's likely desired for use with some
native library, so we're not really accomplishing a whole lot by internalizing
all of these symbols.

Author: alexcrichton

src/librustc/middle/reachable.rs

@@ -260,104 +260,133 @@ impl ReachableContext {
                 continue
             }
             scanned.insert(search_item);
-            self.reachable_symbols.insert(search_item);
-
-            // Find the AST block corresponding to the item and visit it,
-            // marking all path expressions that resolve to something
-            // interesting.
             match self.tcx.items.find(&search_item) {
-                Some(&ast_map::node_item(item, _)) => {
+                Some(item) => self.propagate_node(item, search_item,
+                                                  &mut visitor),
+                None if search_item == ast::CRATE_NODE_ID => {}
+                None => {
+                    self.tcx.sess.bug(format!("found unmapped ID in worklist: \
+                                               {}",
+                                              search_item))
+                }
+            }
+        }
+    }
+
+    fn propagate_node(&self, node: &ast_map::ast_node,
+                      search_item: ast::NodeId,
+                      visitor: &mut MarkSymbolVisitor) {
+        if !*self.tcx.sess.building_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)
+            match *node {
+                ast_map::node_item(item, _) => {
                     match item.node {
-                        ast::item_fn(_, _, _, _, ref search_block) => {
-                            if item_might_be_inlined(item) {
-                                visit::walk_block(&mut visitor, search_block, ())
-                            }
+                        ast::item_fn(_, ast::extern_fn, _, _, _) => {
+                            self.reachable_symbols.insert(search_item);
                         }
+                        _ => {}
+                    }
+                }
+                _ => {}
+            }
+        } else {
+            // If we are building a library, then reachable symbols will
+            // continue to participate in linkage after this product is
+            // produced. In this case, we traverse the ast node, recursing on
+            // all reachable nodes from this one.
+            self.reachable_symbols.insert(search_item);
+        }
 
-                        // Implementations of exported structs/enums need to get
-                        // added to the worklist (as all their methods should be
-                        // accessible)
-                        ast::item_struct(*) | ast::item_enum(*) => {
-                            let def = local_def(item.id);
-                            let impls = match self.tcx.inherent_impls.find(&def) {
-                                Some(&impls) => impls,
-                                None => continue
-                            };
-                            for imp in impls.iter() {
-                                if is_local(imp.did) {
-                                    self.worklist.push(imp.did.node);
-                                }
-                            }
+        match *node {
+            ast_map::node_item(item, _) => {
+                match item.node {
+                    ast::item_fn(_, _, _, _, ref search_block) => {
+                        if item_might_be_inlined(item) {
+                            visit::walk_block(visitor, search_block, ())
                         }
+                    }
 
-                        // Propagate through this impl
-                        ast::item_impl(_, _, _, ref methods) => {
-                            for method in methods.iter() {
-                                self.worklist.push(method.id);
+                    // Implementations of exported structs/enums need to get
+                    // added to the worklist (as all their methods should be
+                    // accessible)
+                    ast::item_struct(*) | ast::item_enum(*) => {
+                        let def = local_def(item.id);
+                        let impls = match self.tcx.inherent_impls.find(&def) {
+                            Some(&impls) => impls,
+                            None => return
+                        };
+                        for imp in impls.iter() {
+                            if is_local(imp.did) {
+                                self.worklist.push(imp.did.node);
                             }
                         }
+                    }
+
+                    // Propagate through this impl
+                    ast::item_impl(_, _, _, ref methods) => {
+                        for method in methods.iter() {
+                            self.worklist.push(method.id);
+                        }
+                    }
 
-                        // Default methods of exported traits need to all be
-                        // accessible.
-                        ast::item_trait(_, _, ref methods) => {
-                            for method in methods.iter() {
-                                match *method {
-                                    ast::required(*) => {}
-                                    ast::provided(ref method) => {
-                                        self.worklist.push(method.id);
-                                    }
+                    // Default methods of exported traits need to all be
+                    // accessible.
+                    ast::item_trait(_, _, ref methods) => {
+                        for method in methods.iter() {
+                            match *method {
+                                ast::required(*) => {}
+                                ast::provided(ref method) => {
+                                    self.worklist.push(method.id);
                                 }
                             }
                         }
+                    }
 
-                        // These are normal, nothing reachable about these
-                        // inherently and their children are already in the
-                        // worklist
-                        ast::item_static(*) | ast::item_ty(*) |
-                            ast::item_mod(*) | ast::item_foreign_mod(*) => {}
+                    // These are normal, nothing reachable about these
+                    // inherently and their children are already in the
+                    // worklist
+                    ast::item_static(*) | ast::item_ty(*) |
+                        ast::item_mod(*) | ast::item_foreign_mod(*) => {}
 
-                        _ => {
-                            self.tcx.sess.span_bug(item.span,
-                                                   "found non-function item \
-                                                    in worklist?!")
-                        }
+                    _ => {
+                        self.tcx.sess.span_bug(item.span,
+                                               "found non-function item \
+                                                in worklist?!")
                     }
                 }
-                Some(&ast_map::node_trait_method(trait_method, _, _)) => {
-                    match *trait_method {
-                        ast::required(*) => {
-                            // Keep going, nothing to get exported
-                        }
-                        ast::provided(ref method) => {
-                            visit::walk_block(&mut visitor, &method.body, ())
-                        }
+            }
+            ast_map::node_trait_method(trait_method, _, _) => {
+                match *trait_method {
+                    ast::required(*) => {
+                        // Keep going, nothing to get exported
                     }
-                }
-                Some(&ast_map::node_method(method, did, _)) => {
-                    if method_might_be_inlined(self.tcx, method, did) {
-                        visit::walk_block(&mut visitor, &method.body, ())
+                    ast::provided(ref method) => {
+                        visit::walk_block(visitor, &method.body, ())
                     }
                 }
-                // Nothing to recurse on for these
-                Some(&ast_map::node_foreign_item(*)) |
-                Some(&ast_map::node_variant(*)) |
-                Some(&ast_map::node_struct_ctor(*)) => {}
-                Some(_) => {
-                    let ident_interner = token::get_ident_interner();
-                    let desc = ast_map::node_id_to_str(self.tcx.items,
-                                                       search_item,
-                                                       ident_interner);
-                    self.tcx.sess.bug(format!("found unexpected thingy in \
-                                               worklist: {}",
-                                               desc))
-                }
-                None if search_item == ast::CRATE_NODE_ID => {}
-                None => {
-                    self.tcx.sess.bug(format!("found unmapped ID in worklist: \
-                                               {}",
-                                              search_item))
+            }
+            ast_map::node_method(method, did, _) => {
+                if method_might_be_inlined(self.tcx, method, did) {
+                    visit::walk_block(visitor, &method.body, ())
                 }
             }
+            // Nothing to recurse on for these
+            ast_map::node_foreign_item(*) |
+            ast_map::node_variant(*) |
+            ast_map::node_struct_ctor(*) => {}
+            _ => {
+                let ident_interner = token::get_ident_interner();
+                let desc = ast_map::node_id_to_str(self.tcx.items,
+                                                   search_item,
+                                                   ident_interner);
+                self.tcx.sess.bug(format!("found unexpected thingy in \
+                                           worklist: {}",
+                                           desc))
+            }
         }
     }
 

src/librustc/middle/trans/base.rs

@@ -2294,7 +2294,7 @@ fn finish_register_fn(ccx: @mut CrateContext, sp: Span, sym: ~str, node_id: ast:
                       llfn: ValueRef) {
     ccx.item_symbols.insert(node_id, sym);
 
-    if !*ccx.sess.building_library {
+    if !ccx.reachable.contains(&node_id) {
         lib::llvm::SetLinkage(llfn, lib::llvm::InternalLinkage);
     }
 
@@ -2504,7 +2504,7 @@ pub fn get_item_val(ccx: @mut CrateContext, id: ast::NodeId) -> ValueRef {
                                     llvm::LLVMAddGlobal(ccx.llmod, llty, buf)
                                 };
 
-                                if !*ccx.sess.building_library {
+                                if !ccx.reachable.contains(&id) {
                                     lib::llvm::SetLinkage(g, lib::llvm::InternalLinkage);
                                 }
 

src/test/run-pass/extern-fn-reachable.rs

@@ -0,0 +1,39 @@
+// 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.
+
+// xfail-fast
+// xfail-linux apparently dlsym doesn't work on program symbols?
+// xfail-android apparently dlsym doesn't work on program symbols?
+// xfail-freebsd apparently dlsym doesn't work on program symbols?
+
+use std::unstable::dynamic_lib::DynamicLibrary;
+
+#[no_mangle] pub extern "C" fn fun1() {}
+#[no_mangle] extern "C" fn fun2() {}
+
+mod foo {
+    #[no_mangle] pub extern "C" fn fun3() {}
+}
+pub mod bar {
+    #[no_mangle] pub extern "C" fn fun4() {}
+}
+
+#[no_mangle] pub fn fun5() {}
+
+fn main() {
+    unsafe {
+        let a = DynamicLibrary::open(None).unwrap();
+        assert!(a.symbol::<int>("fun1").is_ok());
+        assert!(a.symbol::<int>("fun2").is_err());
+        assert!(a.symbol::<int>("fun3").is_err());
+        assert!(a.symbol::<int>("fun4").is_ok());
+        assert!(a.symbol::<int>("fun5").is_err());
+    }
+}