Work in progres, please take a look and give early feedback if this is the way we want to structure…

lib/ffi/ffi.dart

Index: lib/ffi/ffi.dart
diff --git a/lib/ffi/ffi.dart b/lib/ffi/ffi.dart
index 7b2fcc2d5fa3aabf4381f35100a1cbff3dc38923..e63884d3d715a4920654d859fbc093dacc846070 100644
--- a/lib/ffi/ffi.dart
+++ b/lib/ffi/ffi.dart
@@ -9,6 +9,226 @@ library dart.ffi;
 
 import 'dart:_fletch_system' as fletch;
 
+class Foreign1 {

[ahe, 2015/06/30 13:16:50]

Can I assume that you're planning on deleting Foreign and that this class will
be renamed to Foreign?

If so, perhaps you could just do that. It would make the review easier to read,
IMO.

[ricow1, 2015/07/02 16:01:55]

Done.

+  int _value;
+  int get value => _value;

[kasperl, 2015/06/30 07:16:00]

I guess it will be nice when you can stop exposing this and start exposing
something that make more sense (address for ForeignMemory, etc.).

[ricow1, 2015/06/30 07:34:21]

yes

[ricow1, 2015/07/02 16:01:56]

Done.

+
+  Foreign1._(this._value);
+
+  static const int UNKNOWN = 0;
+
+  static const int LINUX   = 1;

[Anders Johnsen, 2015/06/30 06:30:17]

We should use enums going forward.

[ricow1, 2015/06/30 07:34:22]

Acknowledged.

+  static const int MACOS   = 2;
+
+  static const int IA32    = 1;
+  static const int X64     = 2;
+  static const int ARM     = 3;
+
+  static final Foreign1 NULL = new Foreign1();
+
+  // Helper for converting the argument to a machine word.
+  int _convert(argument) {
+    if (argument is Foreign) return argument._value;
+    if (argument is Port) return Foreign._convertPort(argument);
+    if (argument is int) return argument;
+    throw new ArgumentError();
+  }
+}
+
+class ForeignFunction extends Foreign1 {

[kasperl, 2015/06/30 07:16:00]

So to call a method that returns a C function pointer you have to do something
like this:
 
  var ptr = fun.pcall$0(new ForeignPointer());
  var moreFun = new ForeignFunction.fromAddress(ptr.value);

Should ForeignFunction be a ForeignPointer? That way you could write:

  var moreFun = fun.pcall$0(new ForeignFunction());

?

[ricow1, 2015/06/30 07:34:21]

Good point, will do that

[ricow1, 2015/07/02 16:01:55]

Done.

+  ForeignFunction.fromAddress(int value) : super._(value);
+
+  // Support for calling foreign functions that return
+  // machine words as immediate integer value.
+  int icall$0() => Foreign._icall$0(_value);
+  int icall$1(a0) => Foreign._icall$1(_value, _convert(a0));
+  int icall$2(a0, a1) => Foreign._icall$2(_value, _convert(a0), _convert(a1));
+  int icall$3(a0, a1, a2) {
+    return Foreign._icall$3(_value, _convert(a0), _convert(a1), _convert(a2));
+  }
+  int icall$4(a0, a1, a2, a3) {
+    return Foreign._icall$4(_value, _convert(a0), _convert(a1), _convert(a2),
+                            _convert(a3));
+  }
+  int icall$5(a0, a1, a2, a3, a4) {
+    return Foreign._icall$5(_value, _convert(a0), _convert(a1), _convert(a2),
+                            _convert(a3), _convert(a4));
+  }
+  int icall$6(a0, a1, a2, a3, a4, a5) {
+    return Foreign._icall$6(_value, _convert(a0), _convert(a1), _convert(a2),
+                            _convert(a3), _convert(a4), _convert(a5));
+  }
+
+  // Support for calling foreign functions with no return value.
+  void vcall$0() {
+    Foreign._vcall$0(_value);
+  }
+  void vcall$1(a0) {
+    Foreign._vcall$1(_value, _convert(a0));
+  }
+  void vcall$2(a0, a1) {
+    Foreign._vcall$2(_value, _convert(a0), _convert(a1));
+  }
+  void vcall$3(a0, a1, a2) {
+    Foreign._vcall$3(_value, _convert(a0), _convert(a1), _convert(a2));
+  }
+  void vcall$4(a0, a1, a2, a3) {
+    Foreign._vcall$4(_value, _convert(a0), _convert(a1), _convert(a2),
+                     _convert(a3));
+  }
+  void vcall$5(a0, a1, a2, a3, a4) {
+    Foreign._vcall$5(_value, _convert(a0), _convert(a1), _convert(a2),
+                     _convert(a3), _convert(a4));
+  }
+  void vcall$6(a0, a1, a2, a3, a4, a5) {
+    Foreign._vcall$6(_value, _convert(a0), _convert(a1), _convert(a2),
+                     _convert(a3), _convert(a4), _convert(a5));
+  }
+
+  // TODO(ricow): this is insanely specific and only used for rseek.
+  // Support for calling foreign functions that
+  //  - Returns a 64 bit integer value.
+  //  - Takes:
+  //    * a word,
+  //    * a 64 bit int
+  //    * a word
+  int Lcall$wLw(a0, a1, a2) {

[kasperl, 2015/06/30 07:16:00]

What's the purpose of this? Is it because you don't want to "split" on the word
size and have to different implementations of this?

I think adding support for getting 64-bit integers back (something ala
lcall$...) would make sense, but I guess I dislike encoding too much about the
arguments in the signature if we can avoid it.

[ricow1, 2015/06/30 07:34:21]

This is just an existing function, used for rseek. I discussed this a bit with
ajohnsen and sgjesse this morning. We could do something way more generic, where
you basically push the arguments for more specialized functions like this
(initialize with the number of arguments, something like (with better name):
FlexibleForeignFunction f = new FlexibleForeignFunction(numargs: 3);
f.addInt64(a)..addWord(b)..addInt8(c);
f.call();
Then we create the (platform specific) calling code for this.
We can't possibly support all permutations of functions like this.

(the above is just an example, maybe we should initialize it with the types
instead of the number of args)

+    return _Lcall$wLw(_value,
+                      _convert(a0),
+                      _convert(a1),
+                      _convert(a2));
+  }
+
+  // Support for calling foreign functions that return
+  // machine words -- typically pointers -- encapulated in
+  // the given foreign object arguments.
+  ForeignPointer pcall$0(ForeignPointer foreign) {
+    foreign._value = Foreign._icall$0(_value);
+    return foreign;
+  }
+
+  ForeignPointer pcall$1(ForeignPointer foreign, a0) {
+    foreign._value = Foreign._icall$1(_value, _convert(a0));
+    return foreign;
+  }
+
+  ForeignPointer pcall$2(ForeignPointer foreign, a0, a1) {
+    foreign._value = Foreign._icall$2(_value, _convert(a0), _convert(a1));
+    return foreign;
+  }
+}
+
+class ForeignLibrary extends Foreign1 {
+  ForeignLibrary.fromAddress(int address) : super._(address);
+
+  factory ForeignLibrary.fromName(String name) {
+    return new ForeignLibrary.fromAddress(_lookupLibrary(name));
+  }
+
+  static ForeignFunction lookupDefaultLibraries(String name) {

[kasperl, 2015/06/30 07:16:00]

Maybe it would be cleaner to have a ForeignLibrary.default so you would write:

  ForeignLibrary.default.lookupFunction(...)

instead of:

  ForeignLibrary.lookupDefaultLibraries(...)

?

[ricow1, 2015/06/30 07:34:21]

sgtm

[ricow1, 2015/07/02 16:01:55]

default is a reserved word - I called it standard now, but better alternatives
are welcome

+    return new ForeignFunction.fromAddress(Foreign.lookup(name).value);
+  }
+
+  ForeignFunction lookupFunction(String name) {
+    return new ForeignFunction.fromAddress(_lookupFunction(_value, name));
+  }
+
+  static String bundleLibraryName(String libraryName) =>

[kasperl, 2015/06/30 07:16:00]

Add a comment that explains what bundleLibraryName is.

[kasperl, 2015/06/30 07:16:00]

Why do you need the separate _bundlePath function? Can't bundleLibraryName just
be the native one?

[ricow1, 2015/06/30 07:34:21]

Acknowledged.

[ricow1, 2015/06/30 07:34:21]

Acknowledged.

[ricow1, 2015/07/02 16:01:55]

Done.

+      _bundlePath(libraryName);
+
+  void close() {
+    _closeLibrary(_value);
+  }
+
+  @fletch.native static int _lookupLibrary(String name) {
+    var error = fletch.nativeError;
+    throw (error != fletch.indexOutOfBounds) ? error : new ArgumentError();
+  }
+
+  @fletch.native static int _lookupFunction(int _value, String name) {
+    var error = fletch.nativeError;
+    throw (error != fletch.indexOutOfBounds) ? error : new ArgumentError();
+  }
+
+  @fletch.native static int _closeLibrary(int _value) {
+    var error = fletch.nativeError;
+    throw (error != fletch.indexOutOfBounds) ? error : new ArgumentError();
+  }
+
+  @fletch.native external static String _bundlePath(String library);
+}
+
+class ForeignPointer extends Foreign1 {
+  ForeignPointer.fromAddress(int address) : super._(address);
+  ForeignPointer() : super._(0);
+}
+
+class ForeignMemory extends ForeignPointer {

[Anders Johnsen, 2015/06/30 06:30:17]

Should ForeignMemory have a way of auto-deleting (weak pointer based)?

[ricow1, 2015/06/30 06:52:07]

Yes, as stated in the review description I will move up the allocation logic,
and do something about auto cleanup

[kasperl, 2015/06/30 07:16:00]

Could and should this implement some typed_data interface at some point
(ByteData?). Not sure.

[ricow1, 2015/06/30 07:34:21]

Yes I probably could, I will look into it

+  int _length;
+
+  ForeignMemory.fromForeignPointer(ForeignPointer pointer, int this._length) :

[kasperl, 2015/06/30 07:16:00]

No need to type the this._length parameter. The language guarantees that the
static analyzer automatically infers this.

[ricow1, 2015/06/30 07:34:21]

Acknowledged.

[ricow1, 2015/07/02 16:01:55]

Done.

+      super.fromAddress(pointer.value);
+
+  int getInt8(int offset)
+      => Foreign._getInt8(_computeAddress(offset, 1));
+  int getInt16(int offset)
+      => Foreign._getInt16(_computeAddress(offset, 2));
+  int getInt32(int offset)
+      => Foreign._getInt32(_computeAddress(offset, 4));
+  int getInt64(int offset)
+      => Foreign._getInt64(_computeAddress(offset, 8));
+
+  int setInt8(int offset, int value)
+      => Foreign._setInt8(_computeAddress(offset, 1), value);
+  int setInt16(int offset, int value)
+      => Foreign._setInt16(_computeAddress(offset, 2), value);
+  int setInt32(int offset, int value)
+      => Foreign._setInt32(_computeAddress(offset, 4), value);
+  int setInt64(int offset, int value)
+      => Foreign._setInt64(_computeAddress(offset, 8), value);
+
+  int getUint8(int offset)
+      => Foreign._getUint8(_computeAddress(offset, 1));
+  int getUint16(int offset)
+      => Foreign._getUint16(_computeAddress(offset, 2));
+  int getUint32(int offset)
+      => Foreign._getUint32(_computeAddress(offset, 4));
+  int getUint64(int offset)
+      => Foreign._getUint64(_computeAddress(offset, 8));
+
+  int setUint8(int offset, int value)
+      => Foreign._setUint8(_computeAddress(offset, 1), value);
+  int setUint16(int offset, int value)
+      => Foreign._setUint16(_computeAddress(offset, 2), value);
+  int setUint32(int offset, int value)
+      => Foreign._setUint32(_computeAddress(offset, 4), value);
+  int setUint64(int offset, int value)
+      => Foreign._setUint64(_computeAddress(offset, 8), value);
+
+  double getFloat32(int offset)
+      => Foreign._getFloat32(_computeAddress(offset, 4));
+  double getFloat64(int offset)
+      => Foreign._getFloat64(_computeAddress(offset, 8));
+
+  double setFloat32(int offset, double value)
+      => Foreign._setFloat32(_computeAddress(offset, 4), value);
+  double setFloat64(int offset, double value)
+      => Foreign._setFloat64(_computeAddress(offset, 8), value);
+
+  // Helper for checking bounds and computing derived
+  // address for memory address functionality.

[kasperl, 2015/06/30 07:16:00]

derived address -> derived addresses?

[ricow1, 2015/06/30 07:34:21]

Acknowledged.

[ricow1, 2015/07/02 16:01:55]

Done.

+  int _computeAddress(int offset, int n) {
+    if (offset < 0 || offset + n > _length) throw new IndexError(offset, this);
+    return _value + offset;
+  }
+
+  void free() {
+    if (_length > 0) Foreign._free(_value);

[Anders Johnsen, 2015/06/30 06:30:17]

Consider using -1 for length, as malloc(0) may return a valid pointer.

[ricow1, 2015/06/30 07:34:21]

Acknowledged.

+    _value = 0;
+    _length = 0;
+  }
+}
+
 class Foreign {
   static const int UNKNOWN = 0;