[SR-6501] RangeReplaceableCollection default implementations cause infinite recursion

[phausler]

Previous ID	SR-6501
Radar	None
Original Reporter	@phausler
Type	Bug
Status	Resolved
Resolution	Done

Additional Detail from JIRA

Votes	2
Component/s	Standard Library
Labels	Bug
Assignee	@glessard
Priority	Medium

md5: f4a01d143a13f79e49eb47ab0ef5ba7e

is duplicated by:

• SR-5208 Conformance to RangeReplaceableCollection permitted without implementation of replaceSubrange
• SR-5211 The base method implementation for RangeReplaceableCollection should use RangeExpression
• SR-10647 No error when conforming to RangeReplaceableCollection
• SR-11516 Simplest MutableDataProtocol generates initializer that spins forever
• SR-15360 Invalid conformance to RangeReplaceableCollection compiles and causes crash

Issue Description:

class Foo : Collection {
    required init() { }
    
    public var startIndex: Int {
        print("startIndex")
        return 0
    }
    
    public var endIndex: Int {
        print("endIndex")
        return 5
    }
    
    public func index(after index: Int) -> Int {
        print("indexAfter")
        return index + 1
    }
    
    public subscript(_ index: Int) -> Int {
        get { print("[get]"); return 42 }
        set { print("[set]") }
    }
}
extension Foo : RangeReplaceableCollection {}
let f = Foo()
f.append(5)

This results in the error

error: cannot use mutating member on immutable value: 'f' is a 'let' constant
f.append(5)

this is definitely deserving of a "waT?!"... but ok... lets roll with it...

changing from a let to a var then results in a stack overflow:

#&#8203;74859  0x000000010000693b in protocol witness for RangeReplaceableCollection.replaceSubrange<A>(_:with:) in conformance Foo ()
#&#8203;74860  0x000000010045ece5 in specialized RangeReplaceableCollection.replaceSubrange<A, B>(_:with:) ()
#&#8203;74861  0x00000001002cd3e8 in RangeReplaceableCollection.replaceSubrange<A, B>(_:with:) ()
#&#8203;74862  0x000000010000693b in protocol witness for RangeReplaceableCollection.replaceSubrange<A>(_:with:) in conformance Foo ()
#&#8203;74863  0x000000010045ece5 in specialized RangeReplaceableCollection.replaceSubrange<A, B>(_:with:) ()
#&#8203;74864  0x00000001002cd3e8 in RangeReplaceableCollection.replaceSubrange<A, B>(_:with:) ()
#&#8203;74865  0x000000010000693b in protocol witness for RangeReplaceableCollection.replaceSubrange<A>(_:with:) in conformance Foo ()
#&#8203;74866  0x000000010045ece5 in specialized RangeReplaceableCollection.replaceSubrange<A, B>(_:with:) ()
#&#8203;74867  0x00000001002cd3e8 in RangeReplaceableCollection.replaceSubrange<A, B>(_:with:) ()
#&#8203;74868  0x000000010000693b in protocol witness for RangeReplaceableCollection.replaceSubrange<A>(_:with:) in conformance Foo ()
#&#8203;74869  0x00000001002cdd97 in RangeReplaceableCollection.insert(_:at:) ()
#&#8203;74870  0x0000000100006b06 in protocol witness for RangeReplaceableCollection.insert(_:at:) in conformance Foo ()
#&#8203;74871  0x00000001002cd88c in RangeReplaceableCollection.append(_:) ()
#&#8203;74872  0x0000000100004bf3 in main at ...

My guess is that the compiler should not have allowed the adoption of RangeReplaceableCollection...

[phausler]

Hmm seems that it happens on either a struct or a class...

[belkadan]

Yeah, the class thing is SR-142, which is unfortunately correct behavior. The stack overflow seems like a real issue, though. Lance (JIRA User), any insights?

[natecook1000]

It looks like the default implementation here, which is intended to support range expressions, satisfies the actual replaceSubrange(_:with:) requirement: https://github.com/apple/swift/blob/master/stdlib/public/core/RangeReplaceableCollection.swift#L738

That means the compiler isn't giving the error that would tell someone they need to implement the requirement for RangeReplaceableCollection conformance, and that default implementation just calls itself infinitely. cc @airspeedswift

[tkrajacic]

Related: SR-10647

[tkrajacic]

Also related: SR-5211

[gwynne]

Additional context: SR-5211 and SR-10647 both discuss the lack of compiler warning (as @natecook1000 also said) and provide at least one suggestion for an alternative definition from @phausler. This is still very easy to run into when working with collection protocols.

[glessard]

Recently we've eliminated other similar issues by adding an unavailable implementation. This upgrades the problem to a compilation error, although the compiler messages could use improvement.

#38950