diff --git a/include/boost/icl/concept/interval.hpp b/include/boost/icl/concept/interval.hpp index fab7dd9..080963c 100644 --- a/include/boost/icl/concept/interval.hpp +++ b/include/boost/icl/concept/interval.hpp @@ -96,6 +96,12 @@ typename enable_if singleton(const typename interval_traits::domain_type& value) { //ASSERT: This always creates an interval with exactly one element + typedef typename interval_traits::domain_type domain_type; + typedef typename interval_traits::domain_compare domain_compare; + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(value) )); + boost::ignore_unused(); + return interval_traits::construct(value, domain_next(value)); } @@ -127,6 +133,8 @@ singleton(const typename interval_traits::domain_type& value) typedef typename interval_traits::domain_compare domain_compare; BOOST_ASSERT((numeric_minimum::value> ::is_less_than(value))); + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(value))); boost::ignore_unused(); return interval_traits::construct( domain_prior(value) @@ -168,6 +176,12 @@ typename enable_if >::type unit_trail(const typename interval_traits::domain_type& value) { + typedef typename interval_traits::domain_type domain_type; + typedef typename interval_traits::domain_compare domain_compare; + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(value) )); + boost::ignore_unused(); + return interval_traits::construct(value, domain_next(value)); } @@ -202,6 +216,8 @@ unit_trail(const typename interval_traits::domain_type& value) typedef typename interval_traits::domain_compare domain_compare; BOOST_ASSERT((numeric_minimum::value> ::is_less_than(value))); + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(value))); boost::ignore_unused(); return interval_traits::construct( domain_prior(value) @@ -283,11 +299,21 @@ typename enable_if, Type>::type hull(const typename interval_traits::domain_type& left, const typename interval_traits::domain_type& right) { + typedef typename interval_traits::domain_type domain_type; typedef typename interval_traits::domain_compare domain_compare; + boost::ignore_unused(); if(domain_compare()(left,right)) + { + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(right) )); return construct(left, domain_next(right)); + } else + { + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(left) )); return construct(right, domain_next(left)); + } } template @@ -338,6 +364,8 @@ hull(const typename interval_traits::domain_type& left, { BOOST_ASSERT((numeric_minimum::value> ::is_less_than(left) )); + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(right) )); return construct( domain_prior(left) , domain_next(right)); } @@ -345,6 +373,8 @@ hull(const typename interval_traits::domain_type& left, { BOOST_ASSERT((numeric_minimum::value> ::is_less_than(right) )); + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(left) )); return construct( domain_prior(right) , domain_next(left)); } @@ -400,6 +430,12 @@ enable_if< mpl::and_< mpl::or_, is_static_open > , typename interval_traits::domain_type>::type first(const Type& object) { + typedef typename interval_traits::domain_type domain_type; + typedef typename interval_traits::domain_compare domain_compare; + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than(lower(object)) )); + boost::ignore_unused(); + return domain_next(lower(object)); } @@ -408,6 +444,12 @@ inline typename enable_if, typename interval_traits::domain_type>::type first(const Type& object) { + typedef typename interval_traits::domain_type domain_type; + typedef typename interval_traits::domain_compare domain_compare; + BOOST_ASSERT((numeric_maximum::value> + ::is_greater_than_or(lower(object), is_left_closed(object.bounds())) )); + boost::ignore_unused(); + return is_left_closed(object.bounds()) ? lower(object) : domain_next(lower(object)); diff --git a/include/boost/icl/detail/exclusive_less_than.hpp b/include/boost/icl/detail/exclusive_less_than.hpp index 3a4aa65..ba5eb4a 100644 --- a/include/boost/icl/detail/exclusive_less_than.hpp +++ b/include/boost/icl/detail/exclusive_less_than.hpp @@ -17,11 +17,35 @@ namespace boost{ namespace icl template struct exclusive_less_than { - /** Operator operator() implements a strict weak ordering on intervals. */ + /// Enables heterogeneous lookup on a domain point (C++14 std::set / Boost.Container). + typedef void is_transparent; + + typedef typename interval_traits::domain_type domain_type; + + + /** Strict partial ordering on intervals (unchanged). */ bool operator()(const IntervalT& left, const IntervalT& right)const - { - return icl::non_empty::exclusive_less(left, right); + { + return icl::non_empty::exclusive_less(left, right); } + + + /** interval vs point: left lies exclusively to the left of point p. */ + bool operator()(const IntervalT& left, const domain_type& p)const + { + return icl::domain_less(icl::upper(left), p) + || ( !icl::domain_less(p, icl::upper(left)) // upper(left) == p + && !icl::is_right_closed(icl::bounds(left)) ); // ... and right border open + } + + /** point vs interval: point p lies exclusively to the left of right. */ + bool operator()(const domain_type& p, const IntervalT& right)const + { + return icl::domain_less(p, icl::lower(right)) + || ( !icl::domain_less(icl::lower(right), p) // p == lower(right) + && !icl::is_left_closed(icl::bounds(right)) ); // ... and left border open + } + }; }} // namespace boost icl diff --git a/include/boost/icl/detail/interval_lookup.hpp b/include/boost/icl/detail/interval_lookup.hpp new file mode 100644 index 0000000..28e08fe --- /dev/null +++ b/include/boost/icl/detail/interval_lookup.hpp @@ -0,0 +1,243 @@ +/*-----------------------------------------------------------------------------+ +Copyright (c) 2026: Boost.ICL SWO fix (centralized) ++------------------------------------------------------------------------------+ + Distributed under the Boost Software License, Version 1.0. + (See accompanying file LICENCE.txt or copy at + http://www.boost.org/LICENSE_1_0.txt) ++-----------------------------------------------------------------------------*/ +#ifndef BOOST_ICL_DETAIL_INTERVAL_LOOKUP_HPP_JOFA_260610 +#define BOOST_ICL_DETAIL_INTERVAL_LOOKUP_HPP_JOFA_260610 + +/*-----------------------------------------------------------------------------+ +| THE SINGLE HOME FOR THE STRICT-WEAK-ORDERING FIX. | +| | +| The underlying std::set/std::map is ALWAYS pairwise-disjoint. On disjoint | +| intervals exclusive_less_than is a strict total order, so every comparison | +| the container performs between STORED elements is a valid SWO: erase, | +| iteration, rebalancing, etc. are never at risk. | +| | +| The ordering is violated only by an *argument* that overlaps stored | +| elements: such an argument is transiently equivalent to several ordered | +| stored elements, which breaks transitivity of equivalence. Where that | +| actually bites depends on the operation: | +| | +| * lower_bound / upper_bound / equal_range -- a tree descent guided by the | +| overlapping argument can return a wrong (but not rejected) iterator, and | +| ICL then operates on the wrong range. libc++ >= 22 (PR #155245) uses a | +| single-descent search that assumes a SWO, so this is the real, observable| +| SWO bug. The accessors fix it by never handing the tree an overlapping | +| interval: anchor every lookup on a single point, whose equivalence class | +| over disjoint intervals has size <= 1 (a genuine SWO), then apply at most| +| one O(1) correction for an open-bound "kiss". | +| | +| * the add insert (un-hinted AND hinted) -- formally violates the SWO | +| precondition too, but is benign on every standard library tested. A | +| unique-key insert does a single root-to-leaf descent that stops on the | +| first equivalent node, and because the intervals overlapping the argument| +| form a contiguous in-order run, that descent always lands on the run and | +| rejects the insert {pos,false} -- exactly the collision signal ICL's | +| add() relies on. The hinted overload behaves the same: for an overlapping| +| argument the O(1)-hint neighbour check fails and the library falls back | +| to the full search, which also rejects. Routing the add inserts through | +| interval_lookup::insert only removes the formal UB; it does not fix an | +| observed failure. (Residual/gap inserts whose arguments are | +| disjoint-by-construction stay raw -- they are never overlapping.) | +| | +| PERFORMANCE: for discrete / dynamic-bound intervals the anchor is a CLOSED | +| SINGLETON [v,v] -- a key_type value -- so the query takes the HOMOGENEOUS | +| lower_bound/upper_bound overload (libc++ 22's fast descent). Only | +| static-bounds continuous intervals (no singleton()) need a heterogeneous | +| point query (C++14) and fall back to a raw call in C++11. | +| | +| This helper is the ONLY place that logic lives. interval_base_set and | +| interval_base_map call it with a key-extraction functor (identity for a set, | +| ".first" for a map), so the set and map sides share one implementation | +| instead of carrying two copies; both base classes point back here for the | +| full rationale instead of repeating it. | ++-----------------------------------------------------------------------------*/ + +#include +#include // BOOST_CXX_VERSION +#include +#include +#include + +#include +#include +#include +#include // has_static_bounds / has_dynamic_bounds +#include +#include // icl::lower/upper/is_empty/singleton + +namespace boost{ namespace icl{ namespace detail +{ + +// value_type -> const key_type& extractors. +struct identity_key +{ + template const T& operator()(const T& x) const { return x; } +}; +struct pair_first_key +{ + template + auto operator()(const Pair& x) const -> decltype((x.first)) { return x.first; } +}; + +struct interval_lookup +{ + // Two stored/queried intervals overlap iff icl::intersects holds. The call + // sites below always pass two non-empty intervals, for which intersects is + // exactly the comparator equivalence !comp(a,b) && !comp(b,a) over + // exclusive_less_than -- so we use the canonical helper and stay in sync + // with any future change to ICL's overlap/touch semantics. + + //========================================================================= + //= lower_bound + //========================================================================= + + // discrete OR dynamic-bound: HOMOGENEOUS singleton anchor (fast path). + template, + is_discrete::domain_type> >, int>::type = 0> + static It lower_bound(Cont& c, const KeyT& interval, Kov kov) + { + typedef typename interval_traits::domain_type domain_type; + typedef typename interval_traits::domain_compare domain_compare; + if(icl::is_empty(interval)) return c.end(); + const typename Cont::key_compare comp = typename Cont::key_compare(); + It it_; + if(numeric_minimum::value> + ::is_less_than(icl::lower(interval))) + it_ = c.lower_bound(icl::singleton(icl::lower(interval))); // homogeneous + else + it_ = c.begin(); + if(it_ != c.end() && !icl::intersects(kov(*it_), interval) && !comp(interval, kov(*it_))) + ++it_; // one fwd correction + return it_; + } + + // static-bounds continuous: HETEROGENEOUS point anchor (C++14), raw in C++11. + template, + is_continuous::domain_type> >, int>::type = 0> + static It lower_bound(Cont& c, const KeyT& interval, Kov kov) + { +# if (BOOST_CXX_VERSION >= 201402L) + if(icl::is_empty(interval)) return c.end(); + const typename Cont::key_compare comp = typename Cont::key_compare(); + It it_ = c.lower_bound(icl::lower(interval)); // heterogeneous point + if(it_ != c.end() && !icl::intersects(kov(*it_), interval) && !comp(interval, kov(*it_))) + ++it_; + return it_; +# else + // C++11 LIMITATION (static-bounds continuous intervals only): + // there is no heterogeneous point query available here, so we fall + // back to handing the tree the full OVERLAPPING interval. This is the + // exact SWO violation this header otherwise avoids: under a + // single-descent search (libc++ >= 22) the comparator is not a strict + // weak order for an overlapping argument, so this call CAN RETURN A + // WRONG ITERATOR and silently produce incorrect query results. + // Build with C++14 or newer to get the point-anchored, SWO-safe path. + (void)kov; + return c.lower_bound(interval); +# endif + } + + //========================================================================= + //= upper_bound + //========================================================================= + + // discrete OR dynamic-bound: HOMOGENEOUS singleton anchor (fast path). + template, + is_discrete::domain_type> >, int>::type = 0> + static It upper_bound(Cont& c, const KeyT& interval, Kov kov) + { + typedef typename interval_traits::domain_type domain_type; + typedef typename interval_traits::domain_compare domain_compare; + if(icl::is_empty(interval)) return c.end(); + const typename Cont::key_compare comp = typename Cont::key_compare(); + It it_; + if(numeric_maximum::value> + ::is_greater_than(icl::upper(interval))) + it_ = c.upper_bound(icl::singleton(icl::upper(interval))); // homogeneous + else + it_ = c.end(); + if(it_ != c.begin()) + { + It prev_ = it_; --prev_; + if(!comp(kov(*prev_), icl::upper(interval)) && !icl::intersects(kov(*prev_), interval)) + it_ = prev_; // one back correction + } + return it_; + } + + // static-bounds continuous: HETEROGENEOUS point anchor (C++14), raw in C++11. + template, + is_continuous::domain_type> >, int>::type = 0> + static It upper_bound(Cont& c, const KeyT& interval, Kov kov) + { +# if (BOOST_CXX_VERSION >= 201402L) + if(icl::is_empty(interval)) return c.end(); + const typename Cont::key_compare comp = typename Cont::key_compare(); + const typename interval_traits::domain_type anchor = icl::upper(interval); + It it_ = c.upper_bound(anchor); // heterogeneous point + if(it_ != c.begin()) + { + It prev_ = it_; --prev_; + if(!comp(kov(*prev_), anchor) && !icl::intersects(kov(*prev_), interval)) + it_ = prev_; + } + return it_; +# else + // C++11 LIMITATION (static-bounds continuous intervals only): + // same as the lower_bound fallback above -- without a heterogeneous + // point query we hand the tree the full OVERLAPPING interval, which + // violates the strict weak order under a single-descent search + // (libc++ >= 22). This call CAN RETURN A WRONG ITERATOR and silently + // produce incorrect query results. Build with C++14 or newer to get + // the point-anchored, SWO-safe path. + (void)kov; + return c.upper_bound(interval); +# endif + } + + //========================================================================= + //= disjoint-only insertion hardening (removes the formal insert-side UB) + //========================================================================= + + template + static std::pair insert(Cont& c, const Value& v, Kov kov) + { + const typename Cont::key_compare comp = typename Cont::key_compare(); + It lb_ = lower_bound(c, kov(v), kov); + if(lb_ == c.end() || comp(kov(v), kov(*lb_))) + return std::pair(c.insert(lb_, v), true); + return std::pair(lb_, false); + } + + template + static std::pair insert(Cont& c, It hint_, const Value& v, Kov kov) + { + const typename Cont::key_compare comp = typename Cont::key_compare(); + bool good_ = (hint_ == c.end() || comp(kov(v), kov(*hint_))); + if(good_ && hint_ != c.begin()) + { It p_ = hint_; --p_; good_ = comp(kov(*p_), kov(v)); } + if(good_) + return std::pair(c.insert(hint_, v), true); + return insert(c, v, kov); + } +}; + +}}} // namespace detail icl boost + +#endif diff --git a/include/boost/icl/interval_base_map.hpp b/include/boost/icl/interval_base_map.hpp index f464a6a..3f4d189 100644 --- a/include/boost/icl/interval_base_map.hpp +++ b/include/boost/icl/interval_base_map.hpp @@ -9,6 +9,7 @@ Copyright (c) 1999-2006: Cortex Software GmbH, Kantstrasse 57, Berlin #ifndef BOOST_ICL_INTERVAL_BASE_MAP_HPP_JOFA_990223 #define BOOST_ICL_INTERVAL_BASE_MAP_HPP_JOFA_990223 +#include #include #include #include @@ -19,6 +20,7 @@ Copyright (c) 1999-2006: Cortex Software GmbH, Kantstrasse 57, Berlin #include #include #include +#include #include @@ -285,11 +287,14 @@ class interval_base_map return icl::find(*this, key_value); } - /** Find the first interval value pair, that collides with interval + /** Find the first interval value pair, that collides with interval \c key_interval */ const_iterator find(const interval_type& key_interval)const - { - return _map.find(key_interval); + { + const_iterator it_ = lower_bound(key_interval); + if(it_ != end() && !key_compare()(key_interval, (*it_).first)) + return it_; + return end(); } /** Total select function. */ @@ -440,29 +445,34 @@ class interval_base_map //= Iterator related //========================================================================== + // SWO-safe lookup lives in one shared place now: detail::interval_lookup. + // The map passes pair_first_key (extracts (*it).first); see that header. + // The SWO CONTAINER INVARIANT rationale also lives in that header + // (). + iterator lower_bound(const key_type& interval) - { return _map.lower_bound(interval); } + { return detail::interval_lookup::lower_bound(_map, interval, detail::pair_first_key()); } iterator upper_bound(const key_type& interval) - { return _map.upper_bound(interval); } + { return detail::interval_lookup::upper_bound(_map, interval, detail::pair_first_key()); } const_iterator lower_bound(const key_type& interval)const - { return _map.lower_bound(interval); } + { return detail::interval_lookup::lower_bound(_map, interval, detail::pair_first_key()); } const_iterator upper_bound(const key_type& interval)const - { return _map.upper_bound(interval); } + { return detail::interval_lookup::upper_bound(_map, interval, detail::pair_first_key()); } std::pair equal_range(const key_type& interval) - { + { return std::pair - (lower_bound(interval), upper_bound(interval)); + (lower_bound(interval), upper_bound(interval)); } - std::pair + std::pair equal_range(const key_type& interval)const - { + { return std::pair - (lower_bound(interval), upper_bound(interval)); + (lower_bound(interval), upper_bound(interval)); } iterator begin() { return _map.begin(); } @@ -549,12 +559,19 @@ class interval_base_map protected: + // disjoint-only insertion hardening (see interval_base_set::_swo_insert) + std::pair _swo_insert(const value_type& x_) + { return detail::interval_lookup::insert(_map, x_, detail::pair_first_key()); } + + std::pair _swo_insert(const iterator& hint_, const value_type& x_) + { return detail::interval_lookup::insert(_map, hint_, x_, detail::pair_first_key()); } + template iterator gap_insert(iterator prior_, const interval_type& inter_val, const codomain_type& co_val ) { // inter_val is not conained in this map. Insertion will be successful - BOOST_ASSERT(this->_map.find(inter_val) == this->_map.end()); + BOOST_ASSERT(this->find(inter_val) == this->end()); BOOST_ASSERT((!on_absorbtion::is_absorbable(co_val))); return this->_map.insert(prior_, value_type(inter_val, version()(co_val))); } @@ -567,31 +584,23 @@ class interval_base_map // Never try to insert an identity element into an identity element absorber here: BOOST_ASSERT((!(on_absorbtion::is_absorbable(co_val)))); - iterator inserted_ - = this->_map.insert(prior_, value_type(inter_val, Combiner::identity_element())); + std::pair ins_ + = this->_swo_insert(prior_, value_type(inter_val, Combiner::identity_element())); - if((*inserted_).first == inter_val && (*inserted_).second == Combiner::identity_element()) + if(ins_.second) { - Combiner()((*inserted_).second, co_val); - return std::pair(inserted_, true); + Combiner()((*ins_.first).second, co_val); + return std::pair(ins_.first, true); } else - return std::pair(inserted_, false); + return std::pair(ins_.first, false); } std::pair insert_at(const iterator& prior_, const interval_type& inter_val, const codomain_type& co_val ) { - iterator inserted_ - = this->_map.insert(prior_, value_type(inter_val, co_val)); - - if(inserted_ == prior_) - return std::pair(inserted_, false); - else if((*inserted_).first == inter_val) - return std::pair(inserted_, true); - else - return std::pair(inserted_, false); + return this->_swo_insert(prior_, value_type(inter_val, co_val)); } @@ -951,16 +960,19 @@ inline typename interval_base_map_map.end(); std::pair insertion - = this->_map.insert(value_type(inter_val, version()(co_val))); + = this->_swo_insert(value_type(inter_val, version()(co_val))); if(insertion.second) return that()->handle_inserted(insertion.first); else { - // Detect the first and the end iterator of the collision sequence - iterator first_ = this->_map.lower_bound(inter_val), - last_ = prior(this->_map.upper_bound(inter_val)); - //assert(end_ == this->_map.upper_bound(inter_val)); + // Detect the first and the end iterator of the collision sequence. + // Use the member lower_bound/upper_bound (not _map's) so the overlapping run is found via the + // SWO-safe scalar-endpoint anchoring -- exclusive_less_than is not a strict weak ordering, see + // the note at those accessors. + iterator first_ = this->lower_bound(inter_val), + last_ = prior(this->upper_bound(inter_val)); + //assert(end_ == this->upper_bound(inter_val)); iterator it_ = first_; interval_type rest_interval = inter_val; @@ -1144,16 +1156,17 @@ inline typename interval_base_map_map.end(); - std::pair insertion = this->_map.insert(addend); + std::pair insertion = this->_swo_insert(addend); if(insertion.second) return that()->handle_inserted(insertion.first); else { - // Detect the first and the end iterator of the collision sequence - iterator first_ = this->_map.lower_bound(inter_val), - last_ = prior(this->_map.upper_bound(inter_val)); - //assert((++last_) == this->_map.upper_bound(inter_val)); + // Detect the first and the end iterator of the collision sequence. + // Use the member lower_bound/upper_bound (not _map's); see the note at those accessors. + iterator first_ = this->lower_bound(inter_val), + last_ = prior(this->upper_bound(inter_val)); + //assert((++last_) == this->upper_bound(inter_val)); iterator it_ = first_; insert_main(inter_val, co_val, it_, last_); return it_; diff --git a/include/boost/icl/interval_base_set.hpp b/include/boost/icl/interval_base_set.hpp index d693bd4..73b27ca 100644 --- a/include/boost/icl/interval_base_set.hpp +++ b/include/boost/icl/interval_base_set.hpp @@ -19,6 +19,7 @@ Copyright (c) 1999-2006: Cortex Software GmbH, Kantstrasse 57, Berlin # include #endif +#include #include #include #include @@ -30,6 +31,7 @@ Copyright (c) 1999-2006: Cortex Software GmbH, Kantstrasse 57, Berlin #include #include #include +#include #include #include @@ -40,6 +42,12 @@ namespace boost{namespace icl { /** \brief Implements a set as a set of intervals (base class) */ +// SWO CONTAINER INVARIANT: the full rationale (why stored-element comparisons, +// erase, iteration and rebalancing are never at risk, why only an overlapping +// *argument* breaks the ordering, why the query accessors anchor on a point, +// and why _swo_insert hardens the inserts) lives with the code it describes, +// in . The lookup/insert members below +// forward to that shared helper (passing identity_key for a set). template < typename SubType, @@ -239,7 +247,10 @@ class interval_base_set /** Find the first interval, that collides with interval \c key_interval */ const_iterator find(const interval_type& key_interval)const { - return this->_set.find(key_interval); + const_iterator it_ = lower_bound(key_interval); + if(it_ != end() && !key_compare()(key_interval, *it_)) + return it_; + return end(); } //========================================================================== @@ -360,29 +371,32 @@ class interval_base_set const_reverse_iterator rbegin()const { return _set.rbegin(); } const_reverse_iterator rend()const { return _set.rend(); } + // SWO-safe lookup lives in one shared place now: detail::interval_lookup. + // The set passes identity_key (value_type == key_type); see that header. + iterator lower_bound(const value_type& interval) - { return _set.lower_bound(interval); } + { return detail::interval_lookup::lower_bound(_set, interval, detail::identity_key()); } iterator upper_bound(const value_type& interval) - { return _set.upper_bound(interval); } + { return detail::interval_lookup::upper_bound(_set, interval, detail::identity_key()); } const_iterator lower_bound(const value_type& interval)const - { return _set.lower_bound(interval); } + { return detail::interval_lookup::lower_bound(_set, interval, detail::identity_key()); } const_iterator upper_bound(const value_type& interval)const - { return _set.upper_bound(interval); } + { return detail::interval_lookup::upper_bound(_set, interval, detail::identity_key()); } std::pair equal_range(const key_type& interval) { return std::pair - (_set.lower_bound(interval), _set.upper_bound(interval)); + (lower_bound(interval), upper_bound(interval)); } std::pair equal_range(const key_type& interval)const { return std::pair - (_set.lower_bound(interval), _set.upper_bound(interval)); + (lower_bound(interval), upper_bound(interval)); } private: @@ -399,6 +413,28 @@ class interval_base_set sub_type* that() { return static_cast(this); } const sub_type* that()const { return static_cast(this); } +protected: + // --- disjoint-only insertion (hardening) -------------------------------- + // Guarantees we only ever hand .insert a key that is disjoint from + // everything stored, independent of the container's insertion algorithm. + // This only removes a formal SWO-precondition violation: passing an + // overlapping argument to insert technically breaks the comparator's SWO + // contract, but every standard library tested rejects the overlap safely on + // both the un-hinted and hinted overloads, so this is not fixing an observed + // failure (see the rationale in detail::interval_lookup). + // It probes for a colliding interval through the SWO-safe member + // lower_bound and inserts only when the key is disjoint, using a correct + // hint so the container compares 'addend' solely against disjoint + // neighbours. On collision it returns {first-overlapping, false} without + // touching the container. + std::pair _swo_insert(const segment_type& addend) + { return detail::interval_lookup::insert(_set, addend, detail::identity_key()); } + + // Hinted variant: keeps the amortized-constant fast path when 'hint_' is the + // correct disjoint position, otherwise falls back to the lower_bound probe. + std::pair _swo_insert(iterator hint_, const segment_type& addend) + { return detail::interval_lookup::insert(_set, hint_, addend, detail::identity_key()); } + protected: ImplSetT _set; } ; @@ -503,13 +539,15 @@ inline typename interval_base_set::itera if(icl::is_empty(addend)) return this->_set.end(); - std::pair insertion = this->_set.insert(addend); + std::pair insertion = this->_swo_insert(addend); if(insertion.second) return that()->handle_inserted(insertion.first); else { - iterator last_ = prior(this->_set.upper_bound(addend)); + // member upper_bound (not _set's) widens to the full overlapping run via SWO-safe scalar + // anchoring; exclusive_less_than is not a strict weak ordering, see the note at those accessors. + iterator last_ = prior(this->upper_bound(addend)); return that()->add_over(addend, last_); } } @@ -523,13 +561,15 @@ inline typename interval_base_set::itera if(icl::is_empty(addend)) return prior_; - iterator insertion = this->_set.insert(prior_, addend); + std::pair insertion = this->_swo_insert(prior_, addend); - if(*insertion == addend) - return that()->handle_inserted(insertion); + if(insertion.second) + return that()->handle_inserted(insertion.first); else { - iterator last_ = prior(this->_set.upper_bound(addend)); + // member upper_bound (not _set's) widens to the full overlapping run via SWO-safe scalar + // anchoring; exclusive_less_than is not a strict weak ordering, see the note at those accessors. + iterator last_ = prior(this->upper_bound(addend)); return that()->add_over(addend, last_); } } diff --git a/include/boost/icl/split_interval_set.hpp b/include/boost/icl/split_interval_set.hpp index ca0d022..41299e5 100644 --- a/include/boost/icl/split_interval_set.hpp +++ b/include/boost/icl/split_interval_set.hpp @@ -162,7 +162,7 @@ class split_interval_set: iterator add_over(const interval_type& addend, iterator last_) { - iterator first_ = this->_set.lower_bound(addend); + iterator first_ = this->lower_bound(addend); //BOOST_ASSERT(next(last_) == this->_set.upper_bound(inter_val)); iterator it_ = first_; diff --git a/include/boost/icl/type_traits/is_numeric.hpp b/include/boost/icl/type_traits/is_numeric.hpp index 9ecc3a1..a165e7e 100644 --- a/include/boost/icl/type_traits/is_numeric.hpp +++ b/include/boost/icl/type_traits/is_numeric.hpp @@ -54,6 +54,36 @@ struct is_numeric > }; //-------------------------------------------------------------------------- +// The lowest and highest representable values of a numeric domain. Used by +// the bound-guards below to detect under-/overflow of domain_prior/domain_next. +// +// Note that for non-integral types std::numeric_limits::min() is the smallest +// *positive* normalized value, NOT the most negative one. The most negative +// value is lowest(). For integral types lowest()==min(), so lowest() +// is the correct universal choice for the lower extreme. +// +// Types for which is_numeric is true but std::numeric_limits is not specialized +// (e.g. boost::rational) have no usable bounds, so the guards impose no +// constraint (return true) rather than comparing against a bogus lowest()/max() +// that silently default-constructs to T(). If boost::rational ever gets a +// specialization it will be used automatically. +//-------------------------------------------------------------------------- +namespace detail +{ + // Evaluate the bound comparison compare(left, right), but only when + // std::numeric_limits is specialized. Otherwise the domain has no + // usable bounds and the guard imposes no constraint by returning true. + // Centralizes the is_specialized check shared by all four guards below. + template + inline bool numeric_bound_check(Type left, Type right, Compare compare) + { + return std::numeric_limits::is_specialized + ? compare(left, right) + : true; + } +} // namespace detail + +// checks if a value is at/below the lowest representable bound template struct numeric_minimum { @@ -61,26 +91,54 @@ struct numeric_minimum static bool is_less_than_or(Type, bool){ return true; } }; -template +template struct numeric_minimum, true> { static bool is_less_than(Type value) - { return std::less()((std::numeric_limits::min)(), value); } + { return detail::numeric_bound_check(std::numeric_limits::lowest(), value, std::less()); } static bool is_less_than_or(Type value, bool cond) { return cond || is_less_than(value); } }; -template +template struct numeric_minimum, true> { static bool is_less_than(Type value) - { return std::greater()((std::numeric_limits::max)(), value); } + { return detail::numeric_bound_check((std::numeric_limits::max)(), value, std::greater()); } static bool is_less_than_or(Type value, bool cond) { return cond || is_less_than(value); } }; +// checks if a value is at/above the highest representable bound +template +struct numeric_maximum +{ + static bool is_greater_than(Type){ return true; } + static bool is_greater_than_or(Type, bool){ return true; } +}; + +template +struct numeric_maximum, true> +{ + static bool is_greater_than(Type value) + { return detail::numeric_bound_check(value, (std::numeric_limits::max)(), std::less()); } + + static bool is_greater_than_or(Type value, bool cond) + { return cond || is_greater_than(value); } +}; + +template +struct numeric_maximum, true> +{ + static bool is_greater_than(Type value) + { return detail::numeric_bound_check(value, std::numeric_limits::lowest(), std::greater()); } + + static bool is_greater_than_or(Type value, bool cond) + { return cond || is_greater_than(value); } +}; + //-------------------------------------------------------------------------- template struct is_non_floating_point diff --git a/test/Jamfile.v2 b/test/Jamfile.v2 index ab50dd5..04b49ac 100644 --- a/test/Jamfile.v2 +++ b/test/Jamfile.v2 @@ -71,6 +71,7 @@ test-suite "itl" # Bug fixes -------------------------------------------------------------- [ run fix_icl_after_thread_/fix_icl_after_thread.cpp ] [ run fix_tickets_/fix_tickets.cpp ] + [ run fix_icl_swo_/fix_icl_swo.cpp ] # Check for compiler bugs ------------------------------------------------ # compile-fail-tests diff --git a/test/fix_icl_swo_/fix_icl_swo.cpp b/test/fix_icl_swo_/fix_icl_swo.cpp new file mode 100644 index 0000000..84dbd54 --- /dev/null +++ b/test/fix_icl_swo_/fix_icl_swo.cpp @@ -0,0 +1,411 @@ +// ============================================================================= +// Boost.Test suite for the Boost.ICL strict-weak-ordering search fix. +// +// interval_base_set / interval_base_map lower_bound / upper_bound / equal_range use +// exclusive_less_than, which is NOT a strict weak ordering for a query that overlaps +// several disjoint stored intervals. libc++ >= LLVM 22 optimized those lookups into a +// single root-to-leaf descent that assumes SWO, so the unpatched accessors can return +// the wrong boundary (and the interval_map build path can abort in gap_insert). +// +// This file merges two suites: +// * accessor / decomposition tests on continuous (dynamic) intervals, incl. the +// construction case that aborts on an unpatched libc++ >= 22 build; +// * the PR #57 interval-type matrix proving the SWO-safe accessors return ICL's +// documented boundaries for every interval type (static & dynamic, discrete & +// continuous), including the static-continuous case fixed by the point-lookup +// fallback (Patch B). +// +// Header-only Boost.Test (no link step needed): +// c++ -std=c++17 -I test_icl_swo_fix.cpp -o test_icl_swo_fix +// ./test_icl_swo_fix +// +// Build against clang-22 + libc++ (the failing config) WITHOUT defining +// _LIBCPP_ENABLE_LEGACY_TREE_LOWER_UPPER_BOUND: +// * UNPATCHED ICL -> the construction case aborts in gap_insert, or (asserts off) +// the accessor/decomposition checks FAIL. +// * PATCHED ICL -> all test cases pass. +// On libstdc++ / clang<=21 every case passes either way (the assertions encode the +// correct boundary). +// +// Contract under test (exclusive_less_than `prec`): +// lower_bound(Q) = first stored E with !prec(E,Q) -> leftmost E overlapping Q, +// else first E entirely right of Q +// upper_bound(Q) = first stored E with prec(Q,E) -> first E entirely right of Q +// [lower_bound(Q), upper_bound(Q)) = the contiguous run of intervals overlapping Q +// ============================================================================= +#define BOOST_TEST_MODULE icl_swo_search_fix +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include // construct + +#include +#include +#include +#include + +using namespace boost::icl; + +// ---- shared helpers -------------------------------------------------------------------------- + +template +static std::string br(const IvT& i) +{ + std::ostringstream os; + os << left_bracket(i) << i.lower() << ',' << i.upper() + << right_bracket(i); + return os.str(); +} + +template +static std::string dump(const SetT& s) +{ + std::ostringstream os; + for (auto& e : s) os << br(e) << ' '; + return os.str(); +} + +// Brute-force reference: the contiguous run of stored intervals overlapping q, as [first,end). +// overlap == neither side exclusively-less than the other. +template +static void brute_overlap_range(const SetT& s, const typename SetT::key_type& q, + typename SetT::const_iterator& bf, + typename SetT::const_iterator& be) +{ + typename SetT::key_compare comp; + auto overlaps = [&](const typename SetT::key_type& a, const typename SetT::key_type& b) + { return !comp(a, b) && !comp(b, a); }; + bf = s.end(); be = s.end(); + for (auto it = s.begin(); it != s.end(); ++it) + if (overlaps(*it, q)) { if (bf == s.end()) bf = it; be = std::next(it); } +} + +// Matrix helper: build {ivs...}, query Q, expect lower/upper at the given bounds. +template +static void check_run(SetT& s, const IvT& Q, + typename SetT::domain_type lb_a, typename SetT::domain_type lb_b, + bool upper_is_end, + typename SetT::domain_type ub_a = 0, + typename SetT::domain_type ub_b = 0) +{ + auto lb = s.lower_bound(Q); + BOOST_CHECK(lb != s.end()); + if (lb != s.end()) BOOST_CHECK(*lb == construct(lb_a, lb_b)); + + auto ub = s.upper_bound(Q); + if (upper_is_end) BOOST_CHECK(ub == s.end()); + else { BOOST_CHECK(ub != s.end()); if (ub != s.end()) BOOST_CHECK(*ub == construct(ub_a, ub_b)); } + + auto er = s.equal_range(Q); + BOOST_CHECK(er.first == lb); + BOOST_CHECK(er.second == ub); +} + +// ============================================================================================= +// Accessors: interval_set returns the FULL overlapping run (no crash needed). +// ============================================================================================= +BOOST_AUTO_TEST_CASE(set_accessors_overlap_run) +{ + using Iv = interval::type; // continuous_interval by default + using ISet = split_interval_set; + + // Two entries sharing the boundary 1: split_interval_set stores [0,1] and (1,3] separately, + // the layout that makes exclusive_less_than's overlap-equivalence intransitive for a query + // straddling 1. + ISet s; + s += Iv::closed(0, 1); + s += Iv::closed(0, 3); + BOOST_CHECK_MESSAGE(dump(s) == "[0,1] (1,3] ", + "set layout precondition: got '" + dump(s) + "'"); + + struct Case { Iv q; const char* name; }; + const Case cases[] = { + { Iv::closed(0, 3), "Q=[0,3] straddles boundary 1, overlaps BOTH entries" }, + { Iv::closed(1, 1), "Q=[1,1] singleton at the shared boundary, overlaps BOTH" }, + { Iv::closed(0, 1), "Q=[0,1] overlaps [0,1], only touches (1,3] at open 1" }, + { Iv::open(1, 3), "Q=(1,3) overlaps only (1,3]" }, + { Iv::closed(2, 5), "Q=[2,5] overlaps only (1,3]" }, + { Iv::right_open(0, 1), "Q=[0,1) overlaps only [0,1]" }, + { Iv::closed(4, 5), "Q=[4,5] empty run, right of everything" }, + { Iv::closed(-2, -1), "Q=[-2,-1] empty run, left of everything" }, + }; + + for (auto& c : cases) { + ISet::const_iterator bf, be; + brute_overlap_range(s, c.q, bf, be); + const bool empty_run = (bf == be); + + auto lb = s.lower_bound(c.q); + auto ub = s.upper_bound(c.q); + auto er = s.equal_range(c.q); + + if (empty_run) { + // Only contract for an empty run is that the range is EMPTY (lb == ub). The exact + // iterator denoting "empty" is unspecified, so it is not pinned to an element. + BOOST_CHECK_MESSAGE(lb == ub, std::string("empty range lb==ub ") + c.name); + BOOST_CHECK_MESSAGE(er.first == er.second, std::string("empty range equal_range ") + c.name); + } else { + BOOST_CHECK_MESSAGE(lb == bf, std::string("lower_bound ") + c.name); + BOOST_CHECK_MESSAGE(ub == be, std::string("upper_bound ") + c.name); + BOOST_CHECK_MESSAGE(er.first == bf, std::string("equal_range.first ") + c.name); + BOOST_CHECK_MESSAGE(er.second == be, std::string("equal_range.second ") + c.name); + BOOST_CHECK_MESSAGE(std::distance(lb, ub) > 0, std::string("range not inverted ") + c.name); + } + } +} + +// ============================================================================================= +// Decomposition: interval_map is correct for a query spanning multiple stored entries +// (also exercises the unbounded upper-bound walk: Q spans the whole run). +// ============================================================================================= +BOOST_AUTO_TEST_CASE(map_multispan_decomposition) +{ + using Iv = interval::type; + using IMap = interval_map>; + auto dumpm = [](const IMap& m) { + std::ostringstream os; + for (auto& e : m) { + os << br(e.first) << '{'; + for (int v : e.second) os << v; + os << "} "; + } + return os.str(); + }; + + IMap m; + m += std::make_pair(Iv::right_open(0, 1), std::set{0}); + m += std::make_pair(Iv::right_open(1, 2), std::set{1}); + m += std::make_pair(Iv::right_open(2, 3), std::set{2}); + m += std::make_pair(Iv::closed(0, 3), std::set{9}); // spans all three + the point 3 + + const std::string expected = "[0,1){09} [1,2){19} [2,3){29} [3,3]{9} "; + BOOST_CHECK_MESSAGE(dumpm(m) == expected, "map multispan decomposition: got '" + dumpm(m) + "'"); +} + +// ============================================================================================= +// PR #57 interval-type matrix. +// ============================================================================================= + +// ---- right_open_interval (static, discrete) : honnesh's original case ---- +BOOST_AUTO_TEST_CASE(pr57_right_open_int) +{ + typedef right_open_interval Iv; + interval_set s; + s.add(Iv(0,2)); s.add(Iv(3,5)); s.add(Iv(6,9)); s.add(Iv(10,12)); + + check_run(s, Iv(4,7), /*lb*/3,5, /*ub*/false, 10,12); // Q=[4,7) overlaps [3,5) and [6,9) + + // touching at an OPEN upper border: a stored interval starting exactly at upper(Q). + interval_set t; + t.add(Iv(3,5)); t.add(Iv(7,9)); + check_run(t, Iv(1,3), /*lb*/3,5, /*ub*/false, 3,5); // overlaps nothing; lb==ub==[3,5) + check_run(t, Iv(1,4), /*lb*/3,5, /*ub*/false, 7,9); // overlaps [3,5); ub=[7,9) +} + +// ---- closed_interval (static, discrete) ---- +BOOST_AUTO_TEST_CASE(pr57_closed_int) +{ + typedef closed_interval Iv; + interval_set s; + s.add(Iv(0,1)); s.add(Iv(3,4)); s.add(Iv(6,8)); s.add(Iv(10,11)); + + check_run(s, Iv(4,7), /*lb*/3,4, /*ub*/false, 10,11); // Q=[4,7] overlaps [3,4] and [6,8] +} + +// ---- left_open_interval (static, discrete) ---- +BOOST_AUTO_TEST_CASE(pr57_left_open_int) +{ + typedef left_open_interval Iv; // (a,b] = {a+1..b} + interval_set s; + s.add(Iv(-1,1)); s.add(Iv(2,4)); s.add(Iv(5,8)); s.add(Iv(9,11)); + + check_run(s, Iv(3,7), /*lb*/2,4, /*ub*/false, 9,11); // Q=(3,7] overlaps (2,4] and (5,8] +} + +// ---- open_interval (static, discrete) ---- +BOOST_AUTO_TEST_CASE(pr57_open_int) +{ + typedef open_interval Iv; // (a,b) = {a+1..b-1} + interval_set s; + s.add(Iv(-1,2)); s.add(Iv(2,5)); s.add(Iv(5,9)); s.add(Iv(9,12)); + + check_run(s, Iv(3,7), /*lb*/2,5, /*ub*/false, 9,12); // Q=(3,7) overlaps (2,5) and (5,9) +} + +// ---- discrete_interval (DYNAMIC bounds, discrete) ---- +BOOST_AUTO_TEST_CASE(pr57_discrete_interval_int) +{ + typedef discrete_interval Iv; + interval_set s; + s.add(interval::right_open(0,2)); + s.add(interval::right_open(3,5)); + s.add(interval::right_open(6,9)); + s.add(interval::right_open(10,12)); + + Iv Q = interval::right_open(4,7); + auto lb = s.lower_bound(Q); BOOST_CHECK(lb != s.end() && lb->lower()==3); + auto ub = s.upper_bound(Q); BOOST_CHECK(ub != s.end() && ub->lower()==10); +} + +// ---- continuous_interval (DYNAMIC bounds, continuous) ---- +BOOST_AUTO_TEST_CASE(pr57_continuous_interval_double) +{ + typedef continuous_interval Iv; + interval_set s; + s.add(interval::right_open(0.0, 2.0)); + s.add(interval::right_open(3.0, 5.0)); + s.add(interval::right_open(6.0, 9.0)); + s.add(interval::right_open(10.0,12.0)); + + Iv Q = interval::right_open(4.0, 7.0); + auto lb = s.lower_bound(Q); BOOST_CHECK(lb != s.end() && lb->lower()==3.0); + auto ub = s.upper_bound(Q); BOOST_CHECK(ub != s.end() && ub->lower()==10.0); + + // open/closed mix + interval_set t; + t.add(interval::closed(0.0, 1.0)); + t.add(interval::open (2.0, 4.0)); // (2,4) + t.add(interval::closed(5.0, 7.0)); + Iv Q2 = interval::closed(3.0, 6.0); // overlaps (2,4) and [5,7] + auto lb2 = t.lower_bound(Q2); BOOST_CHECK(lb2 != t.end() && lb2->lower()==2.0); + auto ub2 = t.upper_bound(Q2); BOOST_CHECK(ub2 == t.end()); +} + +// ---- right_open_interval (STATIC bounds, continuous) : Patch B point fallback ---- +// has_static_bounds && is_continuous. No singleton exists on a continuous domain, so the +// SWO-safe accessor anchors on a domain POINT via heterogeneous lookup. Pre-patch this is the +// raw call: UB on libc++22 (returns [6,9) instead of [10,12)). +BOOST_AUTO_TEST_CASE(pr57_right_open_double_static) +{ + typedef right_open_interval Iv; + interval_set s; + s.add(Iv(0.0,2.0)); s.add(Iv(3.0,5.0)); s.add(Iv(6.0,9.0)); s.add(Iv(10.0,12.0)); + + auto lb = s.lower_bound(Iv(4.0,7.0)); BOOST_CHECK(lb != s.end() && lb->lower()==3.0); + auto ub = s.upper_bound(Iv(4.0,7.0)); BOOST_CHECK(ub != s.end() && ub->lower()==10.0); // pre-patch: 6.0 +} + +// ============================================================================================= +// LAST: interval_map construction that ABORTS on unpatched libc++ >= LLVM 22. +// Declared last so the checks above run first; on an unpatched build BOOST_ASSERT in gap_insert +// aborts the process here. +// ============================================================================================= +BOOST_AUTO_TEST_CASE(map_crash_case_closed_shared_lower) +{ + using Iv = interval::type; + using IMap = interval_map>; + auto dumpm = [](const IMap& m) { + std::ostringstream os; + for (auto& e : m) { + os << br(e.first) << '{'; + for (int v : e.second) os << v; + os << "} "; + } + return os.str(); + }; + + IMap m; + m += std::make_pair(Iv::closed(0, 1), std::set{0}); + m += std::make_pair(Iv::closed(0, 3), std::set{1}); + m += std::make_pair(Iv::closed(0, 2), std::set{2}); // aborts here if unpatched + + const std::string expected = "[0,1]{012} (1,2]{12} (2,3]{1} "; + BOOST_CHECK_MESSAGE(dumpm(m) == expected, "map crash-case decomposition: got '" + dumpm(m) + "'"); +} + + +// interval_set: add an interval overlapping a stored one. +// Exercises the blind-add path: the overlapping insert must be rejected as a +// collision and merged, regardless of the std lib's insertion algorithm. +BOOST_AUTO_TEST_CASE(set_overlapping_add_is_swo_safe) +{ + interval_set s; + s += interval::right_open(0, 5); // stored, disjoint: {[0,5)} + + BOOST_CHECK_NO_THROW( + s += interval::right_open(3, 8)); // overlaps [0,5) + + // Sanity: the merged result must be a single [0,8). + interval_set expected; + expected += interval::right_open(0, 8); + BOOST_CHECK_EQUAL(s, expected); +} + +// interval_map: add an interval overlapping a stored one. +// Same blind-add path as above; must merge without throwing or corrupting. +BOOST_AUTO_TEST_CASE(map_overlapping_add_is_swo_safe) +{ + interval_map m; + m += std::make_pair(interval::right_open(0, 5), 1); // {[0,5)->1} + + BOOST_CHECK_NO_THROW( + m += std::make_pair(interval::right_open(3, 8), 1)); +} + +// interval_set: multi-interval overlap (collision run). +// The added interval overlaps a contiguous run of stored intervals; the insert +// must land on the run and merge all of them into one. +BOOST_AUTO_TEST_CASE(set_multi_overlap_add_is_swo_safe) +{ + interval_set s; + s += interval::right_open(0, 5); // {[0,5), [10,15)} + s += interval::right_open(10, 15); + + BOOST_CHECK_NO_THROW( + s += interval::right_open(3, 12));// overlaps BOTH stored intervals + + interval_set expected; + expected += interval::right_open(0, 15); + BOOST_CHECK_EQUAL(s, expected); +} + +// interval_set: HINTED add overload with a deliberately stale hint. +// The public add(prior,.) forwards a caller hint with a possibly-overlapping +// argument. Tested standard libraries reject the overlap safely here (the +// O(1)-hint neighbour check fails and they fall back to a full search), so this +// is a robustness guard, not a reproduction of a known corruption. A hint must +// never change the result, so compare against the un-hinted add. +BOOST_AUTO_TEST_CASE(set_hinted_overlapping_add_is_swo_safe) +{ + interval_set s; + s += interval::right_open(0, 1); + s += interval::right_open(2, 3); + s += interval::right_open(4, 5); // {[0,1),[2,3),[4,5)} + + BOOST_CHECK_NO_THROW( + s.add(std::prev(s.end()), interval::right_open(0, 3))); + + interval_set expected; + expected += interval::right_open(0, 1); + expected += interval::right_open(2, 3); + expected += interval::right_open(4, 5); + expected += interval::right_open(0, 3); // un-hinted, known-correct + BOOST_CHECK_EQUAL(s, expected); +} + +// interval_map: HINTED add overload with a stale hint, same robustness guard. +BOOST_AUTO_TEST_CASE(map_hinted_overlapping_add_is_swo_safe) +{ + interval_map m; + m += std::make_pair(interval::right_open(0, 1), 1); + m += std::make_pair(interval::right_open(2, 3), 1); + m += std::make_pair(interval::right_open(4, 5), 1); + + BOOST_CHECK_NO_THROW( + m.add(std::prev(m.end()), std::make_pair(interval::right_open(0, 3), 1))); + + interval_map expected; + expected += std::make_pair(interval::right_open(0, 1), 1); + expected += std::make_pair(interval::right_open(2, 3), 1); + expected += std::make_pair(interval::right_open(4, 5), 1); + expected += std::make_pair(interval::right_open(0, 3), 1); // un-hinted, known-correct + BOOST_CHECK_EQUAL(m, expected); +} diff --git a/test/fix_tickets_/fix_tickets.cpp b/test/fix_tickets_/fix_tickets.cpp index cb6524c..a25c391 100644 --- a/test/fix_tickets_/fix_tickets.cpp +++ b/test/fix_tickets_/fix_tickets.cpp @@ -249,3 +249,381 @@ BOOST_AUTO_TEST_CASE(github_25_intersects_for_empty_interval_throws) BOOST_CHECK_EQUAL(false, intersects(mt_set, Itv::right_open(-1,-1))); } +//------------------------------------------------------------------------------ +// GitHub issue #51: exclusive_less_than strict weak ordering violation +// +// The following tests verify the expected semantics of lower_bound, +// upper_bound, equal_range, and find on interval containers. +//------------------------------------------------------------------------------ + +// interval_set tests +BOOST_AUTO_TEST_CASE(github_51_interval_set_upper_bound) +{ + typedef boost::icl::interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + // upper_bound with query overlapping multiple stored intervals + { + ItvSet::iterator it = s.upper_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(40, 50)); + } + + // upper_bound past all intervals + { + ItvSet::iterator it = s.upper_bound(Itv::right_open(5, 55)); + BOOST_CHECK(it == s.end()); + } +} + +BOOST_AUTO_TEST_CASE(github_51_interval_set_lower_bound) +{ + typedef boost::icl::interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + // lower_bound with query overlapping first and second intervals + { + ItvSet::iterator it = s.lower_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(0, 10)); + } + + // lower_bound for interval before all stored + { + ItvSet::iterator it = s.lower_bound(Itv::right_open(-5, -1)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(0, 10)); + } +} + +BOOST_AUTO_TEST_CASE(github_51_interval_set_equal_range) +{ + typedef boost::icl::interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + // equal_range covering first two intervals + { + std::pair range = + s.equal_range(Itv::right_open(5, 25)); + int count = 0; + for (ItvSet::iterator it = range.first; it != range.second; ++it) + ++count; + BOOST_CHECK_EQUAL(count, 2); + } + + // equal_range covering all intervals + { + std::pair range = + s.equal_range(Itv::right_open(-5, 55)); + int count = 0; + for (ItvSet::iterator it = range.first; it != range.second; ++it) + ++count; + BOOST_CHECK_EQUAL(count, 3); + } +} + +BOOST_AUTO_TEST_CASE(github_51_interval_set_find) +{ + typedef boost::icl::interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + // find for overlapping interval returns first overlapping + { + ItvSet::const_iterator it = s.find(Itv::right_open(5, 25)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(0, 10)); + } + + // find for non-overlapping interval returns end + { + ItvSet::const_iterator it = s.find(Itv::right_open(10, 20)); + BOOST_CHECK(it == s.end()); + } +} + +// split_interval_set tests +BOOST_AUTO_TEST_CASE(github_51_split_interval_set_upper_bound) +{ + typedef boost::icl::split_interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + { + ItvSet::iterator it = s.upper_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(40, 50)); + } +} + +BOOST_AUTO_TEST_CASE(github_51_split_interval_set_lower_bound) +{ + typedef boost::icl::split_interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + { + ItvSet::iterator it = s.lower_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(0, 10)); + } +} + +BOOST_AUTO_TEST_CASE(github_51_split_interval_set_equal_range) +{ + typedef boost::icl::split_interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + { + std::pair range = + s.equal_range(Itv::right_open(5, 25)); + int count = 0; + for (ItvSet::iterator it = range.first; it != range.second; ++it) + ++count; + BOOST_CHECK_EQUAL(count, 2); + } +} + +BOOST_AUTO_TEST_CASE(github_51_split_interval_set_find) +{ + typedef boost::icl::split_interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + { + ItvSet::const_iterator it = s.find(Itv::right_open(5, 25)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(0, 10)); + } + + { + ItvSet::const_iterator it = s.find(Itv::right_open(10, 20)); + BOOST_CHECK(it == s.end()); + } +} + +// separate_interval_set tests +BOOST_AUTO_TEST_CASE(github_51_separate_interval_set_upper_bound) +{ + typedef boost::icl::separate_interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + { + ItvSet::iterator it = s.upper_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(40, 50)); + } +} + +BOOST_AUTO_TEST_CASE(github_51_separate_interval_set_lower_bound) +{ + typedef boost::icl::separate_interval_set ItvSet; + typedef ItvSet::interval_type Itv; + + ItvSet s; + s.add(Itv::right_open(0, 10)); + s.add(Itv::right_open(20, 30)); + s.add(Itv::right_open(40, 50)); + + { + ItvSet::iterator it = s.lower_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != s.end()); + BOOST_CHECK_EQUAL(*it, Itv::right_open(0, 10)); + } +} + +// interval_map tests +BOOST_AUTO_TEST_CASE(github_51_interval_map_upper_bound) +{ + typedef boost::icl::interval_map ItvMap; + typedef ItvMap::interval_type Itv; + + ItvMap m; + m.add(make_pair(Itv::right_open(0, 10), 1)); + m.add(make_pair(Itv::right_open(20, 30), 2)); + m.add(make_pair(Itv::right_open(40, 50), 3)); + + { + ItvMap::iterator it = m.upper_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != m.end()); + BOOST_CHECK_EQUAL(it->first, Itv::right_open(40, 50)); + } +} + +BOOST_AUTO_TEST_CASE(github_51_interval_map_lower_bound) +{ + typedef boost::icl::interval_map ItvMap; + typedef ItvMap::interval_type Itv; + + ItvMap m; + m.add(make_pair(Itv::right_open(0, 10), 1)); + m.add(make_pair(Itv::right_open(20, 30), 2)); + m.add(make_pair(Itv::right_open(40, 50), 3)); + + { + ItvMap::iterator it = m.lower_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != m.end()); + BOOST_CHECK_EQUAL(it->first, Itv::right_open(0, 10)); + } +} + +BOOST_AUTO_TEST_CASE(github_51_interval_map_equal_range) +{ + typedef boost::icl::interval_map ItvMap; + typedef ItvMap::interval_type Itv; + + ItvMap m; + m.add(make_pair(Itv::right_open(0, 10), 1)); + m.add(make_pair(Itv::right_open(20, 30), 2)); + m.add(make_pair(Itv::right_open(40, 50), 3)); + + { + std::pair range = + m.equal_range(Itv::right_open(5, 25)); + int count = 0; + for (ItvMap::iterator it = range.first; it != range.second; ++it) + ++count; + BOOST_CHECK_EQUAL(count, 2); + } +} + +BOOST_AUTO_TEST_CASE(github_51_interval_map_find) +{ + typedef boost::icl::interval_map ItvMap; + typedef ItvMap::interval_type Itv; + + ItvMap m; + m.add(make_pair(Itv::right_open(0, 10), 1)); + m.add(make_pair(Itv::right_open(20, 30), 2)); + m.add(make_pair(Itv::right_open(40, 50), 3)); + + { + ItvMap::const_iterator it = m.find(Itv::right_open(5, 25)); + BOOST_CHECK(it != m.end()); + BOOST_CHECK_EQUAL(it->first, Itv::right_open(0, 10)); + } + + { + ItvMap::const_iterator it = m.find(Itv::right_open(10, 20)); + BOOST_CHECK(it == m.end()); + } +} + +// split_interval_map tests +BOOST_AUTO_TEST_CASE(github_51_split_interval_map_upper_bound) +{ + typedef boost::icl::split_interval_map ItvMap; + typedef ItvMap::interval_type Itv; + + ItvMap m; + m.add(make_pair(Itv::right_open(0, 10), 1)); + m.add(make_pair(Itv::right_open(20, 30), 2)); + m.add(make_pair(Itv::right_open(40, 50), 3)); + + { + ItvMap::iterator it = m.upper_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != m.end()); + BOOST_CHECK_EQUAL(it->first, Itv::right_open(40, 50)); + } +} + +BOOST_AUTO_TEST_CASE(github_51_split_interval_map_lower_bound) +{ + typedef boost::icl::split_interval_map ItvMap; + typedef ItvMap::interval_type Itv; + + ItvMap m; + m.add(make_pair(Itv::right_open(0, 10), 1)); + m.add(make_pair(Itv::right_open(20, 30), 2)); + m.add(make_pair(Itv::right_open(40, 50), 3)); + + { + ItvMap::iterator it = m.lower_bound(Itv::right_open(5, 25)); + BOOST_CHECK(it != m.end()); + BOOST_CHECK_EQUAL(it->first, Itv::right_open(0, 10)); + } +} + +BOOST_AUTO_TEST_CASE(github_51_split_interval_map_equal_range) +{ + typedef boost::icl::split_interval_map ItvMap; + typedef ItvMap::interval_type Itv; + + ItvMap m; + m.add(make_pair(Itv::right_open(0, 10), 1)); + m.add(make_pair(Itv::right_open(20, 30), 2)); + m.add(make_pair(Itv::right_open(40, 50), 3)); + + { + std::pair range = + m.equal_range(Itv::right_open(5, 25)); + int count = 0; + for (ItvMap::iterator it = range.first; it != range.second; ++it) + ++count; + BOOST_CHECK_EQUAL(count, 2); + } +} + +BOOST_AUTO_TEST_CASE(github_51_split_interval_map_find) +{ + typedef boost::icl::split_interval_map ItvMap; + typedef ItvMap::interval_type Itv; + + ItvMap m; + m.add(make_pair(Itv::right_open(0, 10), 1)); + m.add(make_pair(Itv::right_open(20, 30), 2)); + m.add(make_pair(Itv::right_open(40, 50), 3)); + + { + ItvMap::const_iterator it = m.find(Itv::right_open(5, 25)); + BOOST_CHECK(it != m.end()); + BOOST_CHECK_EQUAL(it->first, Itv::right_open(0, 10)); + } + + { + ItvMap::const_iterator it = m.find(Itv::right_open(10, 20)); + BOOST_CHECK(it == m.end()); + } +} + diff --git a/test/test_icl_interval_/test_icl_interval.cpp b/test/test_icl_interval_/test_icl_interval.cpp index 7380cf8..be6554e 100644 --- a/test/test_icl_interval_/test_icl_interval.cpp +++ b/test/test_icl_interval_/test_icl_interval.cpp @@ -36,6 +36,7 @@ using namespace boost::icl; #include #include +#include //- sta.asy.{dis|con} ---------------------------------------------------------- BOOST_AUTO_TEST_CASE_TEMPLATE @@ -100,6 +101,37 @@ BOOST_AUTO_TEST_CASE_TEMPLATE (fastest_itl_left_open_interval_4_bicremental_types, T, discrete_types) { singelizable_interval_4_bicremental_types >(); } +// first()/last() must be symmetric guards over the open boundary. +// last() asserts numeric_minimum (domain_prior can't underflow); first() must +// likewise assert numeric_maximum (domain_next can't overflow). This checks the +// guards pass on valid intervals and that both first() overloads still return +// the correct boundary element. See https://github.com/boostorg/icl/issues/58 +BOOST_AUTO_TEST_CASE(test_itl_interval_first_last_symmetry) +{ + // Static interval types -> first()/last() overloads gated on is_static_*. + BOOST_CHECK_EQUAL(icl::first(right_open_interval(2,7)), 2); // [2,7) + BOOST_CHECK_EQUAL(icl::first(left_open_interval (2,7)), 3); // (2,7] + BOOST_CHECK_EQUAL(icl::first(open_interval (2,7)), 3); // (2,7) + BOOST_CHECK_EQUAL(icl::first(closed_interval (2,7)), 2); // [2,7] + + BOOST_CHECK_EQUAL(icl::last (right_open_interval(2,7)), 6); // [2,7) + BOOST_CHECK_EQUAL(icl::last (left_open_interval (2,7)), 7); // (2,7] + BOOST_CHECK_EQUAL(icl::last (open_interval (2,7)), 6); // (2,7) + BOOST_CHECK_EQUAL(icl::last (closed_interval (2,7)), 7); // [2,7] + + // Dynamic bounds -> the is_discrete_interval first()/last() overloads, + // exercising both branches of the is_left_closed/is_right_closed guards. + BOOST_CHECK_EQUAL(icl::first(interval::right_open(2,7)), 2); + BOOST_CHECK_EQUAL(icl::first(interval::left_open (2,7)), 3); + BOOST_CHECK_EQUAL(icl::first(interval::open (2,7)), 3); + BOOST_CHECK_EQUAL(icl::first(interval::closed (2,7)), 2); + + BOOST_CHECK_EQUAL(icl::last (interval::right_open(2,7)), 6); + BOOST_CHECK_EQUAL(icl::last (interval::left_open (2,7)), 7); + BOOST_CHECK_EQUAL(icl::last (interval::open (2,7)), 6); + BOOST_CHECK_EQUAL(icl::last (interval::closed (2,7)), 7); +} + //- dyn.dis -------------------------------------------------------------------- BOOST_AUTO_TEST_CASE_TEMPLATE (fastest_itl_discrete_interval_ctor_4_discrete_types_base, T, discrete_types) diff --git a/test/test_type_traits_/test_type_traits.cpp b/test/test_type_traits_/test_type_traits.cpp index 5abe196..244d4da 100644 --- a/test/test_type_traits_/test_type_traits.cpp +++ b/test/test_type_traits_/test_type_traits.cpp @@ -10,6 +10,7 @@ Copyright (c) 2008-2009: Joachim Faulhaber #include #include #include +#include #include #include #include @@ -59,6 +60,34 @@ BOOST_AUTO_TEST_CASE_TEMPLATE(test_is_continuous_type_T, T, continuous_types) BOOST_CHECK(!is_discrete::value); } +// https://github.com/boostorg/icl/issues/58 +// numeric_minimum/numeric_maximum must not consult std::numeric_limits unless +// it is actually specialized, and must use lowest() (not min()) for the lower +// extreme so non-integral domains are handled soundly. +BOOST_AUTO_TEST_CASE(test_numeric_minimum_maximum_bounds) +{ + typedef boost::rational Q; + + // Integral: lowest()==min(), behavior preserved. + BOOST_CHECK(( numeric_minimum, is_numeric::value>::is_less_than(-1))); + BOOST_CHECK((!numeric_minimum, is_numeric::value>::is_less_than((std::numeric_limits::min)()))); + BOOST_CHECK(( numeric_maximum, is_numeric::value>::is_greater_than(1))); + BOOST_CHECK((!numeric_maximum, is_numeric::value>::is_greater_than((std::numeric_limits::max)()))); + + // Floating point: the min()/lowest() trap. A negative value is a valid lower bound. + BOOST_CHECK(( numeric_minimum, is_numeric::value>::is_less_than(-1.0))); + BOOST_CHECK((!numeric_minimum, is_numeric::value>::is_less_than(std::numeric_limits::lowest()))); + BOOST_CHECK(( numeric_maximum, is_numeric::value>::is_greater_than(-1.0))); + BOOST_CHECK((!numeric_maximum, is_numeric::value>::is_greater_than(std::numeric_limits::lowest()))); + + // boost::rational: is_numeric==true but numeric_limits unspecialized -> impose no constraint. + BOOST_CHECK((std::numeric_limits::is_specialized == false)); + BOOST_CHECK(( numeric_minimum, is_numeric::value>::is_less_than(Q(-1,1)))); + BOOST_CHECK(( numeric_minimum, is_numeric::value>::is_less_than(Q( 0,1)))); + BOOST_CHECK(( numeric_maximum, is_numeric::value>::is_greater_than(Q(-1,1)))); + BOOST_CHECK(( numeric_maximum, is_numeric::value>::is_greater_than(Q( 0,1)))); +} + BOOST_AUTO_TEST_CASE(test_is_continuous_type) { BOOST_CHECK(is_continuous >::value);