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

pybind11 doesn't allow same name nested class defined in different scope #310

Open
jslee02 opened this issue Apr 18, 2020 · 3 comments
Open

Comments

@jslee02
Copy link
Member

jslee02 commented Apr 18, 2020

Given the following code, pybind11 generates binding code correctly and builds with no errors.

class Base
{
public:
    Base() = default;
    class Option
    {
    public:
        Option() = default;
    };
};

class Derived : public Base
{
public:
    Derived() = default;
    class Option : public Base::Option
    {
    public:
        Option() = default;
    };
};

// Binding code
{
    auto sm = m.def_submodule("common");
    auto attr = sm;
    ::pybind11::class_<chimera_test::common::Base >(attr, "Base")
        .def(::pybind11::init<>());
}
{
    auto sm = m.def_submodule("common");
    auto attr = sm.attr("Base");
    ::pybind11::class_<chimera_test::common::Base::Option >(attr, "Option")
        .def(::pybind11::init<>());
}
{
    auto sm = m.def_submodule("common");
    auto attr = sm;
    ::pybind11::class_<chimera_test::common::CommonDerivedA, chimera_test::common::Base >(attr, "CommonDerivedA")
        .def(::pybind11::init<>());
}
{
    auto sm = m.def_submodule("common");
    auto attr = sm.attr("CommonDerivedA");
    ::pybind11::class_<chimera_test::common::CommonDerivedA::Option, chimera_test::common::Base::Option >(attr, "Option")  // works if changed to "Option2"
        .def(::pybind11::init<>());
}

However, module loading fails saying:

ImportError: generic_type: cannot initialize type "Option": an object with that name is already defined

Boost.Python works fine.


Additional findings:

  1. This issue happens even Base and Derived is in different namespace.
  2. This issue only happens when Option is at the same level. For example, this issue disappear in the following case:
class Base
{
public:
    Base() = default;
    class Option
    {
    public:
        Option() = default;
    };
};

class Derived : public Base
{
public:
    Derived() = default;
    class NestedClass
    {
	public:
        class Option : public Base::Option
        {
        public:
            Option() = default;
        };
    };
};
jslee02 added a commit that referenced this issue Apr 18, 2020
@jslee02
Copy link
Member Author

jslee02 commented Apr 23, 2020

We can avoid this by specifying the name in the configuration YAML, but it would be great to provide options to automatically handle this issue.

Possible options:

  • O1: Prepend the parent class name to the nested class
    base_option = Base.BaseOption()
    derived_option = Derived.DerivedOption()
  • O2: Same as O1 but move the nested class to the parent scope
    base_option = BaseOption()
    derived_option = DerivedOption()
  • O3: Same as O2 but put underscore between class names
    base_option = Base_Option()
    derived_option = Derived_Option()

Additionally, we may want to detect the name confliction to catch this error before generating bindings and either not generating the binding or renaming it to a unique name like appending characters (e.g., multiple underscores or numbers).

@jslee02
Copy link
Member Author

jslee02 commented Apr 26, 2020

@psigen Would you have some suggestions on this? This is another name conflict issue that might need the scope-changing method or/and name mangling method.

@jslee02
Copy link
Member Author

jslee02 commented Apr 26, 2020

Just in case if this is a pybind11 issue: pybind/pybind11#2187

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

No branches or pull requests

1 participant