Add support for custom call policies

[oremanj]

This PR adds a new function annotation, nb::call_policy<Policy>(), which causes static methods of the user-provided Policy to be invoked before and after the bound C++ function. Unlike call_guard, the call policy methods operate on the Python object level and have access to the function arguments (precall can even modify them) and return value. This allows various esoteric user customizations that might be desirable in certain circumstances, with a minimal impact on the footprint of nanobind itself.

Motivating use case: a number of the types of interest in my application support a lightweight callback mechanism. The callback type we use is trivially copyable, so it can't manage ownership of a Python object directly. A custom call policy allows us to automatically take out a reference to the underlying Python callable when a new callback is subscribed, and drop the reference when it's unsubscribed. Allowing precall() to modify the argument list permits normalization (letting equal-but-distinct Python callables map to identical C++ callback objects, so that the C++ unsubscribe logic can find the callback that was originally subscribed).

While working on this I noticed that a function annotated with nb::keep_alive will leak a reference to its return value if the keep_alive can't be set up. I worked around the analogous issue for call policies, but didn't change the implementation of keep_alive in case this is a deliberate performance tradeoff.

[wjakob]

Hi @oremanj,

(Just got back from a 2-week vacation, hence the delay)

this looks like a neat feature and generalization of keep_alive. I am wondering if you would expect any run-time overhead on existing extensions? From a quick skim, I would say that everything should compile to basically the same code, but perhaps there was something I missed.

[oremanj]

I don’t expect any overhead in terms of binary size or runtime cost for extensions that don’t use the new feature. Fixing the bug I mentioned in the last paragraph (keep_alive leaks a reference to the return value if it throws an exception) will imply a minor binary size cost for additional exception-handling landing pads, but I didn’t do that in this PR.

[wjakob]

Hi @oremanj,

I did a more throrow review, thisall looks great, just one comment regarding a testcase.

One general thing which could be improved is to point out other applications. The precall/postcall example linked in the documentation (sequence keep_alive) seems potentially a bit specific/contrived.

In contrast, the following sounds pretty nifty, but I don't really understand it from this high level description:

Motivating use case: a number of the types of interest in my application support a lightweight callback mechanism. The callback type we use is trivially copyable, so it can't manage ownership of a Python object directly. A custom call policy allows us to automatically take out a reference to the underlying Python callable when a new callback is subscribed, and drop the reference when it's unsubscribed. Allowing precall() to modify the argument list permits normalization (letting equal-but-distinct Python callables map to identical C++ callback objects, so that the C++ unsubscribe logic can find the callback that was originally subscribed).

Do you think it would be possible to explain a baby version of such a feature? It would make it more clear that powerful extensions are feasible with this escape hatch.

[oremanj]

Do you think it would be possible to explain a baby version of such a feature? It would make it more clear that powerful extensions are feasible with this escape hatch.

Done. It's a bit too long to include in the documentation, so I added it as a unit test.

[oremanj]

I'm going to need some time to figure out why the 3.12+ Windows/macOS tests are failing. I can't reproduce the failure on my own macOS dev machine, nor on Linux with various sanitize/malloc-check options, nor can I figure out what's going on by staring at it. If you have any ideas, I'm all ears.

[wjakob]

On 3.12+, the CI uses stable ABI builds AFAIK. Can you repro the issue if you set NB_TEST_STABLE_ABI=ON?

[wjakob]

The new example looks fantastic, thank you very much for that!

[oremanj]

The stable ABI issue is a good thought, but unfortunately I'm not able to repro the CI failure on macOS even with -DNB_TEST_STABLE_ABI=ON -DNB_TEST_SHARED_BUILD=1. I tried both debug and release builds, both 3.12.0 and 3.13.0, and I verified that it is actually building stable-ABI extensions.

[oremanj]

I figured it out - the nb_type_data dictoffset / weaklistoffset fields would be uninitialized (rather than zero) on stable-ABI builds if the corresponding feature weren't used, which would then confuse inst_traverse's assumption that nb_dict_ptr returns null if there is no dict. Unrelated to this PR, just happened to be tickled by it. I rolled the fix into this PR instead of making a separate one, hope that's OK: 1d64365

[wjakob]

Thank you for finding this bug, I am surprised this had not come up before.

[wjakob]

GitHub says: "This branch cannot be rebased safely. Rebasing the commits of this branch on top of the base branch cannot be performed automatically as this would create a different result than a regular merge."

Can you rebase on top of master?

[oremanj]

Not sure what's going on there - the rebase succeeded without issue - but I've pushed the rebase version so hopefully it'll work now.

[oremanj]

The CI failures are unrelated to the code (network issue fetching from PyPI) but I can't rerun the workflow since I don't have write access to the repo.

[wjakob]

Thanks, I reran them.