copy not implemented for String

[meggart]

I just realized this on Julia 1.1 as well as master:

julia> copy("I want to be copied")
ERROR: MethodError: no method matching copy(::String)
Closest candidates are:
  copy(::Expr) at expr.jl:36
  copy(::Core.CodeInfo) at expr.jl:64
  copy(::BitSet) at bitset.jl:46
  ...
Stacktrace:
 [1] top-level scope at REPL[3]:1

I was wondering if this is desired behavior or would if it would be ok to define copy(s::String)=s since Strings are immutable.

[JeffBezanson]

I've tried to avoid defining copy for immutable objects. How did this arise? My view is that code calling copy without knowing whether the argument is mutable can't really be generic.

[meggart]

I have a variable a that is either a String or a Vector{UInt8} or a HTTP reply body, which should be converted into a string without invalidating the input. Since String constructors are defined for all possible inputs I just coded a2 = String(copy(a)), and just got a bit surprised by the missing copy method.

There is some precedence for defining copy on immutables (https://github.com/JuliaLang/julia/blob/master/base/number.jl#L88). Of course having this functionality is not critical for me, my current workaround is to define

copied_string(x) = String(copy(x))
copied_string(x::String) = x

which works fine but just feels a bit too verbose. Maybe there already exists a more convenient way to achieve this?

[StefanKarpinski]

I still worry that this is overly pedantic. I guess the argument is something along these lines... If you're copying something, then it must be because you're going to mutate it—otherwise why copy it? But if some of the values are immutable, then that's going to fail anyway, so what kind of code needs this?

[KristofferC]

String is a bit special in that it mutates the input argument for some types.

[meggart]

Indeed, if String would copy by default and there was an additional String! as was suggested here #24388 (comment) this would solve my use case as well.

[stevengj]

We already have lots of copy methods for immutable objects that return an immutable, so that cat is out of the bag:

julia> copy(3)
3

julia> copy(1:10)
1:10

On the other hand, Base.copymutable works for String:

julia> Base.copymutable("Hello")
5-element Array{Char,1}:
 'H': ASCII/Unicode U+0048 (category Lu: Letter, uppercase)
 'e': ASCII/Unicode U+0065 (category Ll: Letter, lowercase)
 'l': ASCII/Unicode U+006C (category Ll: Letter, lowercase)
 'l': ASCII/Unicode U+006C (category Ll: Letter, lowercase)
 'o': ASCII/Unicode U+006F (category Ll: Letter, lowercase)

(Maybe in Julia 2.0 we can make copy equivalent to copymutable, but in the present state it's not crazy to define copy(::String).)

[stevengj]

For example, copy(::Number) was preserved by @JeffBezanson in #15675 even while other immutable copy methods were deleted.

[KristofferC]

so that cat is out of the bag:

I don't get what point you are trying to make. That there should be a generic Base.copy(x) = isbits(x) ? x : throw(MethodError...)?

For example, copy(::Number) was preserved by @JeffBezanson in #15675

Yes:

To minimize breakage, I'm leaving the definitions for Range and Number for now,

[stevengj]

To minimize breakage

Yes, i.e. generic code was relying on those methods, so they can't have been that useless.

[KristofferC]

Yes, they could have been? People do useless stuff all the time. Just look at one of the logs from PkgEval for an upcoming release.

[JeffBezanson]

In this case it was a Vector that needed to be copied, not a String, so I'm inclined to close this.

I'm not a fan of the default copymutable method that calls collect. That's not really a "copy" since an array might be a very different structure than what you started with.

[stevengj]

That's not really a "copy" since an array might be a very different structure than what you started with.

Which is exactly what you want if you want a mutable copy … recall that copymutable was introduced in the first place to fix methods like sort(x) = sort!(copymutable(x)) for immutable collections and iterators.

[meggart]

Which is exactly what you want if you want a mutable copy … recall that copymutable was introduced in the first place to fix methods like sort(x) = sort!(copymutable(x)) for immutable collections and iterators.

As stated above, in the use case I had I needed to copy an array to make sure it is not invalidated when passed to a String constructor. I agree with the reasoning that it should not be necessary to copy immutables and that the actual issue is the fact that the String constructor invalidates its input. The String docs say:

If v is Vector{UInt8} it will be truncated to zero length and
future modification of v cannot affect the contents of the resulting string.
To avoid truncation use String(copy(v)).

So it is explicitly suggested to call copy which causes the MethodError if v can be both a Vector{UInt8} or a String.

I agree we might close this issue or rename it to something Make String constructor copy by default or refer to an already existing issue that discusses this.

[stevengj]

String(copy(v)) is a good use-case for adding copy(s::String) = s, in my opinion, and helps to illustrate in general why copy methods for immutables can be useful.

[JeffBezanson]

The String constructor is covered by #32528 .

I don't think methods should be added for the convenience of not writing a branch when a function doesn't apply to one case.