string overhaul #24999

StefanKarpinski · 2017-12-09T00:57:33Z

Replacement PR for #24439. More description to come...

Edit see this discourse post for a description: https://discourse.julialang.org/t/changes-to-the-representation-of-char/7291/5

nalimilan

It's hard to review so many deep changes, but here are the few comments I can make.

nalimilan · 2017-12-09T14:38:47Z

base/iostream.jl

 function peekchar(s::IOStream)
-    if ccall(:ios_peekutf8, Cint, (Ptr{Void}, Ptr{Char}), s, _chtmp) < 0
+    chref = Ref{UInt32}()


Doesn't this hurt performance?

Not according to @Keno who suggested that I move the externally allocated box inside IIRC.

welcome to the brave new world of julia 0.7

nalimilan · 2017-12-09T14:49:29Z

base/strings/basic.jl

+Predicate indicating whether the given index is the start of the encoding of
+a character in `s` or not. If `isvalid(s, i)` is true then `s[i]` will return
+the character whose encoding starts at that index, if it's false, then `s[i]`
+will raise an invalid index error. Behavior of `next(s, i)` is similar except


It would make sense to mention what exception type is thrown exactly.

nalimilan · 2017-12-09T14:50:16Z

base/strings/basic.jl

+a character in `s` or not. If `isvalid(s, i)` is true then `s[i]` will return
+the character whose encoding starts at that index, if it's false, then `s[i]`
+will raise an invalid index error. Behavior of `next(s, i)` is similar except
+that the character is returned along with the index of the following character.


Do we really guarantee that the iterator state is "the index of the following character"? We don't for arrays for example, and some custom string types could do something different. Same remark for the next docstring.

I think there's a fair amount of generic string code in Base (and presumably elsewhere) that assumes that the string iteration state is the same as the string index. If it's not safe to assume that then we need to make sure that people don't use the iterator state in s[i] indexing expressions and that they only use it to pass to done and next.

I would very much like the ability to override string iteration for MyStringType (which I believe is currently possible?), and have the docs reflect the default behaviour of next if you don't also override start and done.

@iamed2, I'm not quite sure what you're asking for – do you want to allow the iteration state not to be the string index? If so, I'm totally fine with that and it's mostly a documentation issue; for the built-in string types they happen to coincide. Might be worth changing the iteration state of SubString to match its parent's iteration state just to shake out the places where we currently rely on these being the same though.

This'll need to be updated in that case to provide whatever is most efficient:

julia/base/io.jl

Lines 649 to 650 in 224f1e9

# requires that indices for target are small ordered integers bounded by start and endof

function readuntil_indexable(io::IO, target#=::Indexable{T}=#, out)

Pretty easy just to add the offset as needed, if that's what we want. At the time, I had seen that in a number of other places we define the bounds of the strings indices to be 1:n, so just went with it:

julia/base/strings/basic.jl

Lines 24 to 25 in 224f1e9

start(s::AbstractString) = 1

done(s::AbstractString,i) = (i > endof(s))

Yes, those are sane fallback definitions. Most strings work that way. The question is just that if someone overrides those, do we support it working. Note that this is not actually pressing for 1.0 since if we don't allow it now, we can always allow it in the future. However, changing the iteration state of substrings might break people's code, so we would want to do that now.

@iamed2, I'm not quite sure what you're asking for – do you want to allow the iteration state not to be the string index? If so, I'm totally fine with that and it's mostly a documentation issue; for the built-in string types they happen to coincide. Might be worth changing the iteration state of SubString to match its parent's iteration state just to shake out the places where we currently rely on these being the same though.

Yes, exactly :)

nalimilan · 2017-12-09T14:50:54Z

base/strings/basic.jl

