-
-
Notifications
You must be signed in to change notification settings - Fork 112
Swift 4 & bug fixes #5
base: master
Are you sure you want to change the base?
Conversation
@@ -176,7 +176,7 @@ public class Fuse { | |||
|
|||
// Initialize the bit array | |||
var bitArr = [Int](repeating: 0, count: finish + 2) | |||
bitArr[finish + 1] = (1 << i) - 1 | |||
bitArr[finish + 1] = (1 << i) &- 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the fix to stop Swift from trapping from the integer overflow that occurs when the text input is longer than the Int
type can hold (64 characters on 64 bit devices).
I have added a test testItHandlesLongInputs()
which failed before making this change.
@@ -74,7 +74,7 @@ public class Fuse { | |||
/// - Returns: A tuple containing pattern metadata | |||
public func createPattern (from aString: String) -> Pattern? { | |||
let pattern = self.isCaseSensitive ? aString : aString.lowercased() | |||
let len = pattern.characters.count | |||
let len = min(pattern.count, maxPatternLength) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My miss, but the idea is that when the length of the pattern exceeds maxPatternLength
, it should error out.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can make this change, would you rather it return nil in this case or to throw an error? I suspect returning nil is the better option here as changing the signature of this function to func createPattern(from aString: String) throws -> Pattern?
would be a breaking change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point. Returning nil
would be ideal then.
I have updated the project to compile with no warnings with Xcode 9.2 using Swift 4. The biggest change from this was removing the 'characters' view from a string which was deprecated.
I have also fixed the following two issues I found from my usage:
When searching through a long input (>100 characters) the library would trap from an integer overflow occurring in Fuse.swift, line 179. I added a test confirming this behaviour, and fixed it using the overflow safe operator.
maxPatternLength
was declared as a property but never actually used, making the search very slow for long inputs. I have used it when creating the pattern tuple in Fuse.swift, line 77.