Protect internal String methods

[chastell]

This makes String#unsafe_byte_at and String#unsafe_byte_slice protected, as per this comment.

[RX14]

str.write part.to_slice[pointer, byte_count] should work...

[chastell]

Ah, thanks, I’m still learning all of this. 🙂

Unfortunately:

in src/file.cr:472: instantiating 'Slice(UInt8)#[](Pointer(UInt8), Int32)'

        str.write part.to_slice[part.to_unsafe + byte_start, byte_count]
                               ^

in src/slice.cr:195: no overload matches 'Int32#<=' with type Pointer(UInt8)
Overloads are:
 - Int32#<=(other : Int8)
 - Int32#<=(other : Int16)
 - Int32#<=(other : Int32)
 - Int32#<=(other : Int64)
 - Int32#<=(other : Int128)
 - Int32#<=(other : UInt8)
 - Int32#<=(other : UInt16)
 - Int32#<=(other : UInt32)
 - Int32#<=(other : UInt64)
 - Int32#<=(other : UInt128)
 - Int32#<=(other : Float32)
 - Int32#<=(other : Float64)
 - Comparable(T)#<=(other : T)

    unless 0 <= start <= @size
             ^

[chastell]

Actually, should this be just the below?

str.write part.to_slice[byte_start, byte_count]

[RX14]

This should be removed and replaced with just to_unsafe[index] where used.

[straight-shoota]

When all calls to this method get replaced by to_unsafe[index] it could as well be removed entirely.

[asterite]

I think we should really remove this method.

string.to_unsafe[index]
string.unsafe_byte_at_index(index)

the second is even longer!

Also, I'd like to come back with the idea of having general benchmarks of the whole standard library. Then we could see which places don't need to be "unsafe" and work just fine with safe code.

[chastell]

Addressed the comments, just need a double-check on whether

str.write part.unsafe_byte_slice(byte_start, byte_count)

and

str.write part.to_slice[byte_start, byte_count]

are equivalent (they do look so to me and the tests pass).

[asterite]

The latter is a bit slower because it has to do index bounds check. I don't know how it affects performance, though. That's why we need #5508 to know if this kind of refactors are OK to merge.

[chastell]

The latter is a bit slower because it has to do index bounds check.

Well I did initially go with

pointer = part.to_unsafe + byte_start
str.write Slice.new(pointer, byte_count, read_only: true)

but received a change request for String#to_slice… 😄

[straight-shoota]

@chastell This needs a rebase.

[Sija]

@chastell Do you still plan working on that PR?

[straight-shoota]

Closing stale PR.
#unsafe_byte_at has been deprecated in #10559.

#unsafe_byte_slice* is being kept for now. I think there's no need to make it nodoc or protected. Other APIs beside String itself might benefit from more performant string slicing as well.