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

clbind: disambiguate apply_and_return on std::unique_ptr #1591

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

paulapatience
Copy link
Contributor

Previously, returning a std::unique_ptr from a clbound function would complain about ambiguous partial specializations of apply_and_return, the other one being include/clasp/clbind/apply.h:51.

Things I do not understand:

Why did std::move cause problems? Namely, the following:

../include/clasp/clbind/apply.h:118:98: error: non-const lvalue reference to type 'unique_ptr<...>' cannot bind to a temporary of type 'unique_ptr<...>'
  118 |     core::T_sp rv = translate::to_object<std::unique_ptr<RT>, translate::adopt_pointer>::convert(std::move(retval));
      |                                                                                                  ^~~~~~~~~~~~~~~~~

Also, what to do about the write_multiple_values call? In some other functions in apply.h, it is commented out and supplemented with the same printf I added — that is where I took the printf from. Is there really something we have to do there, or can we just remove it?

Finally, there might be other functions in apply.h that need to be disambiguated, likely the ones specialized on gctools::smart_ptr and gctools::multiple_values, but I do not understand enough of what is going on to be sure (I tried specializing them all, but that caused duplicate definitions, e.g., at lines 68 and 88).

Previously, returning a std::unique_ptr from a clbound function would
complain about ambiguous partial specializations of apply_and_return,
the other one being include/clasp/clbind/apply.h:51.
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.

1 participant