[MLton] C codegen and world* regressions

[Stephen Weeks]

> I guess I don't quite follow.  As of right now, while we try to be a
> little better about confusing ints and bools, I don't think we're
> getting any more guarantees.

I was referring to the fact that since we now generate the C header
and the SML imports from a single source, we are less likely to think
we have a bool on one side and a C int-as-bool (0 false, nonzero true)
on the other.  Hence we are less likely to miss converting.  I'm sure
that simply another pass through the whole runtime has helped too.
Nothing deep.

> Maybe you are suggesting that we have
> 
>     typedef bool Bool_t;

Yes.  I think we should define SML bool to be the same as C bool.

I guess I'm also OK with removing bool from the FFI.  We haven't been
using it much, and where we have been, many times we've been using it
incorrectly (i.e. not comparing to zero).  We should keep in mind that
it will break other people's code if we take it out.  However, I
wouldn't be surprised if others have been using it incorrectly as
well.

One problem if we remove bool from the FFI is that we force people to
write C wrappers to convert between bool and int.  Unless C guarantees
something about the calling convention of bools, I don't see how we
can safely pass/return C bools unless we know that's the type we're
dealing with.

So, it still seems best to me to allow bool in the FFI.  Whether we
automatically do the comparison with zero is a separate issue.

> I feel pretty uncomfortable with the prospect that FFI functions
> returning bool need to carefully return either 0 or 1. While I agree
> with Stephen that we can define it however we want, I think that the
> average C programmer expects to be able to return the result of a C
> test as a bool. So, however it's implemented, I think that comparing
> to zero is definitely a good idea.

I do feel the same discomfort.  However, as Matthew points out:

> there has always been this undefined behavior with respect to bool
> in the FFI, and nobody has ever complained, so I'm guessing that
> bool isn't used that much.

Yeah.  So, people have been living without an automatic comparison to
zero.  But I don't see the harm in having one.  I can't imagine speed
is relevant.

As to bool array/vector, we should not allow it in the FFI.

> To make things a little easier in the ML type-checker, the
> elaboration of FFI constructs is allowed to 'peek through' an opaque
> signature match to see the underlying base representation.  So, the
> default behavior will allow an ML 'int' to be used in FFI
> constructs, and it will be an 'int32_t' on the C side.  If you
> compile with '-default-type intinf', then the type checker will
> complain.  The problem is that if you compile with '-default-type
> int64', then the type checker won't complain, but MLton will send
> the value over as an int64_t, which might not be what the C-side is
> expecting.

This is bad.  At the very least we should document this and recommend
a programing style in which _import and _export declarations only use
types from the various C_* structures (which should be made available
in some MLB file).  Then, -default-type will have no effect.  I guess
that with some work we could even make using the C_* structures in the
FFI a requirement.

Along these lines, we should add C_Bool to c-types.sml and recommend
people use that for importing/exporting bool.

> > I looked at all the uses of Bool_t in basis-ffi.h to see if the
> > functions exported by the runtime were confusing bools and ints.  I
> > found a few that I'm not sure about.
> 
> Yeah, I added looking for uses of Bool_t to my todo.

According to your responses then, it looks like all of the following
are broken, because we don't convert C int-as-bool to SML bool.

  PosixFileSys_ST_is{Blk,Chr,Dir,FIFO,Link,Reg,Sock}
  Posix_ProcEnv_isatty
  Posix_Process_if{Exited,Signaled,Stopped}

For now I guess we should fix these by passing ints and manually
comparing to zero.