AliSoftware/Dip

register function is broken in Swift 4

jmartinesp opened this issue · 12 comments

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.

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

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.

@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.

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.

@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.

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

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

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?

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.

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

So far so good.

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.

I get this error using XCode 9 GM