Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Vigilans opened this issue Mar 21, 2018 · 3 comments · Fixed by #1334

Comments

@Vigilans
Copy link
Contributor

Vigilans commented Mar 21, 2018

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
Copy link
Contributor Author

Vigilans commented Mar 21, 2018

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
Copy link
Contributor Author

Vigilans commented Mar 21, 2018

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...

@Vigilans Vigilans changed the title py::enum_ does not behave correctly when the underlying type of enum class is char py::enum_'s __int__ and __hash__ do not behave correctly when the underlying type of enum class is char Mar 22, 2018
@jagerman
Copy link
Member

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.

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 a pull request may close this issue.

2 participants