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

runWorkerTask does not support implicit conversion of arguments #719

Merged
merged 1 commit into from Jul 16, 2014

Conversation

Projects
None yet
2 participants
@MartinNowak
Contributor

MartinNowak commented Jul 16, 2014

Currently runWorkerTask is defined as runWorkerTask(R, ARGS...)(R function(ARGS), ARGS...) which leads to a failure in template deduction when relying on implicit conversion of runWorkerTask arguments.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jul 16, 2014

Member

Is there a good standard idiom for this? Using a generic template parameter + an isFunction constraint? Using two nested templates with variadic parameters each?

Member

s-ludwig commented Jul 16, 2014

Is there a good standard idiom for this? Using a generic template parameter + an isFunction constraint? Using two nested templates with variadic parameters each?

@MartinNowak

This comment has been minimized.

Show comment
Hide comment
@MartinNowak

MartinNowak Jul 16, 2014

Contributor

Would be nice to have some sort of idiom collection.
You're right, it's actually harder than it looks, because the compiler tries to infer ARGS from the function pointer and from the actual arguments.

One good solution that isn't compatible with the existing function would be to use an alias.

import std.traits : ParameterTypeTuple;
void runWorkerTask(alias func)(auto ref ParameterTypeTuple!(typeof(func)) args)
{
}

I'll see, if I can come up with something that still takes a function pointer.

Contributor

MartinNowak commented Jul 16, 2014

Would be nice to have some sort of idiom collection.
You're right, it's actually harder than it looks, because the compiler tries to infer ARGS from the function pointer and from the actual arguments.

One good solution that isn't compatible with the existing function would be to use an alias.

import std.traits : ParameterTypeTuple;
void runWorkerTask(alias func)(auto ref ParameterTypeTuple!(typeof(func)) args)
{
}

I'll see, if I can come up with something that still takes a function pointer.

fix Issue 719 - runWorkerTask does not support implicit conversion of…
… arguments

- Deduce deduce function type and argument types
  separatly to support implicit convesions.

- Pass arguments by ref when possible
@MartinNowak

This comment has been minimized.

Show comment
Hide comment
@MartinNowak

MartinNowak Jul 16, 2014

Contributor

See pull request.
Not the cleanest solution though.

Contributor

MartinNowak commented Jul 16, 2014

See pull request.
Not the cleanest solution though.

@s-ludwig

This comment has been minimized.

Show comment
Hide comment
@s-ludwig

s-ludwig Jul 16, 2014

Member

Thanks, that looks like what I had in mind. The only thing is that I think that auto ref doesn't work properly for 2.064, so that would need to be removed until 2.065 can be considered the base line (for GDC and LDC). I can test and do that though.

Member

s-ludwig commented Jul 16, 2014

Thanks, that looks like what I had in mind. The only thing is that I think that auto ref doesn't work properly for 2.064, so that would need to be removed until 2.065 can be considered the base line (for GDC and LDC). I can test and do that though.

s-ludwig added a commit that referenced this pull request Jul 16, 2014

Merge pull request #719 from MartinNowak/fix719
runWorkerTask does not support implicit conversion of arguments

@s-ludwig s-ludwig merged commit 70ed129 into vibe-d:master Jul 16, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

@MartinNowak MartinNowak deleted the MartinNowak:fix719 branch Jul 16, 2014

@MartinNowak

This comment has been minimized.

Show comment
Hide comment
@MartinNowak

MartinNowak Jul 17, 2014

Contributor

The only thing is that I think that auto ref doesn't work properly for 2.064

Time for a build matrix to unittest all relevant compilers.
See #734.

Contributor

MartinNowak commented Jul 17, 2014

The only thing is that I think that auto ref doesn't work properly for 2.064

Time for a build matrix to unittest all relevant compilers.
See #734.

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