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

register function is broken in Swift 4 #172

Closed
jmartinesp opened this issue Aug 11, 2017 · 12 comments
Closed

register function is broken in Swift 4 #172

jmartinesp opened this issue Aug 11, 2017 · 12 comments

Comments

@jmartinesp
Copy link

Hello, and thanks for Dip, I've been using it for quite a while and I love it so far.

Today I updated my project to Swift 4 and I found out that some container.register { ... } calls cannot be resolved by the compiler.

container.register { TestClass($0) } // Works fine
container.register { TestClass() } // Nope

The error shown is Ambiguous use of 'register(_:type:tag:factory:)'. I guess because of this change, when the register function is used and no argument is ever used inside the closure, the compiler won't be able to choose between all the 0, 1, 2, 3... args register functions in RuntimeArguments.swift.

As far as I know you still haven't updated the library to work with Swift 4, but I just wanted to report this so it's a known issue for future versions.

@ilyapuchka
Copy link
Collaborator

Yes, it's most likely related to this change, but the idea was that it should not break anything. That's sad.

@jmartinesp
Copy link
Author

I think I might have fixed it by wrapping the args in an extra pair of (), like this:

// Original
@discardableResult public func register<T, A, B>(_ scope: ComponentScope = .shared, type: T.Type = T.self, tag: DependencyTagConvertible? = nil, factory: @escaping (A, B) throws -> T) -> Definition<T, (A, B)> {
    return register(scope: scope, type: type, tag: tag, factory: factory, numberOfArguments: 2) { container, tag in try factory(container.resolve(tag: tag), container.resolve(tag: tag)) }
  }

// "Fixed"
@discardableResult public func register<T, A, B>(_ scope: ComponentScope = .shared, type: T.Type = T.self, tag: DependencyTagConvertible? = nil, factory: @escaping ((A, B)) throws -> T) -> Definition<T, (A, B)> {
    return register(scope: scope, type: type, tag: tag, factory: factory, numberOfArguments: 2) { container, tag in try factory((container.resolve(tag: tag), container.resolve(tag: tag))) }
  }

So far everything seems to be working fine, I will report if I find anything odd.

@kaosdg
Copy link

kaosdg commented Aug 30, 2017

@Arasthel have you gotten any further on this? I've done what you suggested, however calling register with no arguments still gives me the ambiguous reference error.

@jmartinesp
Copy link
Author

This is what mi RuntimeArguments file looks like: https://gist.github.com/Arasthel/38434fccf17e78fcf370f4724d86c2ff

It works fine for me, both with 0, 1 and several arguments.

@karlpuusepp
Copy link

@kaosdg I managed to get it working by only changing the single-argument overload from
(A) throws -> Void
to
((A)) throws -> Void.

I'd guess the compiler is not sure if Void works as A or not, but adding extra parentheses seems to resolve the ambiguity.

@ilyapuchka
Copy link
Collaborator

I don't think turning several factory arguments to one tuple argument is a good solution for that.

@kaosdg
Copy link

kaosdg commented Aug 30, 2017

About to put this aside for a few days, but here's where I am so far:

I can get it to the point where the framework compiles, but one unit test gives me trouble with the following test:
testThatItCallsResolvedDependenciesBlockWhenResolvingByTypeForwarding()

container.register { ServiceImp1() }

This one line fails with Ambiguous use of 'register(_:type:tag:factory:)' whereas all other calls similar to this compile without issues.
(my branch is here: https://github.com/kaosdg/Dip/tree/swift4.0 )

I followed a bit of the discussion on SE-0110 and saw this:
https://lists.swift.org/pipermail/swift-evolution-announce/2017-June/000386.html

which leads me to believe there may be some wonkiness with how the Swift 4 compiler is behaving at this point

@matkuznik
Copy link

Hi all. Do anyone found proper solution? I was able to successfully implement @rerelease's proposition and all tests pass.

@kaosdg you need to update one more register function, see this diff

I can agree with @ilyapuchka that this is not the best solution, but for now only one or not?

@karlpuusepp
Copy link

Since SE-0110 was deferred I would assume this is a compiler bug?

The ambiguity can be trivially reproduced in a playground.

func overload<T>(_ arg: () -> T) {

}

func overload<T, A>(_ arg: (A) -> T) {

}

overload { // compiler error here
    return 10
}

Again, wrapping (A) with extra parentheses fixes it.

This was referenced Sep 6, 2017
@stevenkramer
Copy link

I implemented @rerelease's fix in stevenkramer@34dd0a9

So far so good.

@ilyapuchka
Copy link
Collaborator

On Xcode 9 I don't see any compilation errors or tests failures, code snippet provided by @rerelease also does not result in compiler error. So I guess the breaking change in swift compiler was fixed.

@jonasman
Copy link

I get this error using XCode 9 GM

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

No branches or pull requests

7 participants