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

enqueue_detach a non-void returning function #66

Merged

Conversation

jtd-formlabs
Copy link
Contributor

Moved the function requirement into a constexpr if statement and std::ignore any return value from a given function. added a super basic test.

@DeveloperPaul123
Copy link
Owner

Thanks for the PR.

Just curious, but what is the motivating use case for this change?

@jtd-formlabs
Copy link
Contributor Author

No problem!

I have an instance where I have a function that takes a fair amount of arguments and returns a value that I don't care about when running asynchronously. Setting up a lambda myself to do this is a bit tedious because then I have to manually handle all of the arguments, so if this library has this change I can just rely on perfect forwarding.

I had a few extra minutes to play around and figured I'd put up the PR for thoughts. No biggie if there are other reasons to restrict this to void returning functions only.

@DeveloperPaul123
Copy link
Owner

I see, thanks for the break down! I don't think this should be a problem, just want to check for any performance implications but I think it should be ok.

@jtd-formlabs
Copy link
Contributor Author

yeah I don't believe this will have any measurable performance impact on existing tests -- I just pushed a code bubble down into the function body but it's still compile time evaluated.

@DeveloperPaul123
Copy link
Owner

I think I'd be ok merging this. Could you address the formatting issues? The CI uses clang-format 18.1.3

@jtd-formlabs
Copy link
Contributor Author

yep I'll take care of that later today

@DeveloperPaul123 DeveloperPaul123 merged commit 98f7b95 into DeveloperPaul123:master Jul 2, 2024
11 checks passed
@DeveloperPaul123
Copy link
Owner

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants