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

Fix Swift 4 ambiguity #39

Merged
merged 2 commits into from Dec 13, 2017
Merged

Fix Swift 4 ambiguity #39

merged 2 commits into from Dec 13, 2017

Conversation

tkohout
Copy link
Contributor

@tkohout 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)"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link

Andrewpk commented Nov 6, 2017

Is there anything holding this up from getting merged?

@tonyd256
Copy link
Contributor

tonyd256 commented Nov 6, 2017

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

@gfontenot
Copy link
Collaborator

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 mentioned this pull request Nov 8, 2017
@wongzigii
Copy link

Bump

@johnnysay
Copy link
Contributor

Any clue about when this will be merged?

@gfontenot
Copy link
Collaborator

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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@gfontenot
Copy link
Collaborator

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

@gfontenot
Copy link
Collaborator

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

@johnnysay
Copy link
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
Copy link
Collaborator

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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants