Overload ambiguities should not be resolved via ranking

Happy new year everyone!

I stumbled across this in an XCode 15.1 playground and got a kick out of it:

func test() -> Int32 { 1 }
func test() -> Int64 { 2 }

UInt32(test()) // Ambiguous use of 'test()'

Which makes sense, but then if I add another overload:

func test() -> String { "3" }
UInt32(test()) // 3

Any ideas on what's going on here?

1 Like

UInt32 has an initializer which takes a string and parses it. There is a single test overload which returns a string so it selects that one and parses the return value of test.

1 Like

But UInt32 also has:

    @inlinable public init<T>(_ source: T) where T : BinaryInteger

Why does the String initializer have precedence over this one, which would otherwise cause an ambiguity?

1 Like

It’s not generic. I’m not convinced that’s a good rule to use in overload resolution by itself, but it’s the rule this is using.

5 Likes

The plot thickens:

func test() -> Int32 { 1 }
func test() -> Int64 { 2 }
func test() -> String { "3" }
func test() -> Float32 { 4.0 }
func test() -> Float64 { 5.0 }

UInt32(test()) // Ambiguous use of 'test()'

In this case, even though the String initializer can go through, the ambiguity is still raised because the float initializers are conflicting

Edit:

this is true even if the Int overloads are removed; it feels like the the overloads are considered in some compilation order and the first match wins.

The (String) -> UInt32 initializer is sourced from a FixedWidthInteger extension whereas the (Float) -> UInt32 and (Double) -> UInt32 initializers are defined on UInt32 itself. This is another rule for declaration ranking: protocol extension members are 'worse' than protocol requirements, which are 'worse' than members of concrete types (all of these happen after the 'is generic' comparison that John mentions above, though).

The full logic for comparing two declarations can be found here.

3 Likes

Other than the quadratic matching, is there any other reason why ranks are used at all?

While the provided example was somewhat silly, there is a legitimate use case for this as a Sqlite wrapper:

/* All data types supported by sqlite (Int32, Int64, Double, String, UnsafeRawBufferPointer) as
 well as their optionals implement this protocol in order to allow border crossing. */
public protocol SqliteRepresentable {
    @inlinable func bind(to: Sqlite.Statement, at: Int32) throws
}

/* Wraps a prepared statement and offers means to run it with optional parameters, iterate
 the returned results or query the number of affected rows. */
public struct Statement {
    @usableFromInline let statement: OpaquePointer
    @inline(__always) fileprivate init(_ stmt: OpaquePointer) { statement = stmt }
    @usableFromInline func run<T: Sequence<SqliteRepresentable>, R>(_ args: T, do cb: (inout Int32) throws -> R) throws -> R {
        var i = Int32(1); for arg in args {
            try arg.bind(to: self, at: i)
            i += 1
        }
        defer { sqlite3_reset(statement) }
        var code = sqlite3_step(statement)
        let res = try cb(&code)
        guard code == SQLITE_DONE else { throw Error(rawValue: code) }
        return res
    }

    @inlinable public subscript(_ col: Int32) -> Bytes! {
        guard let ptr = sqlite3_column_blob(statement, col) else { return nil }
        return Bytes(start: ptr, count: Int(sqlite3_column_bytes(statement, col)))
    }
    @inlinable public subscript(_ col: Int32) -> Int32! {
        SQLITE_NULL == sqlite3_column_type(statement, col) ? nil : sqlite3_column_int(statement, col)
    }
    @inlinable public subscript(_ col: Int32) -> Int64! {
        SQLITE_NULL == sqlite3_column_type(statement, col) ? nil : sqlite3_column_int64(statement, col)
    }
    @inlinable public subscript(_ col: Int32) -> Double! {
        SQLITE_NULL == sqlite3_column_type(statement, col) ? nil : sqlite3_column_double(statement, col)
    }
    @inlinable public subscript(_ col: Int32) -> String! {
        guard let ptr = sqlite3_column_text(statement, col) else { return nil }
        return String(cString: ptr)
    }

    /* Binds the first numbered `args` of the query to the provided values, runs the
     statement and returns the number of affected rows. */
    @inlinable @discardableResult public func callAsFunction(_ args: SqliteRepresentable...) throws -> Int32 {
        try run(args) { code in sqlite3_changes(sqlite3_db_handle(statement)) }
    }

