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

[BUG] py::type constructor from object does not throw when passed a non-type object #2700

Closed
phcerdan opened this issue Nov 27, 2020 · 3 comments · Fixed by #2701
Closed

Comments

@phcerdan
Copy link

phcerdan commented Nov 27, 2020

Issue description

py::type(object) constructor is returning an instance, which is unexpected.

This report was suggested by @YannickJadoul in gitter: https://gitter.im/pybind/Lobby?at=5fc11de4b12c2622f909e92f

Reproducible example code

// py::type::of<int> fails at compilation time.
// But it works for any wrapped type.
template<typename T>
using IsCPPType = typename std::enable_if<std::is_base_of<pybind11::detail::type_caster_generic, pybind11::detail::make_caster<T>>::value>::type;

template<typename T>
using IsNotCPPType = typename std::enable_if<!std::is_base_of<pybind11::detail::type_caster_generic, pybind11::detail::make_caster<T>>::value>::type;

template<typename TT>
void def_TValue(
        pybind11::class_<TT, std::unique_ptr<TT>> & py_class,
        IsCPPType<typename TT::Value> * = nullptr) {
    py_class.def_property_readonly_static("TValue",
            [](pybind11::object /* self */ ) {
            return pybind11::type::of<typename TT::Value>();
            });
}
template<typename TT>
void def_TValue(pybind11::class_<TT, std::unique_ptr<TT>> & py_class,
        IsNotCPPType<typename TT::Value> * = nullptr) {
    py_class.def_property_readonly_static("TValue",
            [](pybind11::object /* self */ ) {
            return pybind11::type( // This is returning an instance,not a type, it should throw. <---
            // return pybind11::type::of( // This is the correct way of returning the type.
                        pybind11::cast(typename TT::Value())
                    );
            });
}
template<typename T>
struct foo {
  using Value = T;
}
using FooInt = foo<int>;
using FooWrapped = foo<other_cpp_class>;
py_class_with_int = py::class_<FooInt , std::unique_ptr<FooInt>>(m, "FooInt");
def_TValue(py_class_with_int);
py_class_with_wrapped_class = py::class_<FooWrapped , std::unique_ptr<FooWrapped>>(m, "FooWrapped");
def_TValue(py_class_with_wrapped_class);
print(FooInt().TValue)
0 # unexpected? returns an instance!!
print(FooFloat().TValue)
0.0 # same

print(FooWrapped().TValue)
`<class  'other_cpp_class'>` # correct

After changing pybind11::type( for pybind11::type::of( the result is the correct <class 'int'>

@YannickJadoul
Copy link
Collaborator

Additional explanation: I think we decided in that PR (#2364) that py::type(obj) shouldn't work (since there's py::type::of(obj)), but I also thought we made it throw an exception if obj isn't actually a type (cfr. PyType_Check)? Smells like a bug.

@YannickJadoul
Copy link
Collaborator

YannickJadoul commented Nov 27, 2020

Minimal reproducer:

#include <pybind11/pybind11.h>

namespace py = pybind11;

PYBIND11_MODULE(example, m) {
	py::print(py::type(py::cast(int{})));
}

EDIT: Even minimal-er example

#include <pybind11/pybind11.h>

namespace py = pybind11;

PYBIND11_MODULE(example, m) {
        py::print(py::type(py::int_()));
}

@YannickJadoul
Copy link
Collaborator

Got it:

Name(object &&o) : Parent(std::move(o)) \
{ if (o && !check_(o)) throw PYBIND11_OBJECT_CHECK_FAILED(Name, o); }

Checking the value of the PyObject* in a moved-from py::object is not a great plan ... :-|

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants