osteslag / Changeset

Minimal edits from one collection to another
MIT License
802 stars 46 forks source link

[Do not merge] Remove IndexDistance constraint on Changeset and Edit #41

Closed ole closed 7 years ago

ole commented 7 years ago

This removes the constraint on Collection.IndexDistance on Changeset and Edit. Conversions between integer types are a little annoying in Swift, especially in Swift 3. Before the revamp of the integer protocols in SE-0104, we have to go through IntMax to convert between integer types. We should be able to remove the toIntMax() calls when we drop Swift 3 support.

More importantly, while this code currently compiles, it crashes with this runtime error when running the unit tests:

GenericCache(0x114050798): cyclic metadata dependency detected, aborting

The same crash happens in Xcode 8.3.2 and Xcode 9b4 (Swift 3 mode; I haven't checked if it was fixed in Swift 4).

Related bugs:

The crash has something to do with nested generic type metadata. As suggested in one of the issues on JIRA, the crash goes away if we make the enum inside Edit indirect:

public struct Edit<C: Collection> where C.Iterator.Element: Equatable {
    ...

    indirect public enum Operation {
        ...
    }
    ...

I don't know if this is supposed to be a final fix or just a workaround and this issue will eventually be fixed.

Until then, it's probably not worth merging this since the IndexDistance == Int constraint is not a very big deal in practice. But I wanted a place to show you the changes that would be needed, that's why I opened this PR.

ole commented 7 years ago

👍 for hard-wrapping the commit message at 72. Love it.

I never do this for my own stuff, but I figured, let's be a nice open-source citizen every once in a while. 😜

ole commented 7 years ago

I forgot, here's a screenshot of the crash in Xcode 8.3.2 for posterity:

screen shot 2017-07-27 at 14 12 54