    /* Binds the first numbered `args` of the query to the provided values, runs the
      statement and invokes `do` for each row. */
    @inlinable public func callAsFunction(_ args: SqliteRepresentable..., do cb: () throws -> ()) throws {
        try run(args) { code in
            while code == SQLITE_ROW {
                do { try cb() }
                catch Error.DONE { return }
                code = sqlite3_step(statement)
            }
        }
    }
}

Usage:

let query = "select id, name from users where email = ?"
query("bob@example.com") {
  print(query[0] as Int64, query[1] as String)
}

It might not be the sanest interface out there, but I figure that there's usually enough context to make this type safe. At one point I was casting a column to UInt32 without an as Int64 qualifier and was expecting a compiler error which I never got.

At the time I only considered that I wanted to use the higher bit of Int32 as unsigned, but then I went through:

  1. this might be safe overall, but will generate an unnecessary itoa & atoi pair into
  2. this converts to a double, then back to an int which is fine in the UInt32 case but then:
func test() -> Int64 { Int64.max }
func test() -> Float64 { Float64(Int64.max) }

Int64(test() as Int64) // okay
Int64(test()) // crash due to rounding error, needs `.nextDown`

I'd say it's bad that when you added "func test() -> String" it suddenly compiles. IMHO if there are two or more overloads (regardless of whether they are defined in a type itself or in an extension or in a protocol extension) it should be resolved as ambiguous situation and not compile. This is very error prone:

func test() -> Int32 { 1 }
func test() -> Int64 { 2 }
...
func test() -> String { "3" }
...
UInt32(test()) // 3. oops, not the one I wanted
2 Likes

Due to Swift's lookup model (both the fact that adding protocol conformances can add members on conforming types, and that arbitrary modules can extend types/protocols to add members) I think such a harsh policy would be pretty difficult to work with in practice. It would just be too hard to ensure that downstream clients don't end up with conflicting declarations without doing something like Objective-C style prefixing on every method call. Of course, if we had a good way to fully qualify member names for lookup then this concern could be mitigated.

Using explicit "as" cast helps with ambiguity:

func test() -> Int32 { 1 }
func test() -> Int64 { 2 }
func test() -> String { "3" }
func test() -> Float32 { 4.0 }
func test() -> Float64 { 5.0 }

UInt32(test() as Int32)

Sorry, for the shallowness of my previous reply, 300 lines of highly contextual code is a quite a bit to... parse.

Bad puns aside it seems that, since you're using a constraint solver to resolve ambiguities, this is permitted by one of the early exits before 531

I'm certain that they all have significant performance implications but I believe that this case warrants its own logic. Without a full understanding of compiler internals, I'm afraid I'm not qualified to say what that should be, just that it should be there.

Directly extending Int64 does indeed make this safe but...

func test() -> Int64 { Int64.max }
func test() -> Float64 { Float64(Int64.max) }

extension Int64 {
    init(_ val: Int64) { self.init(truncatingIfNeeded: val) }
}

Int64(test()) // Ambiguous use of 'test()'

... I believe that this requires too much arcane knowledge from a casual developer using a supposedly safe language.

If the argument for protocol extension members are 'worse' than (...) members of concrete types is to allow for a default implementation, then it is also an argument for explicitly adding init(_ val: [U]IntN) to every other [U]IntN in the standard library, as the specialized implementation may be (and in this case is) lossy.

The problem here doesn't arise with the initializers themselves; it's with the simultaneous use of test(), a function overloaded by return value. This is a technically supported feature of Swift but strongly advised against for pretty much the reasons you've encountered here—namely, unexpected overloads will be chosen or ambiguities arise which the user cannot reason about. This isn't a problem which in the general case can be patched around by attempting to add tie-breaking overloads all over the place: making one scenario "just work" will inevitably break some other code.

The core team has previously stated that they will never overload functions solely by return value in the standard library. To accomplish something like your SQLite example, use an explicit parameter (which can be given a default value if you wish)—e.g., query[0, as: Int64.self].

1 Like

I agree

I then rest my case.