Priority overload

[henryiii]

Adds py::prepend() as an optional tag when defining a function, which causes an added function to be prepended to the beginning of the overload chain. This can be used to override a default constructor that was provided earlier, for example. When combined with #1122, this will allow the default string (or the old int) constructors for enums to be replaced by users.

No performance penalty, it just prepends instead of appends to the call chain. Provides a possible solution to, and docs for, so closes#2537, so targeting 2.6.0 (related to the original use case, actually :) )

Also adds get_pointer/set_pointer for py::capsule.

[wjakob]

Although I understand the motivation, I'm not in favor of this modification, which further increases the runtime cost and complexity of an already very complex dispatch routine. I think that we ought to make this part of pybind11 shorter and faster, not the other way.

[henryiii]

Does this increase the runtime? All it does is allow someone to attach a function at the beginning of the overload chain instead of the end, but the overload chain is the same length either way. It doesn't even increase the size of the struct, since there's one bit left over for alignment. So in the setup (runtime) phase, it adds one if statement.

This functionality seems useful in a more general case, where a user needs to pre-insert a call. But up to you.

[wjakob]

Aha -- I misread the patch in that case. I was assuming that the priority tag was handled in the method dispatch function, but I see now that it is in the part of the code that creates Python function overloads.

I've recently heard from users who found that Boost.Python dispatches methods faster than pybind11 (!?!?), which still has me a bit concerned that we are piling on too much stuff.

[wjakob]

One naming suggestion: how about calling it py::prepend or py::prepend_overload instead of priority_overload, which is IMO a bit more clear in terms of what happens (preventing question s like: "what does it mean to have two priority overloads?").

[henryiii]

I took the name from the original comment from @jagerman, I don't think he meant it to be final. I like py::prepend.

Unrelated: Any numbers or profile code for the boost Python comparison?

[jagerman]

prepend sounds fine to me. priority_overload was just a spur-of-the-moment name.

[wjakob]

Unrelated: Any numbers or profile code for the boost Python comparison?

Unfortunately we don't currently have a good way of tracking performance over time, which is a bit of a concern. It would be nice to have a CI to catch performance regressions similarly to how we catch bugs with Travis and friends. I played around with Unladen Swallow at some point but haven't had enough free clock cycles to turn it into something useful.

[jagerman]

I've pushed a commit to your branch that ought to get this working (a86451d). Basically the start of the chain was only being half-updated: it also needed to be updated inside the capsule rec_capsule that gets bound as the self argument for dispatcher because dispatcher starts from that pointer, not the local chain_start variable (which only gets used to regenerate the docstring).

[henryiii]

FYI, I'll polish off the tests as soon as I get a chance, hopefully around this weekend.

[henryiii]

@jagerman That was a very long weekend, but it should be ready for review finally!

[jagerman]

Two very minor comments, aside from those it looks good to me.

[henryiii]

Could this be useful in 2.3.0?

[henryiii]

Maybe we could also add a mention in the upgrade guide that __str__ on enums is now defined, so you need to use py::prepend() to override?

[rwgk]

Looks great! I have just some minor naming suggestions.

[YannickJadoul]

Should be rather painless, I think, yes :-)

[rwgk]

This PR was included in Google-global testing: no issues found.

[wjakob]

The implementation looks all good to me, but documentation could be improved IMO.

[wjakob]

(Really sorry that this has been stuck here in 2017. The flurry of recent activity has been wonderful -- thank you for all of your help! @henryiii@YannickJadoul@bstaletic@rwgk@EricCousineau-TRI and @ax3l)

[wjakob]

My vote would also go for set_pointer btw, it promotes clarity.

I feel less strongly about the following: For consistency, we could also have a get_pointer. We will need to continue to provide the operator-based getter, but it could be implemented in terms of this new get_pointer function.

[henryiii]

FWIW, I would be in favor of adding a .get_pointer() too, for consistency, and I think it's a bit more discoverable (ie, Xcode would show it in a completion list while conversions don't show up).