Skip to content

variant_caster cant handle c++ type nullptr_t #839

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

Closed
RedSkotina opened this issue May 8, 2017 · 10 comments
Closed

variant_caster cant handle c++ type nullptr_t #839

RedSkotina opened this issue May 8, 2017 · 10 comments

Comments

@RedSkotina
Copy link

I have compile errors while compile this code (gcc 7.1 and vs 2017 checked)

    py::module m("xpytest", "pybind testing plugin");
    m.def("load_variant", [](std::variant<int, std::nullptr_t> v) { 
        return std::visit(visitor(), v);
    });

gcc output:

F:/devel/mingw-w64/i686-7.1.0-posix-dwarf-rt_v5-rev0/mingw32/bin/g++.exe  -c  "E:/code/zog/ptest3/xpytest.cpp" -O0 -g -std=c++17 -D_hypot=hypot -D_DEBUG  -o ./Debug/up_xpytest.cpp.o -I. -I. -If:\lang\Python35-32\include
In file included from f:\lang\Python35-32\include/pybind11/attr.h:13:0,
                 from f:\lang\Python35-32\include/pybind11/pybind11.h:36,
                 from E:/code/zog/ptest3/xpytest.cpp:3:
f:\lang\Python35-32\include/pybind11/cast.h: In instantiation of 'typename pybind11::detail::make_caster<T>::cast_op_type<T> pybind11::detail::cast_op(pybind11::detail::make_caster<T>&) [with T = std::nullptr_t; typename pybind11::detail::make_caster<T>::cast_op_type<T> = std::nullptr_t&; pybind11::detail::make_caster<T> = pybind11::detail::type_caster<std::nullptr_t>; typename pybind11::detail::intrinsic_type<T>::type = std::nullptr_t]':
f:\lang\Python35-32\include/pybind11/stl.h:305:31:   required from 'bool pybind11::detail::variant_caster<V<Ts ...> >::load_alternative(pybind11::handle, bool, pybind11::detail::type_list<U, Us ...>) [with U = std::nullptr_t; Us = {}; V = std::variant; Ts = {int, std::nullptr_t}]'
f:\lang\Python35-32\include/pybind11/stl.h:308:32:   required from 'bool pybind11::detail::variant_caster<V<Ts ...> >::load_alternative(pybind11::handle, bool, pybind11::detail::type_list<U, Us ...>) [with U = int; Us = {std::nullptr_t}; V = std::variant; Ts = {int, std::nullptr_t}]'
f:\lang\Python35-32\include/pybind11/stl.h:314:32:   required from 'bool pybind11::detail::variant_caster<V<Ts ...> >::load(pybind11::handle, bool) [with V = std::variant; Ts = {int, std::nullptr_t}]'
f:\lang\Python35-32\include/pybind11/cast.h:1469:9:   required from 'bool pybind11::detail::argument_loader<Args>::load_impl_sequence(pybind11::detail::function_call&, std::index_sequence<Indices1 ...>) [with unsigned int ...Is = {0}; Args = {std::variant<int, std::nullptr_t>}; std::index_sequence<Indices1 ...> = std::integer_sequence<unsigned int, 0>]'
f:\lang\Python35-32\include/pybind11/cast.h:1449:34:   required from 'bool pybind11::detail::argument_loader<Args>::load_args(pybind11::detail::function_call&) [with Args = {std::variant<int, std::nullptr_t>}]'
f:\lang\Python35-32\include/pybind11/pybind11.h:132:17:   required from 'pybind11::cpp_function::initialize(Func&&, Return (*)(Args ...), const Extra& ...)::<lambda(pybind11::detail::function_call&)> [with Func = pybind11_init()::<lambda(std::variant<int, std::nullptr_t>)>; Return = const char*; Args = {std::variant<int, std::nullptr_t>}; Extra = {pybind11::name, pybind11::scope, pybind11::sibling}]'
f:\lang\Python35-32\include/pybind11/pybind11.h:128:22:   required from 'struct pybind11::cpp_function::initialize(Func&&, Return (*)(Args ...), const Extra& ...) [with Func = pybind11_init()::<lambda(std::variant<int, std::nullptr_t>)>; Return = const char*; Args = {std::variant<int, std::nullptr_t>}; Extra = {pybind11::name, pybind11::scope, pybind11::sibling}]::<lambda(struct pybind11::detail::function_call&)>'
f:\lang\Python35-32\include/pybind11/pybind11.h:128:19:   required from 'void pybind11::cpp_function::initialize(Func&&, Return (*)(Args ...), const Extra& ...) [with Func = pybind11_init()::<lambda(std::variant<int, std::nullptr_t>)>; Return = const char*; Args = {std::variant<int, std::nullptr_t>}; Extra = {pybind11::name, pybind11::scope, pybind11::sibling}]'
f:\lang\Python35-32\include/pybind11/pybind11.h:62:9:   required from 'pybind11::cpp_function::cpp_function(Func&&, const Extra& ...) [with Func = pybind11_init()::<lambda(std::variant<int, std::nullptr_t>)>; Extra = {pybind11::name, pybind11::scope, pybind11::sibling}; <template-parameter-1-3> = void]'
f:\lang\Python35-32\include/pybind11/pybind11.h:743:22:   required from 'pybind11::module& pybind11::module::def(const char*, Func&&, const Extra& ...) [with Func = pybind11_init()::<lambda(std::variant<int, std::nullptr_t>)>; Extra = {}]'

msvc 2017 output:

1>------ Build started: Project: xpytest, Configuration: Debug Win32 ------
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.25017\bin\HostX86\x86\CL.exe /c /I"f:\lang\Python35-32\include" /I"C:\Program Files (x86)\Visual Leak Detector\include" /Zi /nologo /W3 /WX- /diagnostics:classic /Od /Ob0 /Oy- /D WIN32 /D _WINDOWS /D PYBIND11_CPP17 /D _DEBUG /D "CMAKE_INTDIR=\"Debug\"" /D xpytest_EXPORTS /D _WINDLL /D _MBCS /Gm- /EHsc /RTC1 /MDd /GS /fp:precise /Zc:wchar_t /Zc:forScope /Zc:inline /GR /std:c++latest /Fo"xpytest.dir\Debug\\" /Fd"xpytest.dir\Debug\vc141.pdb" /Gd /TP /analyze- /errorReport:prompt E:\code\zog\pytest2\xpytest.cpp
1>xpytest.cpp
1>f:\lang\python35-32\include\pybind11\common.h(30): warning C4005: 'PYBIND11_CPP17': macro redefinition
1>f:\lang\python35-32\include\pybind11\common.h(30): note: command-line arguments:  see previous definition of 'PYBIND11_CPP17'
1>f:\lang\Python35-32\include\pybind11/pybind11.h(148): warning C4101: 'dd': unreferenced local variable
1>f:\lang\Python35-32\include\pybind11/pybind11.h(63): note: see reference to function template instantiation 'void pybind11::cpp_function::initialize<_Ty,R,pybind11::handle,>(Func &&,Return (__cdecl *)(pybind11::handle))' being compiled
1>        with
1>        [
1>            _Ty=pybind11::detail::keep_alive_impl::<lambda_fcb9b197e266298ec55444104890fed0>,
1>            R=void,
1>            Func=pybind11::detail::keep_alive_impl::<lambda_fcb9b197e266298ec55444104890fed0>,
1>            Return=void
1>        ]
1>f:\lang\Python35-32\include\pybind11/pybind11.h(1310): note: see reference to function template instantiation 'pybind11::cpp_function::cpp_function<pybind11::detail::keep_alive_impl::<lambda_fcb9b197e266298ec55444104890fed0>,,void>(Func &&)' being compiled
1>        with
1>        [
1>            Func=pybind11::detail::keep_alive_impl::<lambda_fcb9b197e266298ec55444104890fed0>
1>        ]
1>f:\lang\python35-32\include\pybind11\cast.h(490): error C2738: 'operator std::nullptr_t &': is ambiguous or is not a member of 'pybind11::detail::type_caster<std::nullptr_t,void>'
1>f:\lang\python35-32\include\pybind11\cast.h(656): note: see declaration of 'pybind11::detail::type_caster<std::nullptr_t,void>'
1>f:\lang\Python35-32\include\pybind11/stl.h(305): note: see reference to function template instantiation '_Ty &pybind11::detail::cast_op<U>(pybind11::detail::type_caster<std::nullptr_t,void> &)' being compiled
1>        with
1>        [
1>            _Ty=std::nullptr_t,
1>            U=std::nullptr_t
1>        ]
1>f:\lang\Python35-32\include\pybind11/stl.h(308): note: see reference to function template instantiation 'bool pybind11::detail::variant_caster<std::variant<int,std::nullptr_t>>::load_alternative<std::nullptr_t,>(pybind11::handle,bool,pybind11::detail::type_list<std::nullptr_t>)' being compiled
1>f:\lang\Python35-32\include\pybind11/stl.h(308): note: see reference to function template instantiation 'bool pybind11::detail::variant_caster<std::variant<int,std::nullptr_t>>::load_alternative<std::nullptr_t,>(pybind11::handle,bool,pybind11::detail::type_list<std::nullptr_t>)' being compiled
1>f:\lang\Python35-32\include\pybind11/stl.h(314): note: see reference to function template instantiation 'bool pybind11::detail::variant_caster<std::variant<int,std::nullptr_t>>::load_alternative<int,std::nullptr_t>(pybind11::handle,bool,pybind11::detail::type_list<int,std::nullptr_t>)' being compiled
1>f:\lang\Python35-32\include\pybind11/stl.h(314): note: see reference to function template instantiation 'bool pybind11::detail::variant_caster<std::variant<int,std::nullptr_t>>::load_alternative<int,std::nullptr_t>(pybind11::handle,bool,pybind11::detail::type_list<int,std::nullptr_t>)' being compiled
1>f:\lang\Python35-32\include\pybind11/stl.h(313): note: while compiling class template member function 'bool pybind11::detail::variant_caster<std::variant<int,std::nullptr_t>>::load(pybind11::handle,bool)'
1>f:\lang\python35-32\include\pybind11\cast.h(1469): note: see reference to function template instantiation 'bool pybind11::detail::variant_caster<std::variant<int,std::nullptr_t>>::load(pybind11::handle,bool)' being compiled
1>f:\lang\Python35-32\include\pybind11/stl.h(329): note: see reference to class template instantiation 'pybind11::detail::variant_caster<std::variant<int,std::nullptr_t>>' being compiled
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.25017\include\tuple(199): note: see reference to class template instantiation 'pybind11::detail::type_caster<std::variant<int,std::nullptr_t>,void>' being compiled
1>C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Tools\MSVC\14.10.25017\include\tuple(689): note: see reference to class template instantiation 'std::_Tuple_val<_This>' being compiled
1>        with
1>        [
1>            _This=pybind11::detail::type_caster<std::variant<int,std::nullptr_t>,void>
1>        ]
1>f:\lang\python35-32\include\pybind11\cast.h(1480): note: see reference to class template instantiation 'std::tuple<pybind11::detail::type_caster<std::variant<int,std::nullptr_t>,void>>' being compiled
1>f:\lang\Python35-32\include\pybind11/pybind11.h(124): note: see reference to class template instantiation 'pybind11::detail::argument_loader<std::variant<int,std::nullptr_t>>' being compiled
1>f:\lang\Python35-32\include\pybind11/pybind11.h(63): note: see reference to function template instantiation 'void pybind11::cpp_function::initialize<_Ty,R,std::variant<int,std::nullptr_t>,pybind11::name,pybind11::scope,pybind11::sibling>(Func &&,Return (__cdecl *)(std::variant<int,std::nullptr_t>),const pybind11::name &,const pybind11::scope &,const pybind11::sibling &)' being compiled
1>        with
1>        [
1>            _Ty=pybind11_init::<lambda_60b635692d52d37640d68fd3d08bd061>,
1>            R=const char *,
1>            Func=pybind11_init::<lambda_60b635692d52d37640d68fd3d08bd061>,
1>            Return=const char *
1>        ]
1>f:\lang\Python35-32\include\pybind11/pybind11.h(744): note: see reference to function template instantiation 'pybind11::cpp_function::cpp_function<_Ty,pybind11::name,pybind11::scope,pybind11::sibling,void>(Func &&,const pybind11::name &,const pybind11::scope &,const pybind11::sibling &)' being compiled
1>        with
1>        [
1>            _Ty=pybind11_init::<lambda_60b635692d52d37640d68fd3d08bd061>,
1>            Func=pybind11_init::<lambda_60b635692d52d37640d68fd3d08bd061>
1>        ]
1>E:\code\zog\pytest2\xpytest.cpp(26): note: see reference to function template instantiation 'pybind11::module &pybind11::module::def<pybind11_init::<lambda_60b635692d52d37640d68fd3d08bd061>,>(const char *,Func &&)' being compiled
1>        with
1>        [
1>            Func=pybind11_init::<lambda_60b635692d52d37640d68fd3d08bd061>
1>        ]
1>Done building project "xpytest.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 1 up-to-date, 0 skipped ==========
dean0x7d added a commit to dean0x7d/pybind11 that referenced this issue May 8, 2017
@RedSkotina
Copy link
Author

