wjakob / nanobind

nanobind: tiny and efficient C++/Python bindings
BSD 3-Clause "New" or "Revised" License
2.14k stars 161 forks source link

[BUG]: common.cpp does not compile in VS 2022 / 170 #606

Closed dtromb closed 3 weeks ago

dtromb commented 3 weeks ago

Problem description

Compilation of common.cpp fails with error

Lib\site-packages\nanobind\src\common.cpp(827,37): error C2039: 'ob_digit': is not a member of '_longobject'

Reproducible example code

#include <cstdint>
#include <nanobind/nanobind.h>
namespace nb = nanobind;
using namespace nb::literals;

NB_MODULE(MODULE_NAME, m) {
    m.doc()               = "empty test";
    m.attr("__version__") = VERSION_INFO;
}
dtromb commented 3 weeks ago

Python is 3.12, VS buildchain is MSBuild version 17.10.4+10fbfbf2e for .NET Framework, nanobind is 2.0.0

wjakob commented 3 weeks ago

Is it possible that you are not using CMake to build your project?

dtromb commented 3 weeks ago

I'm absolutely using CMake, the entire project setup is just the example project on the site, with the _add_module functions removed. FWIW it looks like python 3.11 works, whereas python 3.12 reliably fails.

wjakob commented 3 weeks ago

The way to compile nanobind modules via CMake is through the nanobind_add_module function. Unfortunately, you are on your own if you deviate from this path.

For what it's worth, the part of common.cpp that accesses ob_digit is guarded by a preprocessor check for PY_VERSION_HEX < 0x030c0000. So only Python 11 or lower even compile this part, so something about the explanation does not seem right to me.

dtromb commented 3 weeks ago

I am in fact using nanobind_add_module. Look at the example source on your website - it consists of a single function, _add_module, which adds two numbers together and gets mapped through nanobind. That's what I'm referring to, not nanobind_add_module, when I say "just the example project on the site, with the _add_module functions removed".

Just close away the ticket if you like, but your project certainly does not compile under Python 12 headers, for even simpler use cases - /which you could have easily verified/.

You might have also looked at the code citation in my report, and notice it is not in fact protected by an #if. You might also have done a search for this error and found https://github.com/python/steering-council/issues/79 (and 3.12 sequelae) and read it.

Meanwhile I'll switch back over to pybind11 and recommend others do the same until if/when stuff like this is sorted.

dtourolle commented 3 weeks ago

So @dtromb you are using an out of date version of nanobind. Line 827 of common in master does not have the code in your compiler error. The version 0.3.0 on the other hand has the line: https://github.com/wjakob/nanobind/blob/2bd6e48836a4bd1c9407cdb7277e80d327e1d736/src/common.cpp#L827 vs https://github.com/wjakob/nanobind/blob/master/src/common.cpp#L827

Try pulling a later version of nanobind or downgrading your python to <=3.11.

dtourolle commented 3 weeks ago

You might have also looked at the code citation in my report, and notice it is not in fact protected by an #if. You might also have done a search for this error and found python/steering-council#79 (and 3.12 sequelae) and read it.

I am in no way involved in nanobind, but before I write anything suggesting more work for an unpaid maintainer I think of this image. This kind of meditation can also clear my mind to help do things like check the git version of my code ;)