Skip to content

Conversation


with pytest.warns(
RuntimeWarning,
match="nanobind: attempted to access an uninitialized instance of type",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see any other test that checks for this string, but the place where the documentation discusses it is talking about a unique_ptr ownership transfer, which isn't this?

{ Py_tp_dealloc, (void *) nb_type_dealloc },
{ Py_tp_setattro, (void *) nb_type_setattro },
{ Py_tp_init, (void *) nb_type_init },
{ Py_tp_call, (void *) nb_type_call },
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be a vectorcall instead?

@wjakob
Copy link
Owner

wjakob commented Nov 24, 2025

I am not opposed to improving the error message, but this seems to complex of a change to achieve that goal. It also has a performance cost even in the non-erroneous case.

@virtuald
Copy link
Contributor Author

You're right, I was overthinking it. Now the extra overhead is just an single comparison.

@virtuald
Copy link
Contributor Author

Never mind, my local test environment was stale when I tested it. I'll take a closer look tonight.

@virtuald
Copy link
Contributor Author

virtuald commented Nov 25, 2025

Well, I got rid of the MRO stuff so it's a lot simpler now.

I feel like the other performance improvement would be to convert this tp_call to vectorcall, but I haven't been successful at that yet.

Edit: Ok, I got it using vectorcall when not using the limited API in Python 3.13.. and it looks like Py_tp_vectorcall isn't supported as a slot until 3.14. To fix it in 3.12/3.13 limited API I guess nanobind.nb_meta would need to add a __vectorcalloffset__ that could be set, and add Py_TPFLAGS_HAVE_VECTORCALL to it. But is that too much complexity in the name of performance?

@wjakob
Copy link
Owner

wjakob commented Nov 25, 2025

Instead of the current approach, how about a warning (PyErr_WarnEx) in the type caster when the user tries to pass an uninitialized nanobind object?

@wjakob wjakob force-pushed the master branch 4 times, most recently from 477f67b to 75f7046 Compare December 8, 2025 14:57
@wjakob
Copy link
Owner

wjakob commented Dec 16, 2025

Any news on this @virtuald ?

@virtuald
Copy link
Contributor Author

I need to go back and check, but I'm pretty sure there's already a warning in the type caster when it encounters an uninitialized nanobind object. I don't think that's good enough, better to make it an exception -- and also, in my development experience I rarely encounter warnings because I forget to enable whatever setting it is that shows them, so I'm not convinced that it would be useful for beginner developers consuming a nanobind wrapped library.

Regarding your performance concerns, it seems that if we could get it to only be installed when someone is inheriting from a class (instead of every nano-bound instance) that would be a good tradeoff? Alternatively, this could be an opt-in thing that is enabled by a class_<> argument.

I'll definitely revisit this in the future, but it might be a few weeks.

@wjakob wjakob force-pushed the master branch 3 times, most recently from 1235f9a to 4ba51fc Compare December 19, 2025 10:00
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.

[BUG]: nanobind fails with unhelpful error message when super.__init__ isn't called by overridden constructor

2 participants