Change type of extern fns from *u8 to extern "ABI" fn

cc #3678

Author: nikomatsakis

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/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;
@@ -1740,6 +1741,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/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

@@ -1067,13 +1067,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 {

src/test/auxiliary/extern-crosscrate-source.rs

@@ -19,7 +19,8 @@ pub mod rustrt {
     use std::libc;
 
     extern {
-        pub fn rust_dbg_call(cb: *u8, data: libc::uintptr_t)
+        pub fn rust_dbg_call(cb: extern "C" fn(libc::uintptr_t) -> libc::uintptr_t,
+                             data: libc::uintptr_t)
                              -> libc::uintptr_t;
     }
 }

src/test/compile-fail/extern-no-call.rs renamed to src/test/compile-fail/extern-cstack-lint.rs

@@ -8,10 +8,16 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// error-pattern:expected function but found `*u8`
 extern fn f() {
 }
 
-fn main() {
-    f();
+extern fn call1() {
+    f(); // OK from another extern fn!
 }
+
+fn call2() {
+    f(); //~ ERROR invoking non-Rust fn
+}
+
+
+fn main() {}

src/test/compile-fail/extern-wrong-value-type.rs

@@ -12,6 +12,7 @@ extern fn f() {
 }
 
 fn main() {
-    // extern functions are *u8 types
-    let _x: &fn() = f; //~ ERROR found `*u8`
+    // extern functions are extern "C" fn
+    let _x: extern "C" fn() = f; // OK
+    let _x: &fn() = f; //~ ERROR mismatched types
 }

src/test/run-pass/const-cast.rs

@@ -12,7 +12,7 @@ use std::libc;
 
 extern fn foo() {}
 
-static x: *u8 = foo;
+static x: extern "C" fn() = foo;
 static y: *libc::c_void = x as *libc::c_void;
 static a: &'static int = &10;
 static b: *int = a as *int;

src/test/run-pass/const-cross-crate-extern.rs

@@ -13,8 +13,11 @@
 
 extern mod cci_const;
 use cci_const::bar;
-static foo: *u8 = bar;
+use std::cast::transmute;
+static foo: extern "C" fn() = bar;
 
 pub fn main() {
-    assert_eq!(foo, cci_const::bar);
+    unsafe {
+        assert_eq!(foo, bar);
+    }
 }

src/test/run-pass/const-extern-function.rs

@@ -10,14 +10,16 @@
 
 extern fn foopy() {}
 
-static f: *u8 = foopy;
+static f: extern "C" fn() = foopy;
 static s: S = S { f: foopy };
 
 struct S {
-    f: *u8
+    f: extern "C" fn()
 }
 
 pub fn main() {
-    assert_eq!(foopy, f);
-    assert_eq!(f, s.f);
+    unsafe {
+        assert_eq!(foopy, f);
+        assert_eq!(f, s.f);
+    }
 }

src/test/run-pass/extern-call-deep.rs

@@ -14,7 +14,8 @@ mod rustrt {
     use std::libc;
 
     extern {
-        pub fn rust_dbg_call(cb: *u8, data: libc::uintptr_t)
+        pub fn rust_dbg_call(cb: extern "C" fn(libc::uintptr_t) -> libc::uintptr_t,
+                             data: libc::uintptr_t)
                              -> libc::uintptr_t;
     }
 }

src/test/run-pass/extern-call-deep2.rs

@@ -15,7 +15,8 @@ mod rustrt {
     use std::libc;
 
     extern {
-        pub fn rust_dbg_call(cb: *u8, data: libc::uintptr_t)
+        pub fn rust_dbg_call(cb: extern "C" fn(libc::uintptr_t) -> libc::uintptr_t,
+                             data: libc::uintptr_t)
                              -> libc::uintptr_t;
     }
 }