nicklockwood / Expression

A cross-platform Swift library for evaluating mathematical expressions at runtime
MIT License
829 stars 51 forks source link

Crash in Swift 5.7 when creating stringBounds error #43

Closed nighthawk closed 1 year ago

nighthawk commented 1 year ago

There's a crash in static func stringBounds(_ string: Substring, _ index: String.Index) -> AnyExpression.Error -> AnyExpression.Error happening on Swift 5.7 on macOS or Linux, which can be reproduced by running the Test Suite.

This happens for cases where index < _string.startIndex as let offset = _string.distance(from: _string.startIndex, to: index) will no longer return a negative value but instead crash at runtime as index is out of bounds of the string.

I tried a few approaches but couldn't figure out how to still get that correct negative value. Suggestion welcome! My workaround was to make stringBounds(_ string: String, _ index: Int) take instead an index: Int? and use generic wording in the error if no good index could be provided, but that's worse behaviour than what it did before...

nicklockwood commented 1 year ago

@nighthawk I'm not able to reproduce the crash (I guess it's only on macOS 13 and above?) but can you confirm whether this works as a solution?

    static func stringBounds(_ string: Substring, _ index: String.Index) -> AnyExpression.Error {
        var _string = string.base
        while index > _string.endIndex {
            // Double the length until it fits
            // TODO: is there a better solution for this?
            _string += _string
        }
        let offset = _string.distance(from: string.startIndex, to: index)
        return stringBounds(String(string), offset)
    }
nighthawk commented 1 year ago

Yep, the crash is on macOS 13 only, as far as I can tell. Your code fixes the crash, but the error messages all say that index 0 is invalid when it should be, say, -1 or -3.

nicklockwood commented 1 year ago

What if you replace let offset = _string.distance(from: string.startIndex, to: index) with

let offset = index >= string.startIndex ?
            _string.distance(from: string.startIndex, to: index) :
            -_string.distance(from: index, to: string.startIndex)
nighthawk commented 1 year ago

Nice. That's a winner. Submitted a PR for it.