Defining enum with mutiple identical values should cause an error

[cysiek]

It's quite easy to make the following mistake:

enum SimpleEnum
    {
        ONE, TWO, THREE
    };

    PYBIND11_MODULE(MyPythonModule, module)
    {
        py::enum_<SimpleEnum>(module, "SimpleEnum")
        .value("ONE", SimpleEnum::ONE)          //NOTE: all value function calls are called with the same first parameter value
        .value("ONE", SimpleEnum::TWO)
        .value("ONE", SimpleEnum::THREE)
        .export_values();
}

This code compiles fine without any error/warning. When the module is loaded in Python, you can access only first value of SimpleEnum - "ONE". Would it be possible to make this code throw an error or at least warning at compile time (or eventually during loading python module) ?

Other information:
Pybind version: 2.2.3, Win 10 x64, Visual Studio 2015 x64, Python: Anaconda Python 3.6.5

[bstaletic]

    /// Add an enumeration entry
    enum_& value(char const* name, Type value, const char *doc = nullptr) {
        auto v = pybind11::cast(value, return_value_policy::copy);
        this->attr(name) = v;
        m_entries[pybind11::str(name)] = std::make_pair(v, doc);
        return *this;
    }

This is what pybind11::enum_::value() is doing. You would need to check if m_entries already contains a key that is equal to pybind11::str(name).

[jagerman]

Should be a simple enough check if you want to submit a PR (and turn your original post into a test case):

    pybind11::str pyname(name);
    if (m_entries.contains(pyname))
        throw value_error("...");
    // ... (current code)

[cysiek]

I've created a pull request - #1453
Please note that it's my first PR to open source project, so some "obvious" things might be wrong ;) Feel free to give me any feedback.

[cysiek]

I've fixed formatting issues, but still travis fails for XCode 9 C++ ( https://travis-ci.org/pybind/pybind11/builds/404210394 ). For me it looks a bit like configuration issue, @jagerman @bstaletic could you have a look at it?

[bstaletic]

PR merged. Closing.