Why is there a Collection.flatMap that takes a closure returning String?

Just curious…

I know this region of the API is changing in 4.1, but I’m also seeing it in Xcode 9.2 as well.

Most versions of flatMap() and compactMap() are written in terms of a generic return type for the closure, variously termed U, SegmentOfResult, or ElementOfResult, which makes sense.

But there’s also this:

extension Collection {

    public func compactMap(_ transform: (Self.Element) throws -> String?) rethrows -> [String]

    @available(*, deprecated, renamed: "compactMap(_:)")
    public func flatMap(_ transform: (Self.Element) throws -> String?) rethrows -> [String]
}

This seems weird. It leaks into the documentation, into code completion, and into type inference, leading to oddball error messages complaining that ambiguous return types can’t be converted to String?.

1 Like

Good question, I always thought it was a bug when the compiler showed me that my closure is ambigious with a returning type String (this happens a lot when I write complex chains in RxSwift). So this weird overload was the case. Why does it even exist?

CC @Ben_Cohen @moiseev

Looks like this PR introduces it along with rationale: https://github.com/apple/swift/pull/9466

2 Likes

Hmm, if it‘s source compatibility thing only can we hide it in Swift 5 mode? This overload caused much more pain than you might think. The compiler could not infer complex chains, which it should have, because it tried to convert the type to a String - a completely unrelated type from the given context.

I don’t remember exactly why I had to omit the @available attribute, but I had to do it, that’s for sure. Should not be too hard to figure out why, by adding it.

UPD I think it’s meant to support multi-statement closures and how they are type-checked.

[0, 1].flatMap { x in
  if String(x) == "foo" {
    return "bar"
  }
  return nil
}

Without this overload the type-checker would prefer the flatMap returning String and then fail on return nil as it’s not a valid String value.

The original PR thread cites the following snippet as an example of the potential Swift 3 incompatibility that’s being worked around with the String? overload:

struct S { var p: String }

func f(xs: [S]) -> [String] {
   return xs.flatMap { $0.p }
}

Since Strings are now Collections, the String? overload is potentially needed to prevent flatMap from returning a flattened array of Characters.

But if I understand the details of flatMap() correctly (which I quite possibly do not), this seems like something of a straw man example. flatMap() wants a sequence of consistent types, and it unwraps only one level of sequencing. So if you were dealing with a sequence of arrays of strings—which I would imagine is a more typical application for flatMap here—it would continue to work in Swift 4 just exactly as it did in Swift 3, even without the String? overload.

let woids = [["one", "two", "three"], [], ["four"]]
woids.flatMap { $0 } // [ "one", "two", "three", "four" ]

If in Swift 3 you were calling flatMap() to process a simple sequence of Strings (as in the PR example above), you really shouldn’t have been; the intended result is no different from map() in that context.

There are clearly a lot of nuances here, but I can’t help thinking that the introduction of compactMap() really ought to help resolve all of this. In particular, why is the String? overload being dragged along into 4.1 for compactMap()? There’s no legacy code that calls compactMap, and strings-as-collections shouldn’t be an issue either since compactMap() does not expand sequences.

If you replace flatMap() with map() in this example, the compiler doesn’t know what to make of it and gives a pretty clear description of the problem:

// ERROR: Unable to infer complex closure return type; add explicit type to disambiguate
let x = [0, 1].map { x in
    if String(x) == "foo" {
        return "bar"
    }
    return nil
}

That seems reasonable. Why shouldn’t flatMap() and compactMap() behave identically to map() in this regard? It’s not really a flatMap issue so much as a gap in the current type inference system. Under current rules, this closure requires an explicit return-type declaration.

Special handling for String leads directly to problems with an Int version of this same code (or at least, I’d argue that the error message is problematic):

let x = [0, 1].flatMap { x in
    if x == 1 {
        // ERROR: Cannot convert return expression of type 'Int' to return type 'String?'
        return 1
    }
    return nil
}
1 Like

This overload ist pure nonsense and should be removed. The flatMap or compactMap should stay purely generic. The overload for String does not justify the general problem.

[0, 1].flatMap { x in
  if let y = Double(exactly: x) {
    return y + 42 // Cannot convert return expression of type 'Double' to return type 'String?'
  }
  return nil
}

See that we didn’t even use a string anywhere in the context of your trailing closure?! The diagnostic is not only completely misleading but is simply a bug caused by this overload. If the compiler isn’t smart enough to infer the Optional type from the context of the complex trailing closure then you have to be explicit yourself instead of a global overload in the standard library which simply does more harm than good.

let doubles: [Double] = [0, 1].flatMap { x in
  if let y = Double(exactly: x) {
    return y + 42
  }
  return nil
}
1 Like

Agreed. I got to the same conclusion yesterday.

Not at all straw man. It is, in fact, a quite common misuse of flatMap that I found, for example, in the SwiftPM codebase.

But they did, and it used to compile in Swift 3.

Because flatMap has a long history of mis-uses, but you’re right, it should be possible to clean some of this mess now that we have compactMap.

https://bugs.swift.org/browse/SR-7052

PRs are more than welcome.

Oh well, I guess you knew it all along, but it turned out that only this one overload needed to be purged:


Let's see what the tests show.

1 Like

Is this considered as a starter bug? What would be required to fix the issue, deprecation or simple remove? I’d love to try to solve it as part of the challenge to set up the whole swift project for contribution.

Haha, I was to slow. Well done :) Are there no more weird String overloads in the stdlib?

Oops. Sorry. I felt ashamed of it enough that I decided to take care of it myself.

It’s totally fine, I’ll learn from your PR on how one would have tackled it. 👍

This should not be a challenge! In fact, if you think it is, the great contribution would be to streamline it, or at least improve the set-up documentation.

I meant it’s probably a challenge to set up the whole Swift project for the first time so that it compiles just fine. Could be tricky the first time. At least for someone like me who have not made a single PR to the project yet.

WRT the size of the standard library and the # of functions like this, could we split the backwards compatibility shims into separate libraries? So if we only say “import Swift”, we get latest version of the standard lib (whatever it happens to be), and of your code needs Swift3 API stuff, you’d need to explicitly “import Swift3”?

The syntax would need work (because why only solve one problem when we could address the general case of “depending on multiple versions of the same library within a single target” at the same time), but as a concept, is it worth exploring?