+will raise an invalid index error. Behavior of `next(s, i)` is similar except
+that the character is returned along with the index of the following character.
+In order for `isvalid(s, i)` to be an O(1) function, the encoding of `s` must
+be [self-synchronizing](https://en.wikipedia.org/wiki/Self-synchronizing_code);


Also use this link for the mention of "self-synchronizing" above.

nalimilan · 2017-12-09T14:51:23Z

base/strings/basic.jl

-sizeof(s::AbstractString) = error("type $(typeof(s)) has no canonical binary representation")
+isvalid(s::AbstractString, i::Integer) = typeof(i) === Int ?
+    throw(MethodError(isvalid, Tuple{typeof(s),Int})) :
+        isvalid(s, Int(i))


Weird indentation. Also why not define two methods?

If there are two methods then any string type that defines isvalid(s::StringType, i::Integer) creates an ambiguity. We could not define any isvalid(s::AbstractString, i::Integer) method that converts to Int but that's annoying because then every type that implements the string interface has to implement the same convert to Int logic. This allows subtypes to just implement the Int method in the common case or handle Integer and either one is fine. In the former case this method is called and the error branch is dead code; in the latter case this method is never called at all.

nalimilan · 2017-12-09T15:26:23Z

base/strings/string.jl

+    @inbounds b = codeunit(s, i)
+    b & 0xc0 == 0x80 || @goto ret
+    u |= UInt32(b); i += 1
+@label ret


It's kind of weird to have no indentation for a label, given that it's only valid inside the parent function.

This is a pretty common style in C/C++ for goto labels, e.g. see http://en.cppreference.com/w/cpp/language/goto. The logic in my mind is that they are part of the control flow construct so having them outdented from the code seems fitting.

nalimilan · 2017-12-09T15:28:14Z

base/strings/string.jl

+        while true
+            (i += 1) ≤ n || break
+            @inbounds b = codeunit(s, i) # lead byte
+        @label L


Same remark as above.

nalimilan · 2017-12-09T15:39:16Z

test/strings/basic.jl

-    @test_throws BoundsError chr2ind("hello", 10)
-    @test_throws BoundsError ind2chr("hellø", 10)
-    @test_throws BoundsError chr2ind("hellø", 10)
+    @test ind2chr("hello", -1) == -1


Should we really support these? Why would -1 codeunit be equal to -1 character? That doesn't make a lot of sense.

nalimilan · 2017-12-09T15:43:20Z

test/strings/basic.jl

-            @test nextind(strs[i], -1, 1) == 1
+            @test prevind(strs[i], 20) == 19
+            @test prevind(strs[i], 20, 1) == 19
+            @test prevind(strs[i], 20, 10) == 7


This kind of behavior sounds weird to me. So we assume that all imaginary characters take one code unit, and once we reach actual characters we use them? I can't imagine a situation where it would be useful, and it could hide bugs...

I originally wrote the assumption that they all take one code unit, but realized that for substrings it's very convenient to just pass-through to the backing string which means that the "imaginary" characters are whatever size the actual characters in the backing string are.

So basically that means that the behavior of prevind/nextind is undefined when an out of bounds index is passed, which isn't very Julian IMHO. It would be more standard to throw an error when an out of bounds index is passed, and enable the undefined behavior via @inbounds, but as you said it's convenient in some cases, so...

It's not undefined – it satisfies lots of nice, predictable properties:

prevind(s, i) < i < nextind(s, i)

prevind(s, i, n) ≤ i ≤ nextind(s, i, n) for all n ≥ 0

prevind(s, nextind(s, i, n), n) == thisind(s, i) for all n > 0

prevind(s, nextind(s, i, n), m) == nextind(s, i, n-m) whenever n > m

nextind(s, prevind(s, i, n), m) == prevind(s, i, n-m) whenever n > m

There is a very consistent philosophy to all of these changes: only throw an error when you have to. For character iteration, the error only occurs when you try to ask about the code point of a malformed character – handling a character or a string that has malformed data is fine. For indexing, computing with out-of-bounds indices is fine, the only thing that's a problem is asking for a character or a string that is out of bounds. As I said before, we do this all the time with array indices and it's not a problem this is just another form index arithmetic.

I said it's undefined because the number of code units which are added depend on the parent string (if any), so for example it will be different for a plain String and for a SubString. So in practice if you pass an out-of-bounds index, call nextind/prevind a given number of times, and get an in-bounds index, it's totally unpredictable what character you get.

Array indices are quite different, since in that case the step is always 1.

Where did this out-of-bounds array index come from? If you start in bounds for a given string and then end back in bounds you always get the same answer. Yes, the out of bounds values in the middle may be different but why does that matter?

Yes, normally you won't pass an out of bounds index. But if that happens because of a bug it would have been nice to be able to catch that by default.

Making out of bound code units always size one won’t help with that. We already discussed the bit about why allowing computing out of bounds values is useful. I guess what hasn’t been shown is that these functions need to accept out of bounds indices as well as produce them.

nalimilan · 2017-12-09T15:46:03Z

test/strings/types.jl

@@ -46,10 +55,12 @@ for idx in [0, 1, 4]
    @test SubString("∀∀", 4, idx) == "∀∀"[4:idx]
 end

-# second index beyond endof("∀∀")
-for idx in 5:8
+# index beyond endof("∀∀")


"second" was accurate.

I still don't get what it means.

It refers to the second index (third argument) passed to SubString.

Oh, that was completely unclear to me since the comment doesn't mention SubString at all. I think I'd like to deprecate the three-arg constructor for SubString anyway and always use SubString(s, i:j) instead.

StefanKarpinski · 2017-12-09T17:40:52Z

Regarding indexing past the ends of strings. Here's my reasoning for it. At the very least we need to support indices of 0 and ncodeunits(s)+1 since those come up all the time in generic definitions and not supporting those leads to really awkward code to support edge cases. For example, the current definition of ind2chr(s, i) = length(s, 1, i) breaks down for i == 0 unless we allow zero. Similarly, the definition of chr2ind(s, i) uses prevind(s, 0, -n) and nextind(s, 0, n). Also consider the new simpler definitions of first as s[1:min(end, nextind(s, 0, n))] and last as s[max(1, prevind(s, ncodeunits(s)+1, n)):end]. I didn't end up needing it but there were moments where I was considering writing code like nextind(s, -1, n+1) for those. Instead, I relaxed the requirement that the number of characters passed to nextind/prevind to advance be strictly positive and allowed it to be zero, which ended up being simpler.

My general philosophy here (which I need to write down somewhere in the string docs) is that doing computations with indices outside of the bounds of strings is totally fine – it's only accessing a character past one of the ends of a string that's a problem. By analogy to indexing into an array, it's perfectly normal to compute intermediate values when doing index arithmetic that are outside of the range of the array. The problem is only if at the end you index into the array out of bounds. I ended up not needing to use indices outside of the [0, ncodeunits(s)+1] range into strings, but I find it a little hard to justify allowing those but being really strict about going outside of that range.

Here's a motivating example. Suppose you're writing code to find a particular word in some text with k characters of context around it. With the current "lax" behavior, you can find the word – say it goes from index i to j in text – and then compute the bounds of what you should show as max(1, prevind(text, i, k)) and min(endof(text), nextind(text, j, k)). How do you write this code if prevind and nextind don't let you compute past the end of the string and throw an error instead? You would have to check if the beginning and end of text are each no more than k characters before and after the end of the word first. This is actually quite hard to do efficiently. You could do length(text, 1, i) and length(text, j, endof(test)) but those are O(length(text)) operations instead of O(k). The only alternative I can think of is to do individual single-step prevind and nextind operations, checking at each one if you've hit the ends of the text or not. That's O(k) at least, but now you've lost any efficiency that the three-argument prevind and nextind might have for the string encoding, which, e.g. for the String type are considerable, since every prevind and nextind operation needs to figure out if the index it's at is the beginning of a character or not, which is non-trivial due to the possibility of invalid UTF-8 data.

What about having prevind and nextind allow you to advance beyond the ends of a string but just saturating at 0 and ncodeunits(s)+1? That has all the problems that saturating arithmetic for normal integers does. In the case of UTF-32 (or any fixed-width encoding) where these operations are just normal integer arithmetic, having an API where index operations saturate or throw errors means that you can't have the benefits of simple indexing – instead you're forced to implement nextind in a complicated way that prevents optimizations rather than nextind(s, i, n) = i + n and prevind(s, i, n) = i - n. Even in encodings where nextind and prevind can't be that simple, saturation ruins all the nice universal properties you might want for these functions, such as that prevind(s, i) < i < nextind(s, i) or that prevind(s, nextind(s, i, n), n) == thisind(s, i) and so on. Instead, you have to throw in caveats about the ends of strings and whatnot.

nalimilan · 2017-12-09T17:57:23Z

OK, I see the advantages. But I still still find it disturbing when nextind or prevind start with an out-of-bounds character, iterate over a few imaginary one-codeunit characters, and enter the actual string at some point. But well...

StefanKarpinski · 2017-12-09T18:12:11Z

Can you think of an efficient way to handle the word-search-with-context example any other way?

StefanKarpinski · 2017-12-09T18:37:40Z

I was tempted to just make StringIndexError a KeyError but the name sounds a bit funny. If KeyError were called IndexError it would work a bit better. We have a lot of almost-overlapping error types for bad i in obj[i], I have to wonder if we can't unify them a bit.

nalimilan · 2017-12-09T20:16:57Z

Can you think of an efficient way to handle the word-search-with-context example any other way?

Well, it would work if prevind/nextind stopped when reaching the bounds of the string. But as you noted that behavior has drawbacks too, but in practical terms I'm not sure whether they are really problematic or not.

I was tempted to just make StringIndexError a KeyError but the name sounds a bit funny. If KeyError were called IndexError it would work a bit better. We have a lot of almost-overlapping error types for bad i in obj[i], I have to wonder if we can't unify them a bit.

Given how special string indices are, it makes sense to have a dedicated exception for them.

StefanKarpinski · 2017-12-09T20:26:32Z

But as you noted that behavior has drawbacks too, but in practical terms I'm not sure whether they are really problematic or not.

I suspect I can come up with a slightly more complex use case where coming back in-bounds is required for the above word-with-context example. But there's a reason we don't do saturating integer arithmetic and this is just another form of integer arithmetic.

StefanKarpinski · 2017-12-09T21:11:43Z

I'm getting

 !! No doc found for reference '[`ncodeunits`](@ref)'.

Anyone know why that would be happening? There is a docstring for ncodeunits so I'm confused about what the problem is. I seem to be using the @ref thing wrong somehow although as far as I can tell I'm using the same way as all the other docstrings do.

nalimilan · 2017-12-09T21:48:48Z

I think you need to add the function to the stdlib part of the manual for Documenter to find it.

StefanKarpinski · 2017-12-10T17:28:08Z

@nalimilan, I've thought about this a bit and while there are examples (like the word-context one) that show that it's useful to produce out-of-bounds indices from string index arithmetic functions, I can't think of a concrete use case for accepting them – except for the only-just-out-of-bounds ones, i.e. 0 and ncodeunits(s)+1. If you have some code that produces a very out-of-bounds index and then passes that to another string index arithmetic function, then why not just combine those operations? The identities I showed above demonstrate that you can do since

prevind(s, nextind(s, i, n), m) == nextind(s, i, n - m) whenever n > m
nextind(s, prevind(s, i, n), m) == prevind(s, i, n - m) whenever n > m

So why not replace the RHS with the LHS which does less work and may avoid going out of bounds where the LHS went out of bounds temporarily? So I guess the conservative thing to do where would be to make these functions raise errors when passed starting indices outside of the range 0:ncodeunits(s)+1 even though they are allowed to produce out of bounds values. It bothers me that some out-of-bounds indices are allowed while others aren't. What principled reason do we have for that distinction other than that it's practical? In my view producing very out of bounds indices is safer than saturating at 0 and ncodeunits(s)+1 since saturating might very well hide a computation where you end up back in bounds but should not have – it's better to produce very out of bounds values which are then an error to use.

StefanKarpinski · 2017-12-10T17:31:59Z

Related issue: isvalid. Previously, isvalid simply returned false for out-of-bounds indices. In this PR I've made a clearer distinction between out-of-bounds indices – i.e. those not satisfying 1 ≤ i ≤ ncodeunits(s) – and invalid indices that are not the start of a character. The new isvalid throws an error if given an out-of-bounds index. I'm not sure what the better behavior is.

StefanKarpinski · 2017-12-10T18:08:02Z

I've pushed a new commit deleting the bswap(::Char) method. I had added a deprecation but when I want to test the deprecation, I realized that there are basically no character values ('\0' is the only exception) for which you can bswap the 32-bit code point and get back a valid code point, so this function is essentially completely useless and cannot really be made to work in the new scheme since there's no way to take a completely invalid code point and turn it into a Char value.

nalimilan · 2017-12-10T18:13:23Z

It sounds reasonable to start with the most restrictive behavior (throwing an error), since if it turns out to be problematic in some cases we will be able to change it, which wouldn't be possible in the other direction.

StefanKarpinski · 2017-12-10T19:55:37Z

Yeah, I suppose so, but it just feels a bit weird and undisciplined to allow some out-of-bounds indices – 0 and ncodeunits(s)+1 – but not others – -1 or ncodeunits(s)+2. And we have very real use cases for the former so we don't want to disallow those.

StefanKarpinski · 2017-12-10T20:10:25Z

I have now added the deprecation of ind2chr and chr2ind in a separate commit (for easier review). It was fairly straightforward and improves performance in a few places. In the process I also encountered the #25016 lpad and rpad issue. I've updated the code to those functions but left the inconsistent behavior in-place for now.

nalimilan · 2017-12-10T20:23:49Z

doc/src/manual/strings.md

@@ -565,14 +565,15 @@ Some other useful functions include:

  * [`endof(str)`](@ref) gives the maximal (byte) index that can be used to index into `str`.
  * [`length(str)`](@ref) the number of characters in `str`.
+  * [`length(str, i, j)`](@ref) the number of valid character indices in `str` from `i` to `j`.


Actually I wonder whether we should add this method, as it looks a bit weird. Would it be enough to recommend length(SubString(str, i:j))?

I'm concerned that it's not yet free to create such an intermediate object. @Keno, @vtjnash? Note that the new length method is also less strict – it doesn't require that the inputs be valid indices. You can't properly deprecate the existing ind2chr behavior without that.

It likely can be free if it gets inlined. But seem to recall the SubString constructor does quite a bit of work to munge the indices into something valid? Anyways, doesn't seem like this API decision should be too much influenced by temporary performance concerns (unless it's only for internal usage, but them maybe call it Base.substr_length).

