Segmentation fault when overloading sorted()

The following code results in segmentation fault 11 on Xcode Version 11.4.1 (11E503a):

protocol OrderedCollection: RandomAccessCollection, MutableCollection where Element: Comparable {
	@inlinable func sorted(by areInIncreasingOrder: (Element, Element) throws -> Bool) rethrows -> Self
	@inlinable func sorted() -> Self
}

extension OrderedCollection {
	@inlinable func sorted(by areInIncreasingOrder: (Element, Element) throws -> Bool) rethrows -> Self {
		var newOrdredCollection = self
		try newOrdredCollection.sort(by: areInIncreasingOrder)
		return newOrdredCollection
	}
	
	@inlinable func sorted() -> Self {
		return sorted(by: <)
	}
}

Expanding sorted(by:) in sorted() has the same result:

protocol OrderedCollection: RandomAccessCollection, MutableCollection where Element: Comparable {
	@inlinable func sorted() -> Self
}

extension OrderedCollection {
	@inlinable func sorted() -> Self {
		var newOrdredCollection = self
		try newOrdredCollection.sort(by: <)
		return newOrdredCollection
	}
}

The code compiles fine so long as there is no sorted() implementation:

protocol OrderedCollection: RandomAccessCollection, MutableCollection where Element: Comparable {
	@inlinable func sorted(by areInIncreasingOrder: (Element, Element) throws -> Bool) rethrows -> Self
	@inlinable func sorted() -> Self
}

extension OrderedCollection {
	@inlinable func sorted(by areInIncreasingOrder: (Element, Element) throws -> Bool) rethrows -> Self {
		var newOrdredCollection = self
		try newOrdredCollection.sort(by: areInIncreasingOrder)
		return newOrdredCollection
	}
}

I want to check with the community and see if it's because of an error in my code that caused the problem, before submitting a bug report.

If the compiler is segfaulting, that's a bug regardless of whether the code you've written is correct or not, so please do file a bug report even if you figure out a workaround. :slight_smile:

3 Likes

just reported: https://bugs.swift.org/browse/SR-12751

2 Likes

Is adding "@inlinable" to a protocol method declaration legal? Or useful?

Is it sane for a conforming type to independently define sorted()? Or to make it differ from a call to self.sorted(<)? If the answer to either is "no," then remove it as a customization point.

Defining sorted() in either the protocol definition or an unconditional extension isn't right anyway. An Element type isn't necessarily Comparable, so the call to < may be nonsensical.

Even the most nonsensical input should not crash the compiler.

1 Like

The standard library's sort() and sorted(by:) are declared and implemented as @inlinable in Sequence. They live in Sort.swift along with other sorting functions.

It's also pointed out by this comment to the bug report that the bug is fixed. The code snippet I posted here and in the bug report compiles without errors using swift-DEVELOPMENT-SNAPSHOT-2020-05-05-a-osx (← this is a download link).

So, it should be legal, as far as I can tell.

I don't really know how useful it is. Inline expansions are supposed to reduce the overhead of calling functions, but I don't know what the overall effect is for this particular example.

These are the (signatures of?) standard library's sorted() and sorted(by:):

@inlinable public func sorted() -> [Element]

@inlinable public func sorted(
    by areInIncreasingOrder: (Element, Element) throws -> Bool
) rethrows -> [Element]

Note that both of them return an instance of Array<Element>, not Self<Element>. So without overloading, self.sorted(by: <) returns Array<Element>, instead of Self<Element>. This makes it very awkward and inconvenient when you have a custom type that conforms to both MutableCollection and Sequence. Since MutableCollection's sort() and sort(by:) are mutable (think of it like returning a Self<Element> as self), it only makes sense if sorted() and sorted(by:) are consistent with this behaviour.

Similar overloadings happen in the standard library too. Compare methods in SetAlgebra and how they are overloaded in Set, for example.

You're correct. I fixed it in the code pasted in the bug report, but I forgot to fix it here. I'm fixing it now.