[m-rev.] for post-commit review: det_remove*, rev_sorted_list_to_set

[Julien Fischer]

On Mon, 9 Sep 2019, Peter Wang wrote:

>> So yes, having one checked and one unchecked version works. But this leaves
>> an interesting question: how do we notify users that they should consider
>> replacing existing calls to sorted_list_to_set with the unchecked versions?
>> In most of the set modules, the existing sorted_list_to_set *was* unchecked,
>> so any call site that has been working all this time should switch over to the
>> unchecked version for performance. But we can't mark sorted_list_to_set as
>> obsolete if that remains the name of the checked version. But just adding
>> checking to these modules' versions of sorted_list_to_set would silently
>> add overhead to all their existing callers. One way to avoid this would be
>> to name the two versions unchecked_sorted_list_to_set and
>> checked_sorted_list_to_set, keep the existing sorted_list_to_set in each
>> set module, and mark it obsolete in favor of the checked and unchecked versions.
>> Opinions?
>
> To me, the predicates/functions with "sorted_list_to_" in their names
> already indicate that it is the caller's responsibility to pass sorted
> lists as input. The simplest solution is to leave them as unchecked, but
> make the documentation more explicit.

That's fine by me.

> It's unfortunately that sorted_list_to_set assumes no duplicates.
> I think predicates/functions that assume no duplicates should have an
> indication in their names, e.g. sorted_no_dup_list_to_set.

The problem here that I think does need to be resolved is the
inconsistency between the different set implementations with the
behaviour of sorted_list_to_set w.r.t to duplicates.  Not addressing
this makes swapping the set implementation being used somewhat risky.

Julien.