RedSkotina commented May 8, 2017

i only start learn template magic, but i fix this error by specializing template for nullptr_t via SFINAE(it is correct name for this trick?) :

    template <typename U, typename... Us>
	detail::enable_if_t<!std::is_null_pointer<U>::value, bool>
	 load_alternative(handle src, bool convert, type_list<U, Us...>) {
		auto caster = make_caster<U>();
                if (caster.load(src, convert)) {
                        value = cast_op<U>(caster);
                        return true;
               }
               return load_alternative(src, convert, type_list<Us...>{});
    }

    template <typename U, typename... Us>
	detail::enable_if_t<std::is_null_pointer<U>::value, bool>
	load_alternative(handle src, bool convert, type_list<U, Us...>) {
		if (src.is_none()) {
			value = nullptr;
			return true;
		}
		return load_alternative(src, convert, type_list<Us...>{});
	}

Test:

c++

struct visitor {
        const char *operator()(int) { return "int"; }
        const char *operator()(std::string) { return "std::string"; }
        const char *operator()(double) { return "double"; }
	const char *operator()(nullptr_t) { return "nullptr_t"; }
    };

....

    m.def("load_variant", [](std::variant<int, std::nullptr_t> v) { 
        return std::visit(visitor(), v);
    });

python

t1 = xpytest.load_variant(1)
print(t1)
t2 = xpytest.load_variant(None)
print(t2)

Test result:

int
nullptr_t

@dean0x7d
Copy link
Member

dean0x7d commented May 8, 2017

Should be fixed in #840. The underlying issue is in void_caster, not specific to variant. And your new example here answers my question from the PR: the None -> std::nullptr_t should be enabled to support this. However, that would make std::variant<double*, std::nullptr_t> an ambiguous conversion from None. (Might also need to look into supporting std::monostate.)

@RedSkotina
Copy link
Author

I do not understand completely, but do not the types differ? (double* and nullptr_t).
Can we use std::is_null_pointer and std::is_pointer ?

@dean0x7d
Copy link
Member

dean0x7d commented May 8, 2017

They are different types in C++ at compile time, but when converting at run time from Python's None type, both double* and std::nullptr_t are equally good.

@RedSkotina
Copy link
Author

Then can you give preference to std::nullptr_t because we can convert nullptr_t to double* later ?

@dean0x7d
Copy link
Member

dean0x7d commented May 8, 2017

I was looking into it, but it ends up being a bit complicated because we only have two priority levels and those are already divided into char* vs. everything else. Ideally, None -> std::nullptr_t should be the highest priority overload for None, but this would break the current behavior since char* would have the same priority as all other T* overloads. @jagerman What do you think? Any way around this?

@jagerman
Copy link
Member

jagerman commented May 8, 2017

I think I'm missing something--how does char * enter into this issue (or is that more of a hypothetical question)?

@jagerman
Copy link
Member

jagerman commented May 8, 2017

Perhaps we could solve this by treating char * as an intrinsic type rather than throwing away the pointer? I think in practice, char * is almost used as a C-style string, not as a pointer to a char. (And, in fact, we don't actually support binding something that is a pointer to a single char--pybind always treats it as a c-style string).

I did try severing this in make_caster back in #624, but that was indeed the wrong place. It might make more sense providing intrinsic_t specializations for char *, though.

The other solution—and actually, I think this makes logical sense even with the above—is to change the various type casters to make loading None an operation requiring convert, so that instead of:

if (src.is_none()) {
    value = nullptr;
    return true;
}

we do:

if (src.is_none()) {
    if (!convert) return false;
    value = nullptr;
    return true;
}

Then the nullptr_t caster gets first-pass priority for a None. That makes a lot of sense anyway; it would mean you could explicitly overload with a py::none argument and get the expected behaviour.

@dean0x7d
Copy link
Member

dean0x7d commented May 8, 2017

Yeah, that's what I had in mind for giving void_caster higher priority with regard to None (and the issue with char* which already has lower priority compared to everything else). But I now realize that this wouldn't actually solve anything with regard to variant<double*, std::nullptr_t> because overload resolution is only done for functions but not within variant_caster -- it just loads the first compatible alternative. variant_caster would need to grow a two-phase loader like the function dispatcher, but that feels like an overcomplication.

dean0x7d added a commit that referenced this issue May 9, 2017
* Fix compilation error with std::nullptr_t

* Enable conversion from None to std::nullptr_t and std::nullopt_t

Fixes #839.
@dean0x7d
Copy link
Member

I overestimated the complication to variant_caster::load(). It ended up being just 2 lines of code: #845. Together with #843, this should resolve everything here.

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

3 participants