cppyy icon indicating copy to clipboard operation
cppyy copied to clipboard

No implicit const conversion of iterators when used with emplace

Open agijsberts opened this issue 2 years ago • 8 comments

Hello,

While writing a custom vector-like container I noticed strange behavior when passing non-const iterators to the emplace method of my class. I can replicate the same behavior also with std::list, as demonstrated in the following minimal example:

import cppyy

v = cppyy.gbl.std.vector[int]([1, 2, 3])
v.emplace(v.begin(), 2)

l = cppyy.gbl.std.list[int]([1, 2, 3])
l.insert(l.begin(), 2)
l.emplace(l.begin(), 2)

This fails with the error trace:

Traceback (most recent call last):
  File "<stdin>", line 8, in <module>
TypeError: Template method resolution failed:
  std::_List_iterator<int> std::list<int>::emplace(std::list<int>::const_iterator __position, int&& __args) =>
    TypeError: could not convert argument 1
  Failed to instantiate "emplace(std::_List_iterator<int>*,int)"
  std::_List_iterator<int> std::list<int>::emplace(std::list<int>::const_iterator __position, int&& __args) =>
    TypeError: could not convert argument 1

I can work around the error by replacing begin() with cbegin(), but if I am not mistaken the iterator should implicitly be converted to const_iterator; the equivalent C++ code works without problems. I am not familiar with Cppyy's codebase, but the error leads me to believe it might have something to do with the fact that emplace is a template function, which would also explain why insert works without problem despite also using const_iterator as first argument. This does not explain, however, why vector.emplace works without problems...

Tested with cppyy 3.0.0 (git head), cppyy_backend 1.14.11, cppyy_cling 6.28.0, CPyCppyy 1.12.13.

agijsberts avatar Apr 07 '23 15:04 agijsberts

it might have something to do with the fact that emplace is a template function

Probably, as the implicit conversion works fine per se:

>>> import cppyy
>>> v = cppyy.gbl.std.vector[int]
>>> v.const_iterator(v.iterator())
<cppyy.gbl.std.__wrap_iter<const int*> object at 0x12ee849a0>
>>>

There's a bit of a problem in allowing implicit conversion with templated functions as e.g. type promotions are supposed to be disallowed. IIRC, implicit conversion is therefore disabled altogether for templates.

wlav avatar Apr 10 '23 16:04 wlav

It's actually a bit smarter: implicit conversion is allowed for already instantiated templates, but not for templates currently being instantiated. If I enable allowing implicit conversion in all cases, it runs fine.

I'll have to think a bit on how to refine that.

wlav avatar Apr 10 '23 17:04 wlav

Out of curiosity, any reason why it works currently for std::vector and not for std::list? The only mention I found of vector is the special treatment of stl sequence types at https://github.com/wlav/cppyy/blob/master/python/cppyy/_cpython_cppyy.py#L58, but this seemed unrelated and identical for both container types anyways.

agijsberts avatar Apr 10 '23 18:04 agijsberts

Sorry, what works for std::vector but not for std::list? In the above code, the issue for both is the same: not allowing implicit conversions for templated methods. If I enable that, the code as written above, both vector and list works.

wlav avatar Apr 10 '23 20:04 wlav

And yes, that piece of code is unrelated. It exists so that you can write:

>>> import cppyy
>>> cppyy.gbl.std.vector([1,2,3])
<cppyy.gbl.std.vector<int> object at 0x13182b740>
>>>

Note the lack of template argument.

wlav avatar Apr 10 '23 20:04 wlav

Strange, to be sure I just triple checked and I only get an error on the list operation

l.emplace(l.begin(), 2)

and not on the equivalent vector operation

v.emplace(l.begin(), 2)

The code is exactly the one from my initial post. So if I remove the last line, it runs without problem.

agijsberts avatar Apr 10 '23 20:04 agijsberts

I was running on Mac. There both emplace calls fail, and both insert calls succeed, b/c of the reasons described above. But I've found it now, there's this old workaround in the converters that is specific to STL iterators:

        // CLING WORKAROUND -- special case for STL iterators
            if (realType.rfind("__gnu_cxx::__normal_iterator", 0) /* vector */ == 0
#ifdef __APPLE__
                || realType.rfind("__wrap_iter", 0) == 0
#endif
                // TODO: Windows?
               ) {
                static STLIteratorConverter c;
                result = &c;
            } else
       // -- CLING WORKAROUND

This converter basically disables type checking on iterators that match, and per the commit message, it was b/c the cleaned type from Cling wasn't correct if it included *const&. For std::list, however, the type of the iterator for list is std::_List_const_iterator, so it doesn't use this converter.

wlav avatar Apr 10 '23 21:04 wlav

Thanks a lot for getting to the bottom of this. And sorry, I should've specified that I was running this on Linux.

It's actually a bit smarter: implicit conversion is allowed for already instantiated templates, but not for templates currently being instantiated. If I enable allowing implicit conversion in all cases, it runs fine.

I'll have to think a bit on how to refine that.

I'll leave the issue open for now, as you suggest that generally allowing implicit conversions may actually fix this if it doesn't have other unwanted side effects.

agijsberts avatar Apr 10 '23 21:04 agijsberts