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

Fix Swift 4 ambiguity #39

Merged
merged 2 commits into from Dec 13, 2017

Conversation

Projects
None yet
7 participants
@tkohout
Contributor

tkohout commented Jul 18, 2017

Latest Xcode 9 beta 3 shows ambiguous use of curry when the swift 4 version flag is set for the project.

screen shot 2017-07-18 at 9 34 16 pm

This is likely related to changes in compiler implementation in swift 4 -
see more here: Swinject/SwinjectAutoregistration#29

The solution is to disambiguate by adding parentheses to functions with more than one parameter. As far as I tested this, it should be backward compatible with earlier versions of swift.

I would wait before merging this to see whether it is a bug of compiler in the beta or a real change.

@@ -39,11 +43,11 @@ func curryDefinitionGenerator(arguments: Int) -> String {
let lowerFunctionArguments = inputParameters.map { "\($0.lowercased())" } // ["a", "b", "c"]
let returnType = genericParameters.last! // "D"
let functionArguments = "(\(commaConcat(inputParameters)))" // "(A, B, C)"
let functionArguments = extraParentheses("(\(commaConcat(inputParameters)))", when: inputParameters.count > 1) // "(A, B, C)"

This comment has been minimized.

@tonyd256

tonyd256 Jul 18, 2017

Contributor

what happens if there is only one argument? does it error out? could we skip this condition?

This comment has been minimized.

@tkohout

tkohout Jul 18, 2017

Contributor

yep, you get something like Cannot convert value of type '(String) -> Service1' to expected argument type '((_)) -> _'. There can't be extra parentheses for a single parameter.

@Andrewpk

This comment has been minimized.

Andrewpk commented Nov 6, 2017

Is there anything holding this up from getting merged?

@tonyd256

This comment has been minimized.

Contributor

tonyd256 commented Nov 6, 2017

Not sure who's maintaining this anymore ... @sharplet ?

@gfontenot

This comment has been minimized.

Collaborator

gfontenot commented Nov 6, 2017

It's surprising to me that this works at all, beyond that it's needed. My understanding was that Swift moved away from letting you pass tuples to functions that take multiple arguments, and yet this seems to be exactly what this is doing?

@mokagio mokagio referenced this pull request Nov 8, 2017

Closed

Swift 4 support #40

@wongzigii

This comment has been minimized.

wongzigii commented Dec 11, 2017

Bump

@johnnysay

This comment has been minimized.

Contributor

johnnysay commented Dec 13, 2017

Any clue about when this will be merged?

@gfontenot

This comment has been minimized.

Collaborator

gfontenot commented Dec 13, 2017

I'll get this into a release today.

@gfontenot gfontenot merged commit bf0d347 into thoughtbot:master Dec 13, 2017

@@ -454,7 +454,7 @@
ONLY_ACTIVE_ARCH = YES;
SDKROOT = iphoneos;
SWIFT_OPTIMIZATION_LEVEL = "-Onone";
SWIFT_VERSION = 3.0;
SWIFT_VERSION = 4.0;

This comment has been minimized.

@gfontenot

gfontenot Dec 13, 2017

Collaborator

Does this mean we should do a major version bump? @sharplet I feel like you know more about this compatibility than I do.

This comment has been minimized.

@sharplet

sharplet Dec 13, 2017

Contributor

Yeah I'm pretty sure this would break compilation on Xcode 8.x

This comment has been minimized.

@gfontenot

gfontenot Dec 13, 2017

Collaborator

😞 sigh that's what I was afraid of. Curry 4.0 here we come!

@gfontenot

This comment has been minimized.

Collaborator

gfontenot commented Dec 13, 2017

Curry 4.0 is now released, which includes this change and so should fix compatibility with Swift 4.

@gfontenot

This comment has been minimized.

Collaborator

gfontenot commented Dec 13, 2017

Sorry for the delay on this btw, and thank you for your PR and your patience.

@johnnysay

This comment has been minimized.

Contributor

johnnysay commented Dec 14, 2017

Thank you @gfontenot for your fast reactivity!
However when running pod install / pod update I still have the version 3.0.0.
I have also noticed that in https://github.com/thoughtbot/Curry/blob/master/Curry.podspec the version is still 3.0.0.

@gfontenot

This comment has been minimized.

Collaborator

gfontenot commented Dec 14, 2017

d'oh! Thanks. I'll get that pushed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment