call bytestring on String arguments in ccalls that require Ptr{Uint8} conversion

[stevengj]

The basic issue is that String arguments need to be passed through bytestring before being converted to Ptr{Uint8} by ccall. This should have no overhead for the common cases since bytestring(s::ByteString) = s should be inlined.

Among other things, this fixes #5673, and subsumes #5675.

[kmsquire]

Is there any reason this conversion should not be handled directly by ccall itself? It would save a lot of code changes?

[stevengj]

For non-ByteString types, this makes a copy. Potentially that copy is large if the string is large. So, there is an argument that it should not happen automatically.

[simonster]

I am not sure the copy is a huge concern (if a function might get passed a massive string, it's always possible to restrict the types it accepts), but I do not see a way to make the conversion happen automatically without a special case for Ptr{Uint8} in lower-ccall in julia-syntax.scm. ccalls are lowered so that convert gets called on their arguments, but there is no way to make convert(Ptr{Uint8}, s::String) work properly for all non-ByteStrings, since the copy needs to be rooted for the pointer to be usable, but only the pointer and not the copy is returned.

It looks like lower-ccall roots arguments before conversion, so my concerns in #5675 are unfounded.

[simonster]

Actually, it looks like cconvert(::Type{Ptr{Uint8}}, s::String) = bytestring(s) is commented out in base.jl with the comment # TODO: for some reason this causes a strange type inference problem, but either that type inference problem was actually a rooting issue or there is another issue here.

EDIT: Pretty sure this was not a rooting issue given #1601 and 10d71d9.

[JeffBezanson]

That comment is old enough that it's time to revisit this. All kinds of things have been fixed since then. Fortunately bytestrings are special-cased in ccall so they can be converted to pointers implicitly, without the rooting problem.