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

Support perfect forwarding of arguments #116

Closed
VitaminCpp opened this issue Aug 13, 2019 · 3 comments
Closed

Support perfect forwarding of arguments #116

VitaminCpp opened this issue Aug 13, 2019 · 3 comments

Comments

@VitaminCpp
Copy link

If you define custom factories which take their arguments by reference instead by value, v8pp::class_::create_object() doesn't play well.
As a workaround you have to create the object manually:

ConsoleWrapper my_class_wrapper(ctx.isolate());
  my_class_wrapper
  .set("debug", &ConsoleJs::debug)
  .set("info", &ConsoleJs::info)
  .set("warn", &ConsoleJs::warn)
  .set("error", &ConsoleJs::error)
  .set("log", &ConsoleJs::log)
  .set_json()
  ;
  ctx.set("console", ConsoleWrapper::import_external(ctx.isolate(), v8pp::factory<ConsoleJs, v8pp::raw_ptr_traits>::create(ctx.isolate(), logger())));

So it would be nice if v8pp would support Perfect Forwarding.
This would also eliminate the use of custom factories for reference arguments?

Thanks!

pmed added a commit that referenced this issue Sep 1, 2019
@VitaminCpp
Copy link
Author

I think

static object_pointer_type create(v8::Isolate* isolate, Args... args)
also needs some attention? :)

@pmed
Copy link
Owner

pmed commented Sep 4, 2019

There is a problem with the factory::create() arguments forwarding. There may be a factory specialization with non-template create() declared, so such a line in class_::factory_create::call() default create implementation for class_::ctor<>() :

return detail::call_from_v8<Traits>(&factory<T, Traits>::template create<Args&&...>, args);

would fail on such a specialized factory (see https://github.com/pmed/v8pp/blob/master/test/test_class.cpp#L95 for example).

I currently have no idea how to deal with this. Newer c++17 branch has this problem, since that branch has no more factory concept at all.

@VitaminCpp
Copy link
Author

OK I see... Thanks anyway for your hard work! ;-) Maybe there is a solution for this, but currently I also have no clue...

@pmed pmed closed this as completed Aug 15, 2020
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

No branches or pull requests

2 participants