auto merge of #8666 : nikomatsakis/rust/issue-3678-extern-fn-types, r=pcwalton

Change the type of crust fns like this one:

    extern fn foo() { ... }

from `*u8` to `extern "C" fn()`.

r? @pcwalton (or whomever)

Author: bors

doc/rust.md

@@ -1006,20 +1006,25 @@ code_. They are defined in the same way as any other Rust function,
 except that they have the `extern` modifier.
 
 ~~~
+// Declares an extern fn, the ABI defaults to "C" 
 extern fn new_vec() -> ~[int] { ~[] }
+
+// Declares an extern fn with "stdcall" ABI
+extern "stdcall" fn new_vec_stdcall() -> ~[int] { ~[] }
 ~~~
 
-Extern functions may not be called from Rust code,
-but Rust code may take their value as a raw `u8` pointer.
+Unlike normal functions, extern fns have an `extern "ABI" fn()`.
+This is the same type as the functions declared in an extern
+block.
 
 ~~~
 # extern fn new_vec() -> ~[int] { ~[] }
-let fptr: *u8 = new_vec;
+let fptr: extern "C" fn() -> ~[int] = new_vec;
 ~~~
 
-The primary motivation for extern functions is
-to create callbacks for foreign functions that expect to receive function
-pointers.
+Extern functions may be called from Rust code, but
+caution must be taken with respect to the size of the stack
+segment, just as when calling an extern function normally.
 
 ### Type definitions
 
@@ -1384,14 +1389,13 @@ between the Rust ABI and the foreign ABI.
 A number of [attributes](#attributes) control the behavior of external
 blocks.
 
-By default external blocks assume
-that the library they are calling uses the standard C "cdecl" ABI.
-Other ABIs may be specified using the `abi` attribute as in
+By default external blocks assume that the library they are calling
+uses the standard C "cdecl" ABI.  Other ABIs may be specified using
+an `abi` string, as shown here:
 
 ~~~{.xfail-test}
 // Interface to the Windows API
-#[abi = "stdcall"]
-extern { }
+extern "stdcall" { }
 ~~~
 
 The `link_name` attribute allows the name of the library to be specified.
@@ -1407,6 +1411,12 @@ This is particularly useful for creating external blocks for libc,
 which tends to not follow standard library naming conventions
 and is linked to all Rust programs anyway.
 
+The type of a function
+declared in an extern block
+is `extern "abi" fn(A1, ..., An) -> R`,
+where `A1...An` are the declared types of its arguments
+and `R` is the decalred return type.
+
 ## Attributes
 
 ~~~~~~~~{.ebnf .gram}

src/libextra/rl.rs

@@ -16,27 +16,31 @@ use std::libc::{c_char, c_int};
 use std::local_data;
 use std::str;
 
+#[cfg(stage0)]
 pub mod rustrt {
     use std::libc::{c_char, c_int};
 
-    #[cfg(stage0)]
-    mod macro_hack {
-    #[macro_escape];
-    macro_rules! externfn(
-        (fn $name:ident ($($arg_name:ident : $arg_ty:ty),*) $(-> $ret_ty:ty),*) => (
-            extern {
-                fn $name($($arg_name : $arg_ty),*) $(-> $ret_ty),*;
-            }
-        )
-    )
+    extern {
+        fn linenoise(prompt: *c_char) -> *c_char;
+        fn linenoiseHistoryAdd(line: *c_char) -> c_int;
+        fn linenoiseHistorySetMaxLen(len: c_int) -> c_int;
+        fn linenoiseHistorySave(file: *c_char) -> c_int;
+        fn linenoiseHistoryLoad(file: *c_char) -> c_int;
+        fn linenoiseSetCompletionCallback(callback: *u8);
+        fn linenoiseAddCompletion(completions: *(), line: *c_char);
     }
+}
+
+#[cfg(not(stage0))]
+pub mod rustrt {
+    use std::libc::{c_char, c_int};
 
     externfn!(fn linenoise(prompt: *c_char) -> *c_char)
     externfn!(fn linenoiseHistoryAdd(line: *c_char) -> c_int)
     externfn!(fn linenoiseHistorySetMaxLen(len: c_int) -> c_int)
     externfn!(fn linenoiseHistorySave(file: *c_char) -> c_int)
     externfn!(fn linenoiseHistoryLoad(file: *c_char) -> c_int)
-    externfn!(fn linenoiseSetCompletionCallback(callback: *u8))
+    externfn!(fn linenoiseSetCompletionCallback(callback: extern "C" fn(*i8, *())))
     externfn!(fn linenoiseAddCompletion(completions: *(), line: *c_char))
 }
 

src/librustc/middle/trans/base.rs

@@ -52,6 +52,7 @@ use middle::trans::expr;
 use middle::trans::foreign;
 use middle::trans::glue;
 use middle::trans::inline;
+use middle::trans::llrepr::LlvmRepr;
 use middle::trans::machine;
 use middle::trans::machine::{llalign_of_min, llsize_of};
 use middle::trans::meth;
@@ -1739,6 +1740,10 @@ pub fn copy_args_to_allocas(fcx: @mut FunctionContext,
                             args: &[ast::arg],
                             raw_llargs: &[ValueRef],
                             arg_tys: &[ty::t]) -> @mut Block {
+    debug!("copy_args_to_allocas: raw_llargs=%s arg_tys=%s",
+           raw_llargs.llrepr(fcx.ccx),
+           arg_tys.repr(fcx.ccx.tcx));
+
     let _icx = push_ctxt("copy_args_to_allocas");
     let mut bcx = bcx;
 

src/librustc/middle/trans/builder.rs

@@ -22,6 +22,7 @@ use std::hashmap::HashMap;
 use std::libc::{c_uint, c_ulonglong, c_char};
 use std::vec;
 use syntax::codemap::span;
+use std::ptr::is_not_null;
 
 pub struct Builder {
     llbuilder: BuilderRef,
@@ -483,6 +484,7 @@ impl Builder {
         debug!("Store %s -> %s",
                self.ccx.tn.val_to_str(val),
                self.ccx.tn.val_to_str(ptr));
+        assert!(is_not_null(self.llbuilder));
         self.count_insn("store");
         unsafe {
             llvm::LLVMBuildStore(self.llbuilder, val, ptr);

src/librustc/middle/trans/expr.rs

@@ -824,56 +824,30 @@ fn trans_def_datum_unadjusted(bcx: @mut Block,
 {
     let _icx = push_ctxt("trans_def_datum_unadjusted");
 
-    match def {
+    let fn_data = match def {
         ast::def_fn(did, _) | ast::def_static_method(did, None, _) => {
-            let fn_data = callee::trans_fn_ref(bcx, did, ref_expr.id);
-            return fn_data_to_datum(bcx, ref_expr, did, fn_data);
+            callee::trans_fn_ref(bcx, did, ref_expr.id)
         }
         ast::def_static_method(impl_did, Some(trait_did), _) => {
-            let fn_data = meth::trans_static_method_callee(bcx, impl_did,
-                                                           trait_did,
-                                                           ref_expr.id);
-            return fn_data_to_datum(bcx, ref_expr, impl_did, fn_data);
+            meth::trans_static_method_callee(bcx, impl_did,
+                                             trait_did,
+                                             ref_expr.id)
         }
         _ => {
             bcx.tcx().sess.span_bug(ref_expr.span, fmt!(
                 "Non-DPS def %? referened by %s",
                 def, bcx.node_id_to_str(ref_expr.id)));
         }
-    }
+    };
 
-    fn fn_data_to_datum(bcx: @mut Block,
-                        ref_expr: &ast::expr,
-                        def_id: ast::def_id,
-                        fn_data: callee::FnData) -> DatumBlock {
-        /*!
-        *
-        * Translates a reference to a top-level fn item into a rust
-        * value.  This is just a fn pointer.
-        */
-
-        let is_extern = {
-            let fn_tpt = ty::lookup_item_type(bcx.tcx(), def_id);
-            ty::ty_fn_purity(fn_tpt.ty) == ast::extern_fn
-        };
-        let (rust_ty, llval) = if is_extern {
-            let rust_ty = ty::mk_ptr(
-                bcx.tcx(),
-                ty::mt {
-                    ty: ty::mk_mach_uint(ast::ty_u8),
-                    mutbl: ast::m_imm
-                }); // *u8
-            (rust_ty, PointerCast(bcx, fn_data.llfn, Type::i8p()))
-        } else {
-            let fn_ty = expr_ty(bcx, ref_expr);
-            (fn_ty, fn_data.llfn)
-        };
-        return DatumBlock {
-            bcx: bcx,
-            datum: Datum {val: llval,
-                          ty: rust_ty,
-                          mode: ByValue}
-        };
+    let fn_ty = expr_ty(bcx, ref_expr);
+    DatumBlock {
+        bcx: bcx,
+        datum: Datum {
+            val: fn_data.llfn,
+            ty: fn_ty,
+            mode: ByValue
+        }
     }
 }
 
@@ -1657,6 +1631,7 @@ pub fn cast_type_kind(t: ty::t) -> cast_kind {
         ty::ty_float(*)   => cast_float,
         ty::ty_ptr(*)     => cast_pointer,
         ty::ty_rptr(*)    => cast_pointer,
+        ty::ty_bare_fn(*) => cast_pointer,
         ty::ty_int(*)     => cast_integral,
         ty::ty_uint(*)    => cast_integral,
         ty::ty_bool       => cast_integral,
@@ -1719,10 +1694,16 @@ fn trans_imm_cast(bcx: @mut Block, expr: @ast::expr,
                                               val_ty(lldiscrim_a),
                                               lldiscrim_a, true),
                     cast_float => SIToFP(bcx, lldiscrim_a, ll_t_out),
-                    _ => ccx.sess.bug("translating unsupported cast.")
+                    _ => ccx.sess.bug(fmt!("translating unsupported cast: \
+                                           %s (%?) -> %s (%?)",
+                                           t_in.repr(ccx.tcx), k_in,
+                                           t_out.repr(ccx.tcx), k_out))
                 }
             }
-            _ => ccx.sess.bug("translating unsupported cast.")
+            _ => ccx.sess.bug(fmt!("translating unsupported cast: \
+                                   %s (%?) -> %s (%?)",
+                                   t_in.repr(ccx.tcx), k_in,
+                                   t_out.repr(ccx.tcx), k_out))
         };
     return immediate_rvalue_bcx(bcx, newval, t_out);
 }

src/librustc/middle/trans/foreign.rs

@@ -21,6 +21,7 @@ use middle::trans::cabi;
 use middle::trans::build::*;
 use middle::trans::builder::noname;
 use middle::trans::common::*;
+use middle::trans::llrepr::LlvmRepr;
 use middle::trans::type_of::*;
 use middle::trans::type_of;
 use middle::ty;
@@ -399,7 +400,29 @@ pub fn trans_rust_fn_with_foreign_abi(ccx: @mut CrateContext,
             ccx, vec::append_one((*path).clone(), ast_map::path_name(
                 special_idents::clownshoe_abi
             )));
-        let llty = type_of_fn_from_ty(ccx, t);
+
+        // Compute the LLVM type that the function would have if it
+        // were just a normal Rust function. This will be the type of
+        // the wrappee fn.
+        let llty = match ty::get(t).sty {
+            ty::ty_bare_fn(ref f) => {
+                assert!(!f.abis.is_rust() && !f.abis.is_intrinsic());
+                type_of_rust_fn(ccx, f.sig.inputs, f.sig.output)
+            }
+            _ => {
+                ccx.sess.bug(fmt!("build_rust_fn: extern fn %s has ty %s, \
+                                  expected a bare fn ty",
+                                  path.repr(tcx),
+                                  t.repr(tcx)));
+            }
+        };
+
+        debug!("build_rust_fn: path=%s id=%? t=%s llty=%s",
+               path.repr(tcx),
+               id,
+               t.repr(tcx),
+               llty.llrepr(ccx));
+
         let llfndecl = base::decl_internal_cdecl_fn(ccx.llmod, ps, llty);
         base::trans_fn(ccx,
                        (*path).clone(),

src/librustc/middle/trans/llrepr.rs

@@ -0,0 +1,38 @@
+// Copyright 2012 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 middle::trans::context::CrateContext;
+use middle::trans::type_::Type;
+use lib::llvm::ValueRef;
+
+pub trait LlvmRepr {
+    fn llrepr(&self, ccx: &CrateContext) -> ~str;
+}
+
+impl<'self, T:LlvmRepr> LlvmRepr for &'self [T] {
+    fn llrepr(&self, ccx: &CrateContext) -> ~str {
+        let reprs = self.map(|t| t.llrepr(ccx));
+        fmt!("[%s]", reprs.connect(","))
+    }
+}
+
+impl LlvmRepr for Type {
+    fn llrepr(&self, ccx: &CrateContext) -> ~str {
+        ccx.tn.type_to_str(*self)
+    }
+}
+
+impl LlvmRepr for ValueRef {
+    fn llrepr(&self, ccx: &CrateContext) -> ~str {
+        ccx.tn.val_to_str(*self)
+    }
+}
+
+

src/librustc/middle/trans/mod.rs

@@ -45,3 +45,4 @@ pub mod asm;
 pub mod type_;
 pub mod value;
 pub mod basic_block;
+pub mod llrepr;

src/librustc/middle/typeck/check/mod.rs

@@ -3099,22 +3099,6 @@ pub fn ty_param_bounds_and_ty_for_def(fcx: @mut FnCtxt,
           let typ = fcx.local_ty(sp, nid);
           return no_params(typ);
       }
-      ast::def_fn(_, ast::extern_fn) => {
-        // extern functions are just u8 pointers
-        return ty_param_bounds_and_ty {
-            generics: ty::Generics {
-                type_param_defs: @~[],
-                region_param: None
-            },
-            ty: ty::mk_ptr(
-                fcx.ccx.tcx,
-                ty::mt {
-                    ty: ty::mk_mach_uint(ast::ty_u8),
-                    mutbl: ast::m_imm
-                })
-        };
-      }
-
       ast::def_fn(id, _) | ast::def_static_method(id, _, _) |
       ast::def_static(id, _) | ast::def_variant(_, id) |
       ast::def_struct(id) => {

src/librustc/middle/typeck/collect.rs

@@ -1087,13 +1087,13 @@ pub fn ty_of_item(ccx: &CrateCtxt, it: &ast::item)
         tcx.tcache.insert(local_def(it.id), tpt);
         return tpt;
       }
-      ast::item_fn(ref decl, purity, _, ref generics, _) => {
+      ast::item_fn(ref decl, purity, abi, ref generics, _) => {
         assert!(rp.is_none());
         let ty_generics = ty_generics(ccx, None, generics, 0);
         let tofd = astconv::ty_of_bare_fn(ccx,
                                           &empty_rscope,
                                           purity,
-                                          AbiSet::Rust(),
+                                          abi,
                                           &generics.lifetimes,
                                           decl);
         let tpt = ty_param_bounds_and_ty {