Don't crash with StackoverflowException when long triple-quoted strings are parsed

[kentcb]

Modifies the algorithm for the EndOfInterpolatedString active pattern so that it is non-recursive. I do not see a way of keeping the recursion whilst attaining tail-call (nor do I like the fact that the compiler can't yet enforce tail-call recursion, so accidental breakage is too real a risk)

Fixes #1837

[nojaf]

Thank you for the report of #1837 and this PR.
I'm going to ask the big guns to review this one, just to be on the safe side.
@Smaug123 or @jindraivanek are you up to review this one?

[Smaug123]

If I were being super nitpicky, I'd insist on a more efficient helper function that doesn't involve indexing three times into the same list. The following is completely untested and may not even type check:

let splitAt<'a> (n : int) (xs : 'a list) : ('a list * ('a * 'a list) option) option =
    let rec go n heads rest =
        if n = 0 then
            match rest with
            | [] -> Some (List.rev heads, None)
            | x :: rest -> Some (List.rev heads, x, Some rest)
        else
            match rest with
            | [] -> None
            | x :: rest ->
                go (n - 1) (x :: heads) rest
    go n [] xs

[kentcb]

Thanks @Smaug123. I'm an F# noob so nitpicky is good for my learning. I had the same concerns about indexing into an F# list and played around with a couple of things:

• List.partition. Decided it's not the right tool for the job, since it iterates the entire list and creates two new ones. Not to mention we wanted two lists plus the item between them.
• Converting tokens to array so indexing/slicing was super cheap. Didn't love the result because I had to convert back to lists again, so was concerned about perf there too.

I'll play around with your suggestion over the weekend.

[kentcb]

OK @nojaf @Smaug123 I've played around and got it working with some minor adjustments. I've left a couple of questions for consideration.

[jindraivanek]

Late to the party, but I suggest to simplify this to something along the lines:

let private (|MultipleStringTextTokens|_|) tokens =
    let f =
        function
        | StringTextToken _ -> true
        | _ -> false

    tokens
    |> List.takeWhile f
    |> fun xs ->
        if List.isEmpty xs then
            None
        else
            Some(xs, List.skipWhile f tokens)


let private (|EndOfInterpolatedString|_|) tokens =
    match tokens with
    | MultipleStringTextTokens (stringTokens, rest) ->
        match rest with
        | InterpStringEndOrPartToken endToken :: rest2 -> Some(stringTokens, endToken, rest2)
        | _ -> None
    | _ -> None

Perf of above can be improved by some utility function that returns (List.takeWhile f, List.skipWhile f) in one pass.

[kentcb]

OK all, I've applied feedback. I tried both implementations. My latest commit takes the approach @jindraivanek and removes the List.splitAround function and tests, since they wouldn't be needed in that case. Let me know which approach is preferred and any further feedback.

[nojaf]

Hi Kent, many thanks again for your time and patience to solve this issue.
Almost there!

[kentcb]

@nojaf Done!

[Smaug123]

Nice!

[nojaf]

Many thanks @kentcb for this contribution!
Let me know if you'd like me to release a new version with this fix.