Refactored interval lookup operations

[joaquintides]

Closes #51. Includes regression tests from #53. Branched off https://github.com/boostorg/icl/tree/fix/interval_order.

Explanation of changes

Interval comparison is generally a partial order rather than a strict weak order (SWO). This does not pose any problem with associative containers as long as the intervals in the container are disjoint, since the induced order restricted to those is a SWO (indeed, a total order). A difficulty may arise when doing a lookup operation for an interval k that overlaps with E = elements(container), as the induced order over {k} ∪ E may not be a SWO. All stdlib implementations work in this case as implicitly expected by Boost.ICL except libc++ v22 or higher:

• [libc++] LLVM 22 changed behavior for map and set search operations when used with not quite a strict weak ordering llvm/llvm-project#183189

Whether libc++'s behavior is conformant or not (and whether looking up a key that violates SWO is UB or not) is contested, see

• [libc++ 22] Wrong assumption of SWO guarantees in associative container lookup operations llvm/llvm-project#187667 ,

but, regardless, we can solve the problem by resorting to heterogeneous lookup. When k is of a type other than key_type and the compare predicate is transparent, lookup operations on k are defined by the standard in terms of elements being partitioned by k, without any reference to SWO compliance. boost::icl::detail::assoc_container_adaptor, in combination with boost::icl::detail::transparent_compare, forces lookup operations on the adapted container to be routed through its
heterogeneous lookup overloads, and does nothing for C++11 containers without het lookup. This circumvents libc++'s singular behavior except when in C++11 mode: in this case, we use Boost.Container, which supports het lookup even in C++11 (see impl_config.hpp). Insert functions are also provided that prevent UB when the element would violate SWO (this is UB regardless of the resolution of libc++'s issue).

Additional fixes

• Boost.ICL interval find functions are documented to return the first eligible element, which is not guaranteed by std::(set|map)::find ; assoc_container_adaptor fixes that.
• Boost.ICL assumes set::iterator is the same type as set::const_iterator, which is not guaranteed generally and is certainly not the case for boost::container::set. Fixed by assoc_container_adaptor.
• Minor fixes when Boost.Container is used as a backend (user-definable macro ICL_USE_BOOST_MOVE_IMPLEMENTATION): this case wasn't being tested in CI.

[ldionne]

From the PR description:

All stdlib implementations support this case

I don't think it's correct to claim that stdlibs support this since there is still disagreement on whether this is UB in the first place (which means ICL would merely be getting lucky with other implementations).

[ldionne]

I didn't look at the patch in detail, but when I looked into fixing this myself, one of the things I needed to do was ensure that we also don't call .insert on the underlying std:: container with a key that is potentially overlapping with other elements already in the underlying container. IIRC what happens right now is that ICL will insert a potentially overlapping interval into the underlying container, and then merge it with surrounding intervals depending on the policy. But the initial insert call is UB because of SWO violation.

I don't think you're addressing that in this patch?

[joaquintides]

Umm, good catch. Yes, I'm not addressing that, and if SWO violation in the way we're discussing is to be interpreted as UB then this patch is insufficient. Will try to come up with a regression test against libc++ v22.

[joaquintides]

Added fixes for that in the PR. I wasn't able to write a regression test for the case: presumably because libc++ v22 hasn't translated its SWO assumptions in lookup operations over to insertion code?

[ldionne]

This is silently ABI-breaking. I feel like it's probably more appropriate not to compile in this case.

[joaquintides]

I don't think this is a problem in the context of Boost, which is abundantly not ABI-stable across C++ versions.