Missing checks for FFI declaration types

[simonmar]

A foreign import "wrapper" should have the form f -> IO (FunPtr f), and a foreign import "dynamic" should have the form FunPtr f -> f. GHC currently lacks these checks; the type argument to the FunPtr is ignored in both cases.

Ian Lynagh's example is below; we should be rejecting mkFun2 and mkCallback2.

module A where

import Foreign
import Foreign.C

data D = D

foreign import ccall "dynamic"
  mkFun1 :: FunPtr (CInt -> IO ()) -> (CInt -> IO ())

foreign import ccall "dynamic"
  mkFun2 :: FunPtr (D -> IO ()) -> (CInt -> IO ())

foreign import ccall "wrapper"
  mkCallBack1 :: IO () -> IO (FunPtr (IO ()))

foreign import ccall "wrapper"
  mkCallBack2 :: IO () -> IO (FunPtr D)

[simonpj@…]

commit 66b047ee8528860ef4babdbe38cf708a41e3be1e

Author: Simon Peyton Jones <simonpj@microsoft.com>
Date:   Tue Dec 13 13:36:34 2011 +0000

    Towards fixing Trac #5664
    
    This patch makes normaliseFFIType recurse ito the arguments of FunPtr

 compiler/typecheck/TcForeign.lhs |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

[pcapriotti]

Here is the relevant portion of the FFI specification in the Haskell 2010 report:

Dynamic import.
The type of a dynamic stub has to be of the form (FunPtr ft) -> ft, where ft
may be any foreign type.  As an example, consider

foreign import ccall "dynamic"  
  mkFun :: FunPtr (CInt -> IO ()) -> (CInt -> IO ())

The stub factory mkFun converts any pointer to a C function that gets an
integer value as its only argument and does not have a return value into a
corresponding Haskell function.

Dynamic wrapper.
The type of a wrapper stub has to be of the form ft -> IO (FunPtr ft), where ft
may be any foreign type.  As an example, consider

foreign import ccall "wrapper"  
  mkCallback :: IO () -> IO (FunPtr (IO ()))

The stub factory mkCallback turns any Haskell computation of type IO () into a
C function pointer that can be passed to C routines, which can call back into
the Haskell context by invoking the referenced function.

We are currently checking whether the signature of a dynamic (resp. wrapper) import has a FunPtr argument (resp. result), but not if the wrapped function type is correct.

[pcapriotti]

The attached patch performs the extra check using eqType.

[simonpj]

Thanks. Looks right to me, but it took me a while to be sure. Suggestions for comments to make it easier:

• Needs a Note [blah] with an example in isFFIDynTy.

• Comment on isFFIDynTy mentions Addr but code does not?

• The comment on isFFIDynTy should say what the two argument types are, linked to your example. It took me a while to understand what the curried_res_ty was doing, until I realised that arg_tys was all the args except the FunPtr.

• The comment says that the check is modulo newtypes, but splitTyConApp_maybe does not look through newtypes, so I thought it was wrong. But it's ok because we have previously called normaliseFfiType. Worth a comment to point this out, as is done in TcForeign line 366, for example.

Please push, and add a test. Thanks!

Simon

[p.capriotti@…]

commit 88d61ccd9450ed41b99136269a97b2c118462fa4

Author: Paolo Capriotti <p.capriotti@gmail.com>
Date:   Tue Apr 3 10:41:52 2012 +0100

    Improved checks for "dynamic" and "wrapper" foreign declarations (#5664)

 compiler/typecheck/TcForeign.lhs |    6 ++--
 compiler/typecheck/TcType.lhs    |   43 ++++++++++++++++++++++++++++---------
 2 files changed, 35 insertions(+), 14 deletions(-)

[pcapriotti]

Replying to simonpj:

* Comment on isFFIDynTy mentions Addr but code does not?

I think Addr is not accepted anymore, so that's just a case of out-of-date comment. I removed mentions of Addr in a separate commit.

Please push, and add a test. Thanks!

Done, thank you for the review!