GregPlowman · 2017-12-11T00:59:51Z

I'm getting

!! No doc found for reference 'ncodeunits'.
Anyone know why that would be happening? There is a docstring for ncodeunits so I'm confused about what the problem is. I seem to be using the @ref thing wrong somehow although as far as I can tell I'm using the same way as all the other docstrings do.

Not sure if its related, but I had a similar problem: #23703 (comment)

Removing the backticks fixed the issue, but this seemed less than ideal because I thought "auto-referencing" should work with or without backticks.

StefanKarpinski · 2017-12-11T22:36:32Z

@nalimilan: is it ok with you if I merge my PR as is to avoid it getting lots of conflicts and then make the changes to make indexing more conservative in a separate PR? My general plan is to try this and see how it goes:

make isvalid return false for out-of-bounds indices instead of an error
make thisind, nextind, prevind, length error on most/all out-of-bounds indices
- allow 0 for nextind and the hi argument to length
- allow ncodeunits(s)+1 for prevind and the lo argument of length
- otherwise only allow in-bounds indices

tpapp · 2017-12-12T15:50:43Z

base/strings/basic.jl

+ncodeunits(s::AbstractString)
+
+"""
+    codeunit(s::AbstractString) -> Type{<:Union{UInt8, UInt16, UInt32}}


(from a discussion on Discourse)

I think that the function that retrieves the type should perhaps have a different name than the function that returns a codeunit. eg codeunit_type. Methods of the same function usually share behavior on some abstract level, this is stretching that. I realize that the name is long, but it is only used in a few places (mostly to define the method for various types). I expect that in code outside Base it will be only used to help type inference so that this will not be cumbersome.

nalimilan · 2017-12-12T16:16:32Z

Sorry, I hadn't seen your question. Yes, that's fine with me. (Actually, what worries me more are the conflicts with the search/find PRs, but such is life...)

- {next,getindex}_continued don't need to re-check bounds - short-circuiting is slightly faster in length

also: avoid second bounds check in `get` with default

also: `next` can assume that incoming indices are valid

StefanKarpinski · 2017-12-13T21:58:31Z

Is the memory error on FreeBSD expected these days? cc @iblis17

StefanKarpinski · 2017-12-14T00:07:13Z

Ok, I've already tested the heck out of this change, the BSD failure seems to be an unrelated memory problem, it passes on Windows and Linux and I can't reproduce the macOS spawn failure locally, so I'm assuming that's unrelated too.

StefanKarpinski · 2017-12-14T00:08:21Z

I did not mean to squash that, but I think it's fine. The intermediate commits are not all that useful.

iblislin · 2017-12-14T01:06:10Z

@StefanKarpinski about the ENOMEM: #23273 (comment)

StefanKarpinski · 2017-12-14T01:30:12Z

Actually this wasn't squashed, GitHub just changed how the display merge commits.

fix #24840 all the way, already mostly fixed by #24999

actually fix #24840, partly (badly) fixed by #24999

kshyatt added the domain:strings "Strings!" label Dec 9, 2017

nalimilan reviewed Dec 9, 2017

View reviewed changes

This was referenced Dec 9, 2017

have a single bad index error type? #25005

Closed

wip: string overhaul #24439

Closed

StefanKarpinski force-pushed the sk/strings branch from 5b29efa to 463ea70 Compare December 9, 2017 19:57

nalimilan reviewed Dec 10, 2017

View reviewed changes

StefanKarpinski mentioned this pull request Dec 10, 2017

charseek: replace thisind, nextind and prevind? #25019

Closed

StefanKarpinski mentioned this pull request Dec 11, 2017

Fix #24214: Extended isvalid() for SubString #24231

Closed

StefanKarpinski force-pushed the sk/strings branch 2 times, most recently from 6739272 to 5708e3e Compare December 11, 2017 22:03

StefanKarpinski force-pushed the sk/strings branch 2 times, most recently from c7d5ed1 to 46bcd75 Compare December 12, 2017 15:40

tpapp reviewed Dec 12, 2017

View reviewed changes

StefanKarpinski added 5 commits December 13, 2017 13:46

various String performance tweaks

1f0c6fa

- {next,getindex}_continued don't need to re-check bounds - short-circuiting is slightly faster in length

add various @propagate_inbounds annotations

6f10ca2

also: avoid second bounds check in `get` with default

isvalid: return false out of bounds instead of throwing

937c3ad

also: `next` can assume that incoming indices are valid

bounds checks on string length(s, i, j)

feb1f68

bounds check thisind, nextind and prevind as well

8de25f5

StefanKarpinski force-pushed the sk/strings branch from 94e99d3 to 8de25f5 Compare December 13, 2017 19:01

StefanKarpinski merged commit d192302 into master Dec 14, 2017

StefanKarpinski deleted the sk/strings branch December 14, 2017 00:07

StefanKarpinski restored the sk/strings branch December 14, 2017 00:08

StefanKarpinski deleted the sk/strings branch December 14, 2017 01:30

StefanKarpinski mentioned this pull request Dec 14, 2017

Add get(string, index, default) #22500

Merged

StefanKarpinski added a commit that referenced this pull request Dec 14, 2017

make checkbounds(string, ...) return nothing or error

850b1fb

fix #24840 all the way, already mostly fixed by #24999

This was referenced Dec 14, 2017

make checkbounds(string, ...) return nothing or error #25086

Merged

String as more of a byte vector type? #22616

Closed

StefanKarpinski added a commit that referenced this pull request Dec 15, 2017

make checkbounds(string, ...) return nothing or error

485dad7

actually fix #24840, partly (badly) fixed by #24999

ararslan mentioned this pull request Dec 16, 2017

Overload ncodeunits on 0.7 JuliaData/CategoricalArrays.jl#114

Merged

Vaibhavdixit02 mentioned this pull request Dec 27, 2017

Fixing #24214 #25285

Closed

StefanKarpinski mentioned this pull request Jan 20, 2018

Two-stage unicode tables for UTF-8 Char format #25653

Open

stevengj mentioned this pull request Jun 12, 2018

thisind, 3-arg length/nextind/prevind, codeunit(s) JuliaLang/Compat.jl#573

Merged

KristofferC mentioned this pull request Jun 20, 2018

Benchmarks vs 0.6 in prep for 0.7 release [do not merge] #27030

Closed

bkamins mentioned this pull request May 6, 2020

add better implementations for generic findprev findnext for AbstractString #35742

Merged

stevengj mentioned this pull request Mar 16, 2022

literals for invalid characters? #44646

Closed

This was referenced Jan 14, 2023

faster isascii(::Char) #48281

Closed

incorrect search results for malformed Char #48283

Open

stevengj mentioned this pull request Jul 13, 2023

read(io, Char) doesn't match collect(string) for malformed UTF-8 #50532

Closed

	# requires that indices for target are small ordered integers bounded by start and endof
	function readuntil_indexable(io::IO, target#=::Indexable{T}=#, out)

	start(s::AbstractString) = 1
	done(s::AbstractString,i) = (i > endof(s))

string overhaul #24999

string overhaul #24999

Conversation

StefanKarpinski commented Dec 9, 2017 • edited Loading

nalimilan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanKarpinski Dec 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanKarpinski Dec 9, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

StefanKarpinski commented Dec 9, 2017 • edited Loading

nalimilan commented Dec 9, 2017

StefanKarpinski commented Dec 9, 2017 • edited Loading

StefanKarpinski commented Dec 9, 2017

nalimilan commented Dec 9, 2017

StefanKarpinski commented Dec 9, 2017

StefanKarpinski commented Dec 9, 2017 • edited Loading

nalimilan commented Dec 9, 2017

StefanKarpinski commented Dec 10, 2017

StefanKarpinski commented Dec 10, 2017

StefanKarpinski commented Dec 10, 2017

nalimilan commented Dec 10, 2017

StefanKarpinski commented Dec 10, 2017

StefanKarpinski commented Dec 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GregPlowman commented Dec 11, 2017

StefanKarpinski commented Dec 11, 2017 • edited Loading

Choose a reason for hiding this comment

nalimilan commented Dec 12, 2017

StefanKarpinski commented Dec 13, 2017

StefanKarpinski commented Dec 14, 2017

StefanKarpinski commented Dec 14, 2017

iblislin commented Dec 14, 2017

StefanKarpinski commented Dec 14, 2017

StefanKarpinski commented Dec 9, 2017 •

edited

Loading

StefanKarpinski Dec 9, 2017 •

edited

Loading

StefanKarpinski Dec 9, 2017 •

edited

Loading

StefanKarpinski commented Dec 9, 2017 •

edited

Loading

StefanKarpinski commented Dec 9, 2017 •

edited

Loading

StefanKarpinski commented Dec 9, 2017 •

edited

Loading

StefanKarpinski commented Dec 11, 2017 •

edited

Loading