Yams: add an alias to explicitly dispatch pow
#281
Conversation
We must disambiguate `Double.pow(_: Double, _: Double) -> Double` and `__C.pow(_: Double, _: Double) -> Double` as the Swift for TensorFlow branch adds the former into the standard library. The explicit module dispatch ensures that we make the overload resolution unambiguous allowing building Yams.
| #if os(iOS) || os(macOS) || os(watchOS) || os(tvOS) | ||
| import Darwin | ||
| fileprivate let cpow: (_: Double, _: Double) -> Double = Darwin.pow | ||
| #elseif os(Windows) | ||
| import ucrt | ||
| fileprivate let cpow: (_: Double, _: Double) -> Double = ucrt.pow | ||
| #else | ||
| import Glibc | ||
| fileprivate let cpow: (_: Double, _: Double) -> Double = Glibc.pow | ||
| #endif |
There was a problem hiding this comment.
Can this be simplified to this?
#if canImport(Darwin)
private let cpow: (_: Double, _: Double) -> Double = Darwin.pow
#elseif canImport(ucrt)
private let cpow: (_: Double, _: Double) -> Double = ucrt.pow
#elseif canImport(Glibc)
private let cpow: (_: Double, _: Double) -> Double = Glibc.pow
#else
#error("Unsupported platform")
#endifThere was a problem hiding this comment.
I don't think that is really a simplification - it's different semantics. If I were to have a C module or a Swift module with the name Darwin on Windows, this would cause a build failure. I can certainly change it to that if you feel strongly though.
There was a problem hiding this comment.
I don't feel strongly, and see the risk of name collisions, although I would advise against naming new modules Darwin for a slew of reasons. I consider it a simplification in the sense that if new platforms were introduced (e.g. os(HomePod)) we wouldn't have to touch this code.
There was a problem hiding this comment.
Sure, but I still think that introduction of an os(Darwin) or vendor(Apple) is better. However, that's not the current reality. I'll update this in a few minutes.
There was a problem hiding this comment.
No need to update, I'll merge as-is.
|
I'd love to add Swift for TensorFlow and Windows CI jobs so we can catch these kinds of issues more proactively. @compnerd are you aware of any good examples of open source Swift projects using those platforms on CI that I could take a look at as a jumping off point? |
|
Sure, the question is what are you looking for in terms of Windows CI? Azure Pipelines or GitHub Actions? I'll try to get a recent Windows TensorFlow toolchain uploaded. |
|
Lovely, we use Azure Pipelines for this project, so this looks like a good start: https://github.com/compnerd/swift-win32/blob/master/azure-pipelines.yml |
We must disambiguate
Double.pow(_: Double, _: Double) -> Doubleand__C.pow(_: Double, _: Double) -> Doubleas the Swift for TensorFlowbranch adds the former into the standard library. The explicit module
dispatch ensures that we make the overload resolution unambiguous
allowing building Yams.