Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow CStruct pass-by-value in Nativecall #2648

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@scovit
Copy link

scovit commented Jan 26, 2019

This is the initial implementation of the pass-by-value
functionality for nativecall, it works on Parameters by recognizing
an 'is copy' trait, and as a new 'is copy' trait on Routine for the
return value. The 'copy' key is then added to the parameter/return
hash. The patch includes also tests of the basic functionality.

Allow CStruct pass-by-value in Nativecall
This is the initial implementation of the pass-by-value
functionality for nativecall, it works on Parameters by recognizing
an 'is copy' trait, and as a new 'is copy' trait on Routine for the
return value. The 'copy' key is then added to the parameter/return
hash. The patch includes also tests of the basic functionality.
@lizmat

This comment has been minimized.

Copy link
Contributor

lizmat commented Jan 26, 2019

Suggest we merge this after the 2019.01 release

@scovit

This comment has been minimized.

Copy link
Author

scovit commented Jan 26, 2019

Surely there is no hurry as I will be the only one using this functionality for quite some time. I would like to get feedback on the use of the "is copy" trait to indicate pass-by-value. The advantage of this choice is that it is not already used, and it is already implemented and optimized as part of the Parameter class.

The drawback is that, after analysing the Nativecall MoarVM source code, I start to suspect that 'copying' might be a desirable semantics Perl-side in general, while pass by value is very C-side and specific of CStruct? For this reason, another trait name might be preferable. Of course this is not a problem right now, since all of this is not implemented nor planned, but might be an useful discussion if there will ever be a plan to design a NativeCall 2.0 module.

@jonathanstowe

This comment has been minimized.

Copy link
Contributor

jonathanstowe commented Jan 26, 2019

Well with this I think I can lose the horrible wrapper in the GDBM binding 😌

@jonathanstowe

This comment has been minimized.

Copy link
Contributor

jonathanstowe commented Jan 26, 2019

And I think it makes the IBM MQ one which I have been planning for a while but didn't want write another ugly wrapper possible.

@@ -203,6 +205,10 @@ multi sub map_return_type($type) {
!! nqp::istype($type, Num) ?? Num !! $type;
}

my role CopyReturnStruct {
method ret-struct-copy(--> 1) { }

This comment has been minimized.

@b2gills

b2gills Jan 26, 2019

Contributor

CopyReturnStrct doesn't need a method as far as I can tell.

@AlexDaniel

This comment has been minimized.

Copy link
Member

AlexDaniel commented Jan 26, 2019

@scovit fwiw, maybe this change needs to be reflected on https://docs.perl6.org/language/nativecall, after we merge it.

Also please consider mentoring a GSoC project for improving nativecall: https://github.com/perl-gsoc-2019/ideas (submit it as a new idea).

@scovit scovit closed this Feb 8, 2019

@scovit scovit deleted the scovit:cstruct-by-value branch Feb 8, 2019

@scovit scovit restored the scovit:cstruct-by-value branch Feb 8, 2019

@scovit scovit deleted the scovit:cstruct-by-value branch Feb 8, 2019

@scovit

This comment has been minimized.

Copy link
Author

scovit commented Feb 11, 2019

If anybody is wondering why I closed this,
I am working on MoarVM and rakudo with those patches applied locally, so I am keeping them updated, and upon a rebase, github decided to close the pull request.

@jonathanstowe, all, if you want to try this out, or to use it, it is easy, you can find the patch here:

MoarVM: https://github.com/scovit/MoarVM/tree/cstruct-by-value
Rakudo: https://github.com/scovit/rakudo/tree/cstruct-by-value

MoarVM needs to be compiled with --has-libffi

Because of this, I wouldn't use this for code that needs to be publicly shared, just yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.