[swift-dev] Should we remove _customContainsEquatableElement from stdlib?

[Ling Wang]

Do you mean something like this in validation-test/stdlib/SequenceType.swift.gyb:

SequenceTypeTests.test("Range<Element>.contains/WhereElementIsComparable/dispatch") {
  MinimalComparableValue.timesLessWasCalled = 0
  let start = 0
  let end = 10
  let range = Range(start: MinimalComparableValue(start), end: MinimalComparableValue(end))
  let count = 20
  for test in 0...count {
    expectEqual(
      test >= start && test < end,
      range.contains(MinimalComparableValue(test)))
  }
  expectEqual(
    count * 2, MinimalComparableValue.timesLessWasCalled)
}

There is one issue. MinimalComparableValue doesn’t conform to ForwardIndexType. I’m not sure it is proper for me to add this conformance in an extension because the definition of MinimalComparableValue is “A type that conforms only to `Equatable` and `Comparable`”.

What’s your suggestion?

> On Dec 31, 2015, at 11:21 AM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> 
> Yes, I think it is good -- this is exactly the use case for this
> method, except that @_transparent should be dropped, and the comment
> is duplicating the code, so it should be dropped, too.  The change
> also needs a simple test that checks that calling .contains() on a
> suitable Range should dispatch to this method (you can detect that by
> using MinimalComparableValue and supplying it a custom lessImpl that
> counts the number of invocations).
> 
> Dmitri
> 
> On Thu, Dec 31, 2015 at 7:05 PM, Ling Wang <an00na at gmail.com> wrote:
>> Following the previous discussion do you think this is a good patch:
>> 
>> // O(1) implementation of `contains()` for ranges of comparable elements.
>> extension Range where Element: Comparable {
>>  @_transparent
>>  @warn_unused_result
>>  func _customContainsEquatableElement(
>>    element: Generator.Element
>>  ) -> Bool? {
>>    return element >= self.startIndex && element < self.endIndex
>>  }
>> }
>> 
>>> On Dec 30, 2015, at 6:42 PM, Ling Wang <an00na at gmail.com> wrote:
>>> 
>>> Got it. `contains` is only declared in extension. Interesting workaround. Thanks.
>>> 
>>>> On Dec 30, 2015, at 4:45 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
>>>> 
>>>> On Wed, Dec 30, 2015 at 8:34 PM, Ling Wang via swift-dev
>>>> <swift-dev at swift.org> wrote:
>>>>> After reviewing the code of stdlib I found no one actually implements _customContainsEquatableElement:
>>>>> 1. Its default implementation in `SequenceType` just returns nil.
>>>>> 2. The implementation in `Set` delegates to `contains` which is bad because it reverses their relationship: the default implementation of `contains` in `SequenceType` delegates to `_customContainsEquatableElement`.
>>>>> 3. In all other place it just delegates to another `SequenceType`.
>>>>> 
>>>>> So no one is doing real work.
>>>>> 
>>>>> If the current _customContainsEquatableElement is only a relic I suggest we remove it and clean up related code.
>>>> 
>>>> It is not a relic.  It allows the contains() method to perform dynamic
>>>> dispatch in case the container knows something extra about the
>>>> elements.
>>>> 
>>>> Consider the case when you have a Set typed as a plain Sequence or a
>>>> Collection.  In that case, you won't be able to call the custom
>>>> Set.contains() method, the overload resolution will only see the
>>>> protocol extension.  This extra entry point,
>>>> _customContainsEquatableElement, allows us to perform dynamic dispatch
>>>> and use the fast Set implementation if it is available.
>>>> 
>>>> Dmitri
>>>> 
>>>> --
>>>> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
>>>> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/
>>> 
>> 
> 
> 
> 
> -- 
> main(i,j){for(i=2;;i++){for(j=2;j<i;j++){if(!(i%j)){j=0;break;}}if
> (j){printf("%d\n",i);}}} /*Dmitri Gribenko <gribozavr at gmail.com>*/