JuliaLang / julia / 1503

78%

Build Type

push

buildkite

Committed by web-flow

Commit Message

Port `StringView` to Base (#60526)

This PR ports
[StringViews.jl](https://github.com/JuliaStrings/StringViews.jl) to
Base, as discussed in #60037. Closes #60037.

_Tests have been written by a chat-bot and reviewed by me_

## Decisions for reviewers before reviewing
* Can we use `@inbounds` with a `StringView` wrapping an unknown array
type? The code currently does, but I'm not against removing it. If we do
remove it, we may need to re-jigger a bunch of string related code,
since shared code paths of strings and string views would mean that
`String` performance could be degraded by removing inbounds annotations.

## Motivation
We generally want to avoid adding more types to Base when it can live in
a package, so this PR requires some justification:

### StringViews are the more fundamental string
`String` and `SubString` are an abstraction over an underlying byte
array, and almost all its operations are defined in terms of loading
bytes from that array. This abstraction of strings as "arrays in a
trench coat" is made explicit by string views.
Hence, `String` and `SubString` can be implemented in terms of string
views, but not the other way around. And it is better if Base contains
the foundations and packages provide implementations on top of those,
instead of the other way around.

One hint that the relationship is inverted is to look at the
implementation of `StringViews.jl`: It re-implements tonnes of internal
Base string functions, instead of calling into generic, foundational
methods in Base.
We also have comments like this in Base: `# duck-type s so that external
UTF-8 string packages like StringViews can hook in`, where the sensible
thing would be to _define_ getindex for UTF8-encoded strings in terms of
this method, instead of encouraging use of Base internals.

Due to the strong intersection of strings as an abstraction and string
views, this PR only has ~100 LOC in `base/strings/stringview.jl`,
whereas `src/` in `StringViews.jl` has ~800 LO... (continued)

Coverage Stats

75 of 93 new or added lines in 8 files covered. (80.65%)

132 existing lines in 11 files now uncovered.

65347 of 83954 relevant lines covered (77.84%)

24288390.47 hits per line