From d3499dfcaf6340f8b181ede2df12e77a2988344b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bogumi=C5=82=20Kami=C5=84ski?= <bkamins@sgh.waw.pl>
Date: Mon, 26 Jun 2017 03:04:05 +0200
Subject: [PATCH] Improve implementation of rstrip and lstrip (#22496)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* change chomp! to chomp in readchomp

`chomp` uses `SubString` so `chomp!` is not needed to ensure that copying of big string is not performed.

* Remove chomp! function

`chomp!` was only used in `readchomp` and I propose to replace it with `chomp` there. As `chomp!` was not exported I understand that it can be removed without deprecation.

* Make rstrip and lstrip use SubString

In this way copying of string is avoided, and in `strip` currently the string is actually copied twice. A basic benchmark is given below:
```
julia> using BenchmarkTools

julia> s = " "^10*"x"^1000*" "^10;

julia> @benchmark strip(s) # current definition
BenchmarkTools.Trial:
  memory estimate:  2.13 KiB
  allocs estimate:  2
  --------------
  minimum time:     502.056 ns (0.00% GC)
  median time:      537.579 ns (0.00% GC)
  mean time:        631.383 ns (5.74% GC)
  maximum time:     6.003 μs (0.00% GC)
  --------------
  samples:          10000
  evals/sample:     197

julia> @benchmark strip(s) # with SubString definition
BenchmarkTools.Trial:
  memory estimate:  64 bytes
  allocs estimate:  2
  --------------
  minimum time:     334.062 ns (0.00% GC)
  median time:      339.819 ns (0.00% GC)
  mean time:        362.150 ns (1.00% GC)
  maximum time:     8.236 μs (93.19% GC)
  --------------
  samples:          10000
  evals/sample:     243
```

* Correct strip tests

* Bring back chomp!

* reverse removal of chomp!

* Add NEWS entry for lstrip and rstrip.

* remove trailing whitespace in NEWS.md
---
 NEWS.md              | 2 ++
 base/strings/util.jl | 9 +++++----
 test/strings/util.jl | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index b78e03c269e17a..69ac90ecc2b09b 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -47,6 +47,8 @@ This section lists changes that do not have deprecation warnings.
 Library improvements
 --------------------
 
+  * The functions `strip`, `lstrip` and `rstrip` now return `SubString` ([#22496]).
+
   * The functions `base` and `digits` digits now accept a negative
     base (like `ndigits` did) ([#21692]).
 
diff --git a/base/strings/util.jl b/base/strings/util.jl
index c4c44595451285..2ac087aab90750 100644
--- a/base/strings/util.jl
+++ b/base/strings/util.jl
@@ -137,15 +137,16 @@ julia> lstrip(a)
 ```
 """
 function lstrip(s::AbstractString, chars::Chars=_default_delims)
+    e = endof(s)
     i = start(s)
     while !done(s,i)
         c, j = next(s,i)
         if !(c in chars)
-            return s[i:end]
+            return SubString(s, i, e)
         end
         i = j
     end
-    s[end+1:end]
+    SubString(s, e+1, e)
 end
 
 """
@@ -171,11 +172,11 @@ function rstrip(s::AbstractString, chars::Chars=_default_delims)
     while !done(r,i)
         c, j = next(r,i)
         if !(c in chars)
-            return s[1:end-i+1]
+            return SubString(s, 1, endof(s)-i+1)
         end
         i = j
     end
-    s[1:0]
+    SubString(s, 1, 0)
 end
 
 """
diff --git a/test/strings/util.jl b/test/strings/util.jl
index 92298b767b07b7..d0d232b5ba977a 100644
--- a/test/strings/util.jl
+++ b/test/strings/util.jl
@@ -26,7 +26,7 @@ for s in ("", " ", " abc", "abc ", "  abc  "), f in (lstrip, rstrip, strip)
         ft = f(t)
         @test s == t
         @test fs == ft
-        @test typeof(ft) == typeof(t[1:end])
+        @test typeof(ft) == SubString{T}
 
         b = convert(SubString{T}, t)
         fb = f(b)