Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

add TypeDef to allow for runtime type checking #77

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

This is an idea I've been kicking around to let users more aggressively check at runtime that the right argument types are being passed. Specifically trying to handle the opaque pointer situation, though theoretically can be used with any type.

I decided to make it an alternate path such that users who don't want more overhead don't have to use it.

var myType = TypeDef(ref.refType(ref.types.void))

var func = ffi.ForeignFunction(somefunc, ref.types.void, [myType])

var instance = ref.alloc(ref.refType(ref.types.void))

// will throw
func(instance)

// will succeed
func(myType.cast(instance))

Just to clarify, this is more to handle the case where an opaque pointer is returned from one function and then used as an argument in another, so if your function returns myType and you try using that in something that accepts yetAnotherType it would fail

Member

TooTallNate commented Sep 28, 2012

So in a nutshell, the main problem is that these two types are treated essentially the same, even though we expect them to be different types:

var one = ref.refType('void')
var two = ref.refType('void')

It would be expected code like this should throw an error:

var one = ref.refType('void')
var two = ref.refType('void')

var S = Struct({
  a: one
})

var s = new S
s.a = ref.alloc(two)

However, I could also see cases where this would be a problem. What about the NULL pointer? This probably shouldn't throw...

s.a = ref.NULL

NULL might be one of the few edge cases, but we definitely need to think this through. Also, it definitely belongs at the ref "type" level, probably not node-ffi.

I know that the error reporting when invoking a foreign function improperly sucks, so we also should address that, maybe with a try/catch. I've opened a new issue: #78.

Actually it would only expect it to throw if they were TypeDefd, and right now each TypeDef returns a new instance for comparison. That's probably ideal for core data types, but less than ideal for Struct or ArrayType where it should probably cache its typedef on the parent type.

I'm closing this pull request and redoing on the ref level

@tjfontaine tjfontaine closed this Sep 28, 2012

@ghost

ghost commented Sep 28, 2012

I have implemented smoething like this, but the changes I made to ref were significant and it didn't seem like something that would be wanted for the official lib.

https://github.com/Benvie/ref/blob/master/lib/ref.js
https://github.com/Benvie/ref-struct/blob/master/lib/struct.js

Typing is extensively implement throughout and did end up in significant modifications to core functionality.

Member

TooTallNate commented Oct 2, 2012

@Benvie Additional typing should be implementable using the "type" interface as it is. @tjfontaine has done it generically in his ref-strict module. What did you need to modify in ref/ref-struct themselves?

@ghost

ghost commented Oct 2, 2012

x instanceof uint32

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment