Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for newest Swift beta #407

Merged
merged 11 commits into from Sep 9, 2016
Merged

Updates for newest Swift beta #407

merged 11 commits into from Sep 9, 2016

Conversation

@gfontenot
Copy link
Collaborator

gfontenot commented Aug 25, 2016

This is currently hanging my compiler, which is fun. I'll come back to this,
but this is at least moving us in the right direction of being able to support
the most recent beta of Swift 3.

@@ -2,7 +2,7 @@ import Foundation

/// A type safe representation of JSON.
public enum JSON {
case object([String: JSON])
case object([AnyHashable: JSON])

This comment has been minimized.

Copy link
@jshier

jshier Aug 25, 2016

Contributor

I don't think this is necessary as JSON objects must have String keys. [String : Any should work fine (as it does in my port).

This comment has been minimized.

Copy link
@gfontenot

gfontenot Aug 26, 2016

Author Collaborator

Ah, good point. I made this change because I was getting an EXC_BAD_ACCESS crash when trying to run the tests. I'll see if I can work around that a different way.

self = .array(v.map(JSON.init))

case let v as [String: AnyObject]:
case let v as [AnyHashable: Any]:

This comment has been minimized.

Copy link
@jshier

jshier Aug 25, 2016

Contributor

Same as above, I don't think the keys need to be AnyHashable, just String.

@@ -3,7 +3,7 @@ import Argo

class SwiftDictionaryDecodingTests: XCTestCase {
func testDecodingAllTypesFromSwiftDictionary() {
let typesDict = [
let typesDict: [AnyHashable: Any] = [

This comment has been minimized.

Copy link
@jshier

jshier Aug 25, 2016

Contributor

A String key works here as well.

This is now the default, and the compiler will actually warn us if it _can't_
be used. So we can safely remove all of these instances of `@noescape` without
any functional change.
@gfontenot gfontenot force-pushed the gf-updates branch from 96f99d0 to 6c8a6e6 Aug 26, 2016
@gfontenot

This comment has been minimized.

Copy link
Collaborator Author

gfontenot commented Aug 26, 2016

I'm unclear on if performance took a hit on this (for whatever reason). If someone that has a better idea of where the performance tests should be sitting could run the tests that'd be much appreciated.

@gfontenot

This comment has been minimized.

Copy link
Collaborator Author

gfontenot commented Aug 26, 2016

I want to make another run at the precedence group after doing the same for Runes, so this isn't completely finished yet.

@gfontenot gfontenot changed the title [WIP] Updates for newest Swift beta Updates for newest Swift beta Aug 29, 2016
@gfontenot

This comment has been minimized.

Copy link
Collaborator Author

gfontenot commented Aug 29, 2016

ping @thoughtbot/ios I think this is ready for review now.

@tonyd256

This comment has been minimized.

Copy link
Contributor

tonyd256 commented Sep 6, 2016

👍

gfontenot added 10 commits Aug 25, 2016
This has been deprecated, and the where clause actually needs to be part of
the larger function definition now.
This doesn't happen implicitly anymore.
This has been renamed in Swift 3.0.
id is now imported as Any instead of AnyObject. This means that we'll need to
update our API to match.
We're defining this in relation to the operators defined in Runes.
Specifically, we want this to have a higher precedence than any of those
operators. That means dropping this just above ApplicativeSequence.

Note that because of the precedence change wrt <|>, we now need to add parens
to get it all to compile properly.
We don't care about these. They are horribly noisy.
This makes sure we're nesting the decode operators between Applicative
Sequence and nil coalescing. Seems like a reasonable thing to do.
Turns out, these need to be globally unique! Fun stuff. To work around this we
can fall back to our good old friend "Explicit Prefix"! We'll just tack on a
quick package name to the front of this precedence group and voila! No
conflicts!

![photo of a fire in a dumpster which accurately reflects my feelings on this subject](http://languagelog.ldc.upenn.edu/myl/DumpsterFire2.jpg)
@gfontenot gfontenot force-pushed the gf-updates branch from aba6caf to ccf74e0 Sep 6, 2016
@aschuch

This comment has been minimized.

Copy link

aschuch commented Sep 8, 2016

Installing this branch using Carthage and the latest Xcode 8 GM, I am seeing the following build failure:

*** Building scheme "Argo-tvOS" in Argo.xcworkspace
** BUILD FAILED **


The following build commands failed:
    CompileSwift normal arm64
    CompileSwiftSources normal arm64 com.apple.xcode.tools.swift.compiler
(2 failures)
/REDACTED/Carthage/Checkouts/Argo/Argo/Types/Decoded/Monad.swift:1:8: error: no such module 'Runes'
A shell task (/usr/bin/xcrun xcodebuild -workspace /REDACTED/Carthage/Checkouts/Argo/Argo.xcworkspace -scheme Argo-tvOS -configuration Release -sdk appletvos ONLY_ACTIVE_ARCH=NO BITCODE_GENERATION_MODE=bitcode CODE_SIGNING_REQUIRED=NO CODE_SIGN_IDENTITY= CARTHAGE=YES clean build) failed with exit code 65:
** BUILD FAILED **


The following build commands failed:
    CompileSwift normal arm64
    CompileSwiftSources normal arm64 com.apple.xcode.tools.swift.compiler
(2 failures)
@gfontenot

This comment has been minimized.

Copy link
Collaborator Author

gfontenot commented Sep 8, 2016

hmm, that doesn't seem right. Building locally seems fine to me.

❯ carthage build --no-skip-current
*** xcodebuild output can be found in /var/folders/h7/g6pn0vy96r3dbrlj_8ytgpym0000gn/T/carthage-xcodebuild.1SGBK0.log
*** Building scheme "Curry-watchOS" in Curry.xcodeproj
*** Building scheme "Curry-Mac" in Curry.xcodeproj
*** Building scheme "Curry-tvOS" in Curry.xcodeproj
*** Building scheme "Curry-iOS" in Curry.xcodeproj
*** Building scheme "Runes-Mac" in Runes.xcodeproj
*** Building scheme "Runes-iOS" in Runes.xcodeproj
*** Building scheme "Runes-tvOS" in Runes.xcodeproj
*** Building scheme "Runes-watchOS" in Runes.xcodeproj
*** Building scheme "Argo-watchOS" in Argo.xcworkspace
*** Building scheme "Argo-tvOS" in Argo.xcworkspace
*** Building scheme "Argo-Mac" in Argo.xcworkspace
*** Building scheme "Argo-iOS" in Argo.xcworkspace
@aschuch

This comment has been minimized.

Copy link

aschuch commented Sep 9, 2016

Hm, this is super strange. I've just cloned again and tried to build locally and the error persists.

➜ git clone  git@github.com:thoughtbot/Argo.git
Cloning into 'Argo'...
➜ cd Argo 
(master|✔)➜ git co gf-updates 
(gf-updates|✔)➜ carthage version
0.17.2
(gf-updates|✔)➜ carthage build --no-skip-current
*** xcodebuild output can be found in /var/folders/80/316467jj2czbnz4cmvt0nw800000gn/T/carthage-xcodebuild.YbgLDL.log
*** Building scheme "Argo-Mac" in Argo.xcworkspace
** BUILD FAILED **

The following build commands failed:
    CompileSwift normal x86_64
    CompileSwiftSources normal x86_64 com.apple.xcode.tools.swift.compiler
(2 failures)
/REDACTED/Argo/Types/Decoded/Applicative.swift:1:8: error: no such module 'Runes'
A shell task (/usr/bin/xcrun xcodebuild -workspace /REDACTED/Argo.xcworkspace -scheme Argo-Mac -configuration Release ONLY_ACTIVE_ARCH=NO CODE_SIGNING_REQUIRED=NO CODE_SIGN_IDENTITY= CARTHAGE=YES clean build) failed with exit code 65:
** BUILD FAILED **

The following build commands failed:
    CompileSwift normal x86_64
    CompileSwiftSources normal x86_64 com.apple.xcode.tools.swift.compiler
(2 failures)

However, if I run carthage update before carthage build --no-skip-current, everything works as expected.

(gf-updates|✔)➜ carthage update                 
*** Fetching Curry
*** Fetching Runes
*** Checking out Curry at "408134ffa8dd1c09b0050f65eae300381b934bf6"
*** Checking out Runes at "e00cf681ec03a2dbf2b4cd6dd1e24d522f55474b"
*** xcodebuild output can be found in /var/folders/80/316467jj2czbnz4cmvt0nw800000gn/T/carthage-xcodebuild.NhTK7x.log
*** Building scheme "Curry-tvOS" in Curry.xcodeproj
*** Building scheme "Curry-Mac" in Curry.xcodeproj
*** Building scheme "Curry-watchOS" in Curry.xcodeproj
*** Building scheme "Curry-iOS" in Curry.xcodeproj
*** Building scheme "Runes-Mac" in Runes.xcodeproj
*** Building scheme "Runes-watchOS" in Runes.xcodeproj
*** Building scheme "Runes-iOS" in Runes.xcodeproj
*** Building scheme "Runes-tvOS" in Runes.xcodeproj
(gf-updates|✔)➜ carthage build --no-skip-current
*** xcodebuild output can be found in /var/folders/80/316467jj2czbnz4cmvt0nw800000gn/T/carthage-xcodebuild.xWjNYl.log
*** Building scheme "Curry-watchOS" in Curry.xcodeproj
*** Building scheme "Curry-tvOS" in Curry.xcodeproj
*** Building scheme "Curry-Mac" in Curry.xcodeproj
*** Building scheme "Curry-iOS" in Curry.xcodeproj
*** Building scheme "Runes-Mac" in Runes.xcodeproj
*** Building scheme "Runes-watchOS" in Runes.xcodeproj
*** Building scheme "Runes-tvOS" in Runes.xcodeproj
*** Building scheme "Runes-iOS" in Runes.xcodeproj
*** Building scheme "Argo-Mac" in Argo.xcworkspace
*** Building scheme "Argo-tvOS" in Argo.xcworkspace
*** Building scheme "Argo-watchOS" in Argo.xcworkspace
*** Building scheme "Argo-iOS" in Argo.xcworkspace
@gfontenot

This comment has been minimized.

Copy link
Collaborator Author

gfontenot commented Sep 9, 2016

It looks like you hadn't run carthage bootstrap (or bin/setup) to pull down the updated dependencies. The sha for Runes that you checked out after running carthage update is the same as the one listed in the Cartfile.resolved.

@gfontenot

This comment has been minimized.

Copy link
Collaborator Author

gfontenot commented Sep 9, 2016

Actually, I'm seeing this error too, but I don't think it has anything to do with this PR. I think we're not handling the Runes dependency properly and need to revisit that.

@gfontenot gfontenot merged commit ccf74e0 into master Sep 9, 2016
2 checks passed
2 checks passed
ci/circleci Your tests passed on CircleCI!
Details
hound No violations found. Woof!
@gfontenot gfontenot deleted the gf-updates branch Sep 9, 2016
@edwardaux edwardaux mentioned this pull request Sep 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.