pybind / pybind11

Seamless operability between C++11 and Python
https://pybind11.readthedocs.io/
Other
15.64k stars 2.1k forks source link

py::enum_'s `__int__` and `__hash__` do not behave correctly when the underlying type of enum class is char #1331

Closed Vigilans closed 3 years ago

Vigilans commented 6 years ago

Issue description

I am exposing an enum class to python, and this is my code:

// enum class to expose
enum class Player : char { 
    White = -1, None = 0, Black = 1 
};
// pybind11 wrapper
py::enum_<Player>(mod, "Player", "Gomoku player types")
    .value("white", Player::White)
    .value("none",  Player::None)
    .value("black", Player::Black)
    //.def(py::init<int>()) this line run into another error
    .def("__int__",   [](Player p) { return static_cast<int>(p); })
    .def("__float__", [](Player p) { return static_cast<double>(p); })
    .def("__hash__",  [](Player p) { return static_cast<int>(p); })
    .def("__neg__",   [](Player p) { return -p; })
    .def_static("calc_score", [](Player p, Player w) { return getFinalScore(p, w); });

As I was testing the module in python, I ran into such a strange issue:

>>> int(Player.black)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __int__ returned non-int (type str)

>>> hash(Player.black)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __hash__ method should return an integer

>>> float(Player.black)
1.0

In fact, similar issues have occurred when I pass the py::arithmetic() extra parameter to py::enum_.

Then I checked the inner member functions and discovered these:

>>> Player.black.__int__()
'\x01'
>>> Player.white.__int__()
'ÿ'
>>> Player.white.__hash__()
'ÿ'

So, why does a static_cast<int> resulted in a str in python?

Intuitively, I change the underlying type of enum class Player from char to int, and:

>>> Player.black.__int__()
1
>>> Player.white.__hash__()
-1
>>> int(Player.none)
0

Thus, it is the underlying type char of enum class player that causes all these bugs.

Vigilans commented 6 years ago

BTW, when defining the enum class:

// pybind11 wrapper
py::enum_<Player>(mod, "Player", "Gomoku player types")
    .value("white", Player::White)
    .value("none",  Player::None)
    .value("black", Player::Black)
    //.def(py::init<int>()) this line run into another error
    .def("__int__",   [](Player p) { return static_cast<int>(p); })
    .def("__float__", [](Player p) { return static_cast<double>(p); })
    .def("__hash__",  [](Player p) { return static_cast<int>(p); })
    .def("__neg__",   [](Player p) { return -p; })
    .def_static("calc_score", [](Player p, Player w) { return getFinalScore(p, w); });

the def(py::init<int>()) caused another bug in compiling:

C2440 Unable to convert “initializer list” to “Gomoku::Player”

And I don't have any idea on how to solve this (:з」∠) ……

Vigilans commented 6 years ago

Oh, I have solved the bug of py::init...I try to pass a lambda function to py::init instead, and everything works fine:

def(py::init([](int p) { return Player(p); }))

Maybe the form of creating an enum <Player(int)> is not so identical with a constructor, and causes some conflicts...

jagerman commented 6 years ago

This smells like a bug: the basic enum_ implementation defines __int__ and __hash__ which return the enum cast to the enum's underlying type.

(Your __int__ and __hash__ are just defining overloads which will never be called since the originals, with the same arguments, are going to be called first).

If you want to take a stab at a PR to fix it, I think changing class enum_ to add something like (untested): using ScalarInt = detail::conditional_t< detail::is_std_char_type<Scalar>::value, detail::conditional_t<std::is_signed<Scalar>::value, int, unsigned int>, Scalar>; and then casting to (ScalarInt) rather than (Scalar) in the provided __int__ and __hash__ implementations ought to fix it.