Broken conversion char* -> std::string

[jdemeyer]

Cython-0.28b1 miscompiles

from libcpp.string cimport string

cdef extern:
    void foo(string&)

foo("hello")

The call foo("hello") becomes

foo(((std::string &)"hello"));

In Cython-0.27.3, this was instead compiled to

  __pyx_t_1 = __pyx_convert_string_from_py_std__in_string(__pyx_n_b_hello); if (unlikely(PyErr_Occurred())) __PYX_ERR(1, 6, __pyx_L1_error)
  foo(__pyx_t_1);

[scoder]

Phew, nice one. While integrating the test case, I also noticed that this generates invalid code:

cdef string& func(string& s):
    return s

It creates a return value variable of type string& but the C++ compiler rejects that as uninitialised. What a beauty.

Do you see a way how to make this work? The memory management also looks fishy here. Is your example actually safe in C++? We can't just wrap the literal in new std::string(...), can we? (That could be done directly in BytesNode.generate_evaluation_code()).

[jdemeyer]

It creates a return value variable of type string& but the C++ compiler rejects that as uninitialised.

Cython's support for C++ references in general is very poor. So I wouldn't worry in particular about this specific example.

[scoder]

in general is very poor

Well, be fair - it's not that bad and it is getting better over time.

I found this explanation for what I suspected, which suggests that your code is incorrect. In C++, you cannot pass a string literal into a function that takes a non-const string&.

Now, that does not mean that Cython could not implement this, and you already showed that the previously generated code worked - despite being somewhat suboptimal. It even generated a warning in fact, although not visible by default: "Cannot pass Python object as C++ data structure reference (string &), will pass by copy.".

I've restored the old behaviour and improved it a bit, let's see who else complains now.

[jdemeyer]

OK, I did not think about the difference between const std::string& and std::string&. In fact, I probably only care about the former and should have declared the function as taking a const std::string&.

[tvalentyn]

With migration from Cython 0.27 to 0.28.2 I noticed a compilation error "Cannot assign type 'vector[int64]' to 'const vector[int64]'" in a codebase that
looks roughly like:

bool foo[L](const vector[L] &)
...
cdef vector[int64] bar
...
foo[int64](bar)

I traced the change to removal of lines 5545, 5546: 9a88d3c#diff-4548b0f91e1236cc95eb7cbd6a960533L5545.

@scoder , is the new behavior an intended one?
cc: @robertwb

[tvalentyn]

Opened #2211 for my last comment.