IntrepidPursuits / swift-wisdom

A collection of additions to the Swift Standard Library created by Intrepid Pursuits developers
MIT License
39 stars 14 forks source link

Allow the same version to compile in both Xcode 9 and 10. #156

Closed nivektric closed 6 years ago

nivektric commented 6 years ago

We have a client with a dependency on swift-wisdom that would like to be able to compile their codebase in both Xcode 9 and 10 simultaneously while they transition.

This conditionally compiles a handful of methods based on either Xcode 10's version of Swift 4.1 (4.1.50) or the existing 4.1.

In Swift 4.1 classic, a separate method signature is required for CountableRange because it is its own type.

In Swift 4.1.50, CountableRange is aliased to Range, so a separate method causes a compile time issue.

Unfortunately there's no way to specify only the additional method in Swift 4.1 classic because the swift version selector only accepts >, not <, which results in a small amount of duplicated code.

bobgilmore commented 6 years ago

Ah, we experimented with similar changes in PR #155 and #153 , but never (as far as I remember) hit upon the existence of Swift 4.1.50, and therefore never could get it to work right.

I think that this change is good, but we'll want to update Intrepid.podspec to either 0.10.3 (my preferred option) or to 0.11.0. Worth discussing with the other code reviewers.

I'm also taking the liberty of adding @jpmendel to the Reviewers list because of his work on #155

bobgilmore commented 6 years ago

Oh, also need to update the "Swift Versions" section of README.md

jpmendel commented 6 years ago

Yeah, that sounds good to me. Version 0.10.3 can be the one that can compile in XC 9 and 10, and then we can move to 0.11.0 for full Swift 4.2 support.

aspitz-intrepid commented 6 years ago

swift(>=4.1.50) definitely looks like the key to the earlier problems we encountered trying to solve this. Have you seen this - https://github.com/apple/swift-evolution/blob/master/proposals/0212-compiler-version-directive.md . In the Motivation section it looks like you'd need to use #if swift(>=4.1.50) || (swift(>=3.4) && !swift(>=4.0)) if we have any concerns that someone might try to use the Swift 4 compiler in version 3 compatibility mode. Also note the new compiler directive. I don't think we need to worry about it here but it looks like it was implemented in Swift 4.2

Kevin-Monahan-Intrepid commented 6 years ago

@aspitz-intrepid Good catch. Will update.

Kevin-Monahan-Intrepid commented 6 years ago

@aspitz-intrepid Actually, we do explicitly state which Pod versions are compatible with which Swift versions in the README, and 0.9.0 and above does not support Swift 3.

Given that this will (hopefully) be a very short-lived change, I'd like to avoid over-complicating it. Any objections to just leaving it as is with a check only for swift(>=4.1.50)?

Kevin-Monahan-Intrepid commented 6 years ago

@bobgilmore I bumped the podspec version but didn't update the README because this change doesn't actually affect Swift version compatibility. Let me know if I'm misinterpreting something.

bobgilmore commented 6 years ago

@nivektric I'd like to somehow indicate that 0.10.3 is the one to add Swift 4.1.50 (Xcode 10) support. So maybe something like...

Swift 4.1.50 (Xcode 10 Compatible) -> 0.10.3
Swift 4.0 -> 0.9.0 through 0.10.x
Swift 3.2 (Xcode 9 Compatible) -> 0.8.3 through 0.8.4
Swift 3 -> 0.6.1 through 0.8.1
Swift 2 -> anything through 0.5.2
ghost commented 6 years ago
1 Message
:book: Executed 174 tests, with 0 failures (0 unexpected) in 0.572 (0.746) seconds

Generated by :no_entry_sign: Danger

nivektric commented 6 years ago

@bobgilmore @jpmendel @aspitz-intrepid Any other comments? If not, could I get some approvals?