theseion / libgit2-pharo-bindings

libgit2 bindings for Pharo
http://theseion.github.io/libgit2-pharo-bindings
7 stars 29 forks source link

try to get optCoerceToNull working #19

Closed theseion closed 9 years ago

theseion commented 9 years ago

A while ago I sent this mail to the list (see https://pharo.fogbugz.com/f/cases/14479/Enable-nil-to-NULL-coercion-for-NBExternalObjectType):

Once again I’m struggling with NativeBoost. I’m trying to use optCoerceNilToNull with this function:

^self call: #(LGitReturnCodeEnum git_commit_create( LGitId * theCommitId, LGitRepository repo, String update_ref, LGitSignature * theAuthor, LGitSignature * theCommitter, String message_encoding, String theMessage, LGitTree theTree, int parent_count, LGitCommit * parentsPointer)) options: #( optCoerceNilToNull )

The last argument can be NULL, per documentation, (no parents). LGitCommit is a subclass of NBExternalObject and apparently NBExternalObjctType>>pushAsPointer: does not use the optCoerceNilToNull option (the super implementation in NBExternalType>>pushAsPointer: however does). The upshot of course is that the call fails with “an instance of LGitCommit expected”.

My questions:

  • The documentation says:

"#optCoerceNilToNull" "passing nil as a pointer-type argument, converts it to C NULL “

but apparently this doesn’t apply to every pointer type. Should the documentation be updated?

  • Why does NBExternlObjectType not use optCoerceNilToNull? I don’t want to use tricks to pass null if I don’t have to.
  • Related: there are instances where even non-pointer arguments are allowed to be NULL. Why does optCoerceNilToNull only work for pointer types?

Answer:

Yay! :) I have zero assembly skills… I’ve tried the following but then get an “undefined label done18” error:

if coerceNilToNull is set and the argument is indeed nil, you want to jump over all other operations on this argument (see below). (and if coerceNilToNull is set, and the argument is not nil, you should still verify the class of the oop, this is missing in your code!)

NBExternalObject>>pushAsPointer: gen "push a pointer to handle value" | asm proxy oop notNil done |

proxy := gen proxy.
asm := gen asm.

<<<<<<<<<<<<<< new

done := asm uniqueLabelName: 'done’.

->>>>>>>>>>>>>>> new

oop := gen reserveTemp.
loader emitLoad: gen to: oop.

<<<<<<<<<<<<<< new

"handle nils, if we care" gen optCoerceNilToNull ifTrue: [ notNil := asm uniqueLabelName: 'notNil'. proxy nilObject. asm cmp: asm EAX with: oop; jne: notNil; xor: asm EAX with: asm EAX; jmp: done; label: notNil ] ifFalse: [ "we can skip class verification, if loader loads receiver, since nothing to fear there" loader isReceiver ifFalse: [ self verifyClassOf: oop is: objectClass generator: gen. ] ].

->>>>>>>>>>>>>>> new

proxy fetchPointer: (self handleIvarIndex) ofObject: oop. "handle ivar" proxy varBytesFirstFieldOf: asm EAX. "handle value ptr"

       "done label here:"
       asm label: done.
theseion commented 9 years ago

Got it working. Proposed change in inbox.

tinchodias commented 9 years ago

Nice! El 01/03/2015 10:03, "Max Leske" notifications@github.com escribió:

Closed #19 https://github.com/theseion/LibGit/issues/19.

— Reply to this email directly or view it on GitHub https://github.com/theseion/LibGit/issues/19#event-243960496.