From ffbf02ac2364b63c503b1946b71fccdf930b5314 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 13 May 2020 22:51:53 +0100 Subject: [PATCH 1/7] make taking views of string indexing give substrings --- base/strings/substring.jl | 5 +++++ test/strings/basic.jl | 16 ++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/base/strings/substring.jl b/base/strings/substring.jl index d98dab5ae790b..16effecf1cf8a 100644 --- a/base/strings/substring.jl +++ b/base/strings/substring.jl @@ -47,6 +47,11 @@ end SubString(s::AbstractString) = SubString(s, 1, lastindex(s)) SubString{T}(s::T) where {T<:AbstractString} = SubString{T}(s, 1, lastindex(s)) + +@propagate_inbounds view(s::AbstractString, r) = SubString(s, r) +@propagate_inbounds maybeview(s::AbstractString, r::UnitRange{<:Integer}) = view(s, r) +@propagate_inbounds maybeview(s::AbstractString, args...) = getindex(s, args...) + convert(::Type{SubString{S}}, s::AbstractString) where {S<:AbstractString} = SubString(convert(S, s)) convert(::Type{T}, s::T) where {T<:SubString} = s diff --git a/test/strings/basic.jl b/test/strings/basic.jl index 8dc7647a7219a..978b10f26e4b6 100644 --- a/test/strings/basic.jl +++ b/test/strings/basic.jl @@ -166,6 +166,22 @@ end @test endswith(z)(z) end +@testset "SubStrings and Views" begin + x = "abcdefg" + @test SubString(x, 2:4) == "bcd" + @test view(x, 2:4) == "bcd" + @test view(x, 2:4) isa SubString + @test (@view x[4:end]) == "defg" + @test (@view x[4:end]) isa SubString + + # We don't (at present) make non-contiguous SubStrings with views + @test_throws MethodError (@view x[[1,3,5]]) == "ace" + @test (@views (x[[1,3,5]])) isa String + + @test (@views (x[1], x[1:2], x[[1,4]])) isa Tuple{Char, SubString, String} +end + + @testset "filter specialization on String issue #32460" begin @test filter(x -> x ∉ ['작', 'Ï', 'z', 'ξ'], GenericString("J'étais n작작é pour plaiÏre à toute âξme un peu fière")) == From e10c46d59acb8913b0a570aedcd91a7e54e523b3 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 13 May 2020 23:31:24 +0100 Subject: [PATCH 2/7] add news --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index c3173351ec9c5..32f271c6a7cbc 100644 --- a/NEWS.md +++ b/NEWS.md @@ -43,6 +43,7 @@ Standard library changes ------------------------ * The `nextprod` function now accepts tuples and other array types for its first argument ([#35791]). * The function `isapprox(x,y)` now accepts the `norm` keyword argument also for numeric (i.e., non-array) arguments `x` and `y` ([#35883]). +* `view`, `@view`, and `@views` now work on `AbstractString`s, returning a `SubString` when appropriate ([#35879]). #### LinearAlgebra From eba2f11c71e47d9e318e2b602dd5df0019c08bc8 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Wed, 13 May 2020 23:31:55 +0100 Subject: [PATCH 3/7] Remove excess whitespace --- base/strings/substring.jl | 1 - 1 file changed, 1 deletion(-) diff --git a/base/strings/substring.jl b/base/strings/substring.jl index 16effecf1cf8a..d8ead5faec8c8 100644 --- a/base/strings/substring.jl +++ b/base/strings/substring.jl @@ -47,7 +47,6 @@ end SubString(s::AbstractString) = SubString(s, 1, lastindex(s)) SubString{T}(s::T) where {T<:AbstractString} = SubString{T}(s, 1, lastindex(s)) - @propagate_inbounds view(s::AbstractString, r) = SubString(s, r) @propagate_inbounds maybeview(s::AbstractString, r::UnitRange{<:Integer}) = view(s, r) @propagate_inbounds maybeview(s::AbstractString, args...) = getindex(s, args...) From b3e1837d8a7f815fab47b7fac8b8a6b67fd56e41 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Sat, 16 May 2020 17:21:51 +0100 Subject: [PATCH 4/7] handle single character string views correctly --- base/strings/substring.jl | 2 +- test/strings/basic.jl | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/base/strings/substring.jl b/base/strings/substring.jl index d8ead5faec8c8..86f5a3388ced0 100644 --- a/base/strings/substring.jl +++ b/base/strings/substring.jl @@ -47,7 +47,7 @@ end SubString(s::AbstractString) = SubString(s, 1, lastindex(s)) SubString{T}(s::T) where {T<:AbstractString} = SubString{T}(s, 1, lastindex(s)) -@propagate_inbounds view(s::AbstractString, r) = SubString(s, r) +@propagate_inbounds view(s::AbstractString, r::UnitRange{<:Integer}) = SubString(s, r) @propagate_inbounds maybeview(s::AbstractString, r::UnitRange{<:Integer}) = view(s, r) @propagate_inbounds maybeview(s::AbstractString, args...) = getindex(s, args...) diff --git a/test/strings/basic.jl b/test/strings/basic.jl index 978b10f26e4b6..43048cde9dcf4 100644 --- a/test/strings/basic.jl +++ b/test/strings/basic.jl @@ -175,10 +175,14 @@ end @test (@view x[4:end]) isa SubString # We don't (at present) make non-contiguous SubStrings with views - @test_throws MethodError (@view x[[1,3,5]]) == "ace" + @test_throws MethodError (@view x[[1,3,5]]) @test (@views (x[[1,3,5]])) isa String - @test (@views (x[1], x[1:2], x[[1,4]])) isa Tuple{Char, SubString, String} + # We don't (at present) make single character SubStrings with views + @test_throws MethodError (@view x[3]) + @test (@views (x[3])) isa Char + + @test (@views (x[3], x[1:2], x[[1,4]])) isa Tuple{Char, SubString, String} end From eb214bde080a6023b42a2062fd02be8a71a99261 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 19 May 2020 20:35:41 +0100 Subject: [PATCH 5/7] test values of views --- test/strings/basic.jl | 1 + 1 file changed, 1 insertion(+) diff --git a/test/strings/basic.jl b/test/strings/basic.jl index 43048cde9dcf4..07193cb41a729 100644 --- a/test/strings/basic.jl +++ b/test/strings/basic.jl @@ -183,6 +183,7 @@ end @test (@views (x[3])) isa Char @test (@views (x[3], x[1:2], x[[1,4]])) isa Tuple{Char, SubString, String} + @test (@views (x[3], x[1:2], x[[1,4]])) == ('c', "ab", "ad") end From 37d183365869ed9ddbaba6824929c6fcf404cce8 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 19 May 2020 22:00:35 +0100 Subject: [PATCH 6/7] make substring etc work on AbstractUnitRanges --- NEWS.md | 1 + base/strings/substring.jl | 6 +++--- test/strings/basic.jl | 42 ++++++++++++++++++++++++--------------- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/NEWS.md b/NEWS.md index 32f271c6a7cbc..70b9fc10647c6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -44,6 +44,7 @@ Standard library changes * The `nextprod` function now accepts tuples and other array types for its first argument ([#35791]). * The function `isapprox(x,y)` now accepts the `norm` keyword argument also for numeric (i.e., non-array) arguments `x` and `y` ([#35883]). * `view`, `@view`, and `@views` now work on `AbstractString`s, returning a `SubString` when appropriate ([#35879]). +* All `AbstractUnitRange{<:Integer}`s now work with `SubString`, `view`,`@view` and `@views` on strings ([#35879]). #### LinearAlgebra diff --git a/base/strings/substring.jl b/base/strings/substring.jl index 86f5a3388ced0..312ca1941fbce 100644 --- a/base/strings/substring.jl +++ b/base/strings/substring.jl @@ -37,7 +37,7 @@ end @propagate_inbounds SubString(s::T, i::Int, j::Int) where {T<:AbstractString} = SubString{T}(s, i, j) @propagate_inbounds SubString(s::AbstractString, i::Integer, j::Integer=lastindex(s)) = SubString(s, Int(i), Int(j)) -@propagate_inbounds SubString(s::AbstractString, r::UnitRange{<:Integer}) = SubString(s, first(r), last(r)) +@propagate_inbounds SubString(s::AbstractString, r::AbstractUnitRange{<:Integer}) = SubString(s, first(r), last(r)) @propagate_inbounds function SubString(s::SubString, i::Int, j::Int) @boundscheck i ≤ j && checkbounds(s, i:j) @@ -47,8 +47,8 @@ end SubString(s::AbstractString) = SubString(s, 1, lastindex(s)) SubString{T}(s::T) where {T<:AbstractString} = SubString{T}(s, 1, lastindex(s)) -@propagate_inbounds view(s::AbstractString, r::UnitRange{<:Integer}) = SubString(s, r) -@propagate_inbounds maybeview(s::AbstractString, r::UnitRange{<:Integer}) = view(s, r) +@propagate_inbounds view(s::AbstractString, r::AbstractUnitRange{<:Integer}) = SubString(s, r) +@propagate_inbounds maybeview(s::AbstractString, r::AbstractUnitRange{<:Integer}) = view(s, r) @propagate_inbounds maybeview(s::AbstractString, args...) = getindex(s, args...) convert(::Type{SubString{S}}, s::AbstractString) where {S<:AbstractString} = diff --git a/test/strings/basic.jl b/test/strings/basic.jl index 07193cb41a729..aa6974e53a157 100644 --- a/test/strings/basic.jl +++ b/test/strings/basic.jl @@ -168,22 +168,32 @@ end @testset "SubStrings and Views" begin x = "abcdefg" - @test SubString(x, 2:4) == "bcd" - @test view(x, 2:4) == "bcd" - @test view(x, 2:4) isa SubString - @test (@view x[4:end]) == "defg" - @test (@view x[4:end]) isa SubString - - # We don't (at present) make non-contiguous SubStrings with views - @test_throws MethodError (@view x[[1,3,5]]) - @test (@views (x[[1,3,5]])) isa String - - # We don't (at present) make single character SubStrings with views - @test_throws MethodError (@view x[3]) - @test (@views (x[3])) isa Char - - @test (@views (x[3], x[1:2], x[[1,4]])) isa Tuple{Char, SubString, String} - @test (@views (x[3], x[1:2], x[[1,4]])) == ('c', "ab", "ad") + @testset "basic unit range" begin + @test SubString(x, 2:4) == "bcd" + @test view(x, 2:4) == "bcd" + @test view(x, 2:4) isa SubString + @test (@view x[4:end]) == "defg" + @test (@view x[4:end]) isa SubString + end + + @testset "other AbstractUnitRanges" begin + @test SubString(x, Base.OneTo(3)) == "abc" + @test view(x, Base.OneTo(4)) == "abcd" + @test view(x, Base.OneTo(4)) isa SubString + end + + @testset "views but not view" begin + # We don't (at present) make non-contiguous SubStrings with views + @test_throws MethodError (@view x[[1,3,5]]) + @test (@views (x[[1,3,5]])) isa String + + # We don't (at present) make single character SubStrings with views + @test_throws MethodError (@view x[3]) + @test (@views (x[3])) isa Char + + @test (@views (x[3], x[1:2], x[[1,4]])) isa Tuple{Char, SubString, String} + @test (@views (x[3], x[1:2], x[[1,4]])) == ('c', "ab", "ad") + end end From 64e261e14b354085e943e5aebdf97c003fdcfb79 Mon Sep 17 00:00:00 2001 From: Lyndon White Date: Tue, 19 May 2020 23:52:10 +0100 Subject: [PATCH 7/7] Fix missing space --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 70b9fc10647c6..f75f68543fec5 100644 --- a/NEWS.md +++ b/NEWS.md @@ -44,7 +44,7 @@ Standard library changes * The `nextprod` function now accepts tuples and other array types for its first argument ([#35791]). * The function `isapprox(x,y)` now accepts the `norm` keyword argument also for numeric (i.e., non-array) arguments `x` and `y` ([#35883]). * `view`, `@view`, and `@views` now work on `AbstractString`s, returning a `SubString` when appropriate ([#35879]). -* All `AbstractUnitRange{<:Integer}`s now work with `SubString`, `view`,`@view` and `@views` on strings ([#35879]). +* All `AbstractUnitRange{<:Integer}`s now work with `SubString`, `view`, `@view` and `@views` on strings ([#35879]). #### LinearAlgebra