Skip to content

Commit 74fea5d

Browse files
committed
fix: logic error in String.Slice.takeWhile (#10868)
This PR fixes a bug in `String.Slice.takeWhile` which caused it to get its bookkeeping wrong and panic. The new version only uses safe operations on `String.Slice.Pos`. (cherry picked from commit 196d501)
1 parent 7133f1a commit 74fea5d

File tree

5 files changed

+53
-27
lines changed

5 files changed

+53
-27
lines changed

src/Init/Data/String/Basic.lean

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1870,7 +1870,7 @@ theorem Pos.Raw.unoffsetBy_offsetBy {p q : Pos.Raw} : (p.offsetBy q).unoffsetBy
18701870
/-- Given a position in `s` that is at least `p₀`, obtain the corresponding position in
18711871
`s.replaceStart p₀`. -/
18721872
@[inline]
1873-
def Slice.Pos.toReplaceStart {s : Slice} (p₀ : s.Pos) (pos : s.Pos) (h : p₀.offset ≤ pos.offset) :
1873+
def Slice.Pos.toReplaceStart {s : Slice} (p₀ : s.Pos) (pos : s.Pos) (h : p₀ ≤ pos) :
18741874
(s.replaceStart p₀).Pos where
18751875
offset := pos.offset.unoffsetBy p₀.offset
18761876
isValidForSlice := Pos.Raw.isValidForSlice_replaceStart.2 (by
@@ -1904,6 +1904,27 @@ theorem Slice.Pos.get_eq_get_ofReplaceStart {s : Slice} {p₀ : s.Pos} {pos : (s
19041904
pos.get h = (ofReplaceStart pos).get (by rwa [← ofReplaceStart_endPos, ne_eq, ofReplaceStart_inj]) := by
19051905
simp [Slice.Pos.get, Nat.add_assoc]
19061906

1907+
/-- Given a position in `s.replaceEnd p₀`, obtain the corresponding position in `s`. -/
1908+
@[inline]
1909+
def Slice.Pos.ofReplaceEnd {s : Slice} {p₀ : s.Pos} (pos : (s.replaceEnd p₀).Pos) : s.Pos where
1910+
offset := pos.offset
1911+
isValidForSlice := (Pos.Raw.isValidForSlice_replaceEnd.1 pos.isValidForSlice).2
1912+
1913+
@[simp]
1914+
theorem Slice.Pos.offset_ofReplaceEnd {s : Slice} {p₀ : s.Pos} {pos : (s.replaceEnd p₀).Pos} :
1915+
(ofReplaceEnd pos).offset = pos.offset := (rfl)
1916+
1917+
/-- Given a position in `s` that is at most `p₀`, obtain the corresponding position in `s.replaceEnd p₀`. -/
1918+
@[inline]
1919+
def Slice.Pos.toReplaceEnd {s : Slice} (p₀ : s.Pos) (pos : s.Pos) (h : pos ≤ p₀) :
1920+
(s.replaceEnd p₀).Pos where
1921+
offset := pos.offset
1922+
isValidForSlice := Pos.Raw.isValidForSlice_replaceEnd.2 ⟨h, pos.isValidForSlice⟩
1923+
1924+
@[simp]
1925+
theorem Slice.Pos.offset_toReplaceEnd {s : Slice} {p₀ : s.Pos} {pos : s.Pos} {h : pos ≤ p₀} :
1926+
(toReplaceEnd p₀ pos h).offset = pos.offset := (rfl)
1927+
19071928
theorem Slice.Pos.copy_eq_append_get {s : Slice} {pos : s.Pos} (h : pos ≠ s.endPos) :
19081929
∃ t₁ t₂ : String, s.copy = t₁ ++ singleton (pos.get h) ++ t₂ ∧ t₁.utf8ByteSize = pos.offset.byteIdx := by
19091930
obtain ⟨t₂, ht₂⟩ := (s.replaceStart pos).copy.eq_singleton_append (by simpa [← ValidPos.ofCopy_inj, ← ofReplaceStart_inj])

src/Init/Data/String/Pattern/Basic.lean

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ class ForwardPattern (ρ : Type) where
7070
Checks whether the slice starts with the pattern. If it does, the slice is returned with the
7171
prefix removed; otherwise the result is {name}`none`.
7272
-/
73-
dropPrefix? : Slice → ρ → Option Slice
73+
dropPrefix? : (s : Slice) → ρ → Option s.Pos
7474

7575
namespace Internal
7676

@@ -153,10 +153,10 @@ def defaultStartsWith (s : Slice) (pat : ρ) : Bool :=
153153
| _ => false
154154

155155
@[specialize pat]
156-
def defaultDropPrefix? (s : Slice) (pat : ρ) : Option Slice :=
156+
def defaultDropPrefix? (s : Slice) (pat : ρ) : Option s.Pos :=
157157
let searcher := ToForwardSearcher.toSearcher s pat
158158
match searcher.step with
159-
| .yield _ (.matched _ endPos) _ => some (s.replaceStart endPos)
159+
| .yield _ (.matched _ endPos) _ => some endPos
160160
| _ => none
161161

162162
@[always_inline, inline]
@@ -196,7 +196,7 @@ class BackwardPattern (ρ : Type) where
196196
Checks whether the slice ends with the pattern. If it does, the slice is returned with the
197197
suffix removed; otherwise the result is {name}`none`.
198198
-/
199-
dropSuffix? : Slice → ρ → Option Slice
199+
dropSuffix? : (s : Slice) → ρ → Option s.Pos
200200

201201
namespace ToBackwardSearcher
202202

@@ -212,10 +212,10 @@ def defaultEndsWith (s : Slice) (pat : ρ) : Bool :=
212212
| _ => false
213213

214214
@[specialize pat]
215-
def defaultDropSuffix? (s : Slice) (pat : ρ) : Option Slice :=
215+
def defaultDropSuffix? (s : Slice) (pat : ρ) : Option s.Pos :=
216216
let searcher := ToBackwardSearcher.toSearcher s pat
217217
match searcher.step with
218-
| .yield _ (.matched startPos _) _ => some (s.replaceEnd startPos)
218+
| .yield _ (.matched startPos _) _ => some startPos
219219
| _ => none
220220

221221
@[always_inline, inline]

src/Init/Data/String/Pattern/String.lean

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -218,9 +218,9 @@ def startsWith (s : Slice) (pat : Slice) : Bool :=
218218
false
219219

220220
@[inline]
221-
def dropPrefix? (s : Slice) (pat : Slice) : Option Slice :=
221+
def dropPrefix? (s : Slice) (pat : Slice) : Option s.Pos :=
222222
if startsWith s pat then
223-
some <| s.replaceStart <| s.pos! <| pat.rawEndPos.offsetBy s.startPos.offset
223+
some <| s.pos! <| pat.rawEndPos.offsetBy s.startPos.offset
224224
else
225225
none
226226

@@ -254,9 +254,9 @@ def endsWith (s : Slice) (pat : Slice) : Bool :=
254254
false
255255

256256
@[inline]
257-
def dropSuffix? (s : Slice) (pat : Slice) : Option Slice :=
257+
def dropSuffix? (s : Slice) (pat : Slice) : Option s.Pos :=
258258
if endsWith s pat then
259-
some <| s.replaceEnd <| s.pos! <| s.endPos.offset.unoffsetBy pat.rawEndPos
259+
some <| s.pos! <| s.endPos.offset.unoffsetBy pat.rawEndPos
260260
else
261261
none
262262

src/Init/Data/String/Slice.lean

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ Examples:
251251
-/
252252
@[inline]
253253
def dropPrefix? [ForwardPattern ρ] (s : Slice) (pat : ρ) : Option Slice :=
254-
ForwardPattern.dropPrefix? s pat
254+
(ForwardPattern.dropPrefix? s pat).map s.replaceStart
255255

256256
/--
257257
If {name}`pat` matches a prefix of {name}`s`, returns the remainder. Returns {name}`s` unmodified
@@ -357,18 +357,18 @@ Examples:
357357
-/
358358
@[inline]
359359
partial def takeWhile [ForwardPattern ρ] (s : Slice) (pat : ρ) : Slice :=
360-
go s
360+
go s.startPos
361361
where
362362
@[specialize pat]
363-
go (curr : Slice) : Slice :=
364-
if let some nextCurr := dropPrefix? curr pat then
365-
if curr.startInclusive.offset < nextCurr.startInclusive.offset then
363+
go (curr : s.Pos) : Slice :=
364+
if let some nextCurr := ForwardPattern.dropPrefix? (s.replaceStart curr) pat then
365+
if (s.replaceStart curr).startPos < nextCurr then
366366
-- TODO: need lawful patterns to show this terminates
367-
go nextCurr
367+
go (Pos.ofReplaceStart nextCurr)
368368
else
369-
s.replaceEnd <| s.pos! <| curr.startInclusive.offset
369+
s.replaceEnd curr
370370
else
371-
s.replaceEnd <| s.pos! <| curr.startInclusive.offset
371+
s.replaceEnd curr
372372

373373
/--
374374
Finds the position of the first match of the pattern {name}`pat` in a slice {name}`true`. If there
@@ -524,7 +524,7 @@ Examples:
524524
-/
525525
@[inline]
526526
def dropSuffix? [BackwardPattern ρ] (s : Slice) (pat : ρ) : Option Slice :=
527-
BackwardPattern.dropSuffix? s pat
527+
(BackwardPattern.dropSuffix? s pat).map s.replaceEnd
528528

529529
/--
530530
If {name}`pat` matches a suffix of {name}`s`, returns the remainder. Returns {name}`s` unmodified
@@ -629,18 +629,18 @@ Examples:
629629
-/
630630
@[inline]
631631
partial def takeEndWhile [BackwardPattern ρ] (s : Slice) (pat : ρ) : Slice :=
632-
go s
632+
go s.endPos
633633
where
634634
@[specialize pat]
635-
go (curr : Slice) : Slice :=
636-
if let some nextCurr := dropSuffix? curr pat then
637-
if nextCurr.endExclusive.offset < curr.endExclusive.offset then
635+
go (curr : s.Pos) : Slice :=
636+
if let some nextCurr := BackwardPattern.dropSuffix? (s.replaceEnd curr) pat then
637+
if nextCurr < (s.replaceEnd curr).endPos then
638638
-- TODO: need lawful patterns to show this terminates
639-
go nextCurr
639+
go (Pos.ofReplaceEnd nextCurr)
640640
else
641-
s.replaceStart <| s.pos! <| curr.endExclusive.offset
641+
s.replaceStart curr
642642
else
643-
s.replaceStart <| s.pos! <| curr.endExclusive.offset
643+
s.replaceStart curr
644644

645645
/--
646646
Finds the position of the first match of the pattern {name}`pat` in a slice {name}`true`, starting

tests/lean/run/string_slice.lean

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ Tests for `String.Slice` functions
5555
#guard "red red green blue".toSlice.takeWhile "red " == "red red ".toSlice
5656
#guard "red green blue".toSlice.takeWhile (fun (_ : Char) => true) == "red green blue".toSlice
5757

58+
#guard (" ".toSlice.dropWhile ' ' |>.takeWhile Char.isLower) == "".toSlice
59+
#guard (" ".toSlice.dropWhile ' ' |>.takeEndWhile Char.isLower) == "".toSlice
60+
#guard ("∃a∃".toSlice.drop 1 |>.takeWhile Char.isLower) == "a".toSlice
61+
#guard ("∃a∃".toSlice.dropEnd 1 |>.takeEndWhile Char.isLower) == "a".toSlice
62+
5863
#guard "red green blue".toSlice.dropPrefix? "red " == some "green blue".toSlice
5964
#guard "red green blue".toSlice.dropPrefix? "reed " == none
6065
#guard "red green blue".toSlice.dropPrefix? 'r' == some "ed green blue".toSlice

0 commit comments

Comments
 (0)