diff --git a/oss/interval_tree/MAINTAINER_README.md b/oss/interval_tree/MAINTAINER_README.md index edc430dfc1b..402857f5d63 100644 --- a/oss/interval_tree/MAINTAINER_README.md +++ b/oss/interval_tree/MAINTAINER_README.md @@ -1,17 +1,32 @@ -# Notes for Future Maintainers - -This was originally imported by @PankajBhojwani in September 2020. - -The provenance information (where it came from and which commit) is stored in the file `cgmanifest.json` in the same directory as this readme. -Please update the provenance information in that file when ingesting an updated version of the dependent library. -That provenance file is automatically read and inventoried by Microsoft systems to ensure compliance with appropriate governance standards. - -## What should be done to update this in the future? - -1. Go to the ekg/intervaltree repository on GitHub. -2. Take the file IntervalTree.h wholesale and drop it into the directory here. -3. Don't change anything about it. -4. Validate that the license in the root of the repository didn't change and update it if so. It is sitting in the same directory as this readme. - If it changed dramatically, ensure that it is still compatible with our license scheme. Also update the NOTICE file in the root of our repository to declare the third-party usage. -5. Submit the pull. - +# Notes for Future Maintainers + +This was originally imported by @PankajBhojwani in September 2020. + +The provenance information (where it came from and which commit) is stored in the file `cgmanifest.json` in the same directory as this readme. +Please update the provenance information in that file when ingesting an updated version of the dependent library. +That provenance file is automatically read and inventoried by Microsoft systems to ensure compliance with appropriate governance standards. + +## Local deviation from upstream + +**`IntervalTree.h` here is _not_ a byte-for-byte copy of upstream.** Do not assume it is. +`Scalar` is compared against `Scalar{}` instead of `0`, and `center` is default-initialized +rather than set to `0`, so that a user-defined struct such as `til::point` can be used as a +key. This came in with the original import in #7691 and corresponds to the still-open +upstream pull request . Without it this file +does not compile against `til::point` at all. + +Note that `is_valid()` assumes `std::numeric_limits` is meaningful. We satisfy that +by specializing `std::numeric_limits` for the `til` coordinate types rather than by patching +this file; see `src/inc/til/point.h` and #20486. + +## What should be done to update this in the future? + +1. Go to the ekg/intervaltree repository on GitHub. +2. Take the file IntervalTree.h wholesale and drop it into the directory here. +3. Don't change anything about it, **except** that you must re-apply the local deviation + listed above. Taking upstream wholesale without it will break the build, because + `til::point` cannot be compared against `0`. +4. Validate that the license in the root of the repository didn't change and update it if so. It is sitting in the same directory as this readme. + If it changed dramatically, ensure that it is still compatible with our license scheme. Also update the NOTICE file in the root of our repository to declare the third-party usage. +5. Submit the pull. + diff --git a/src/inc/LibraryIncludes.h b/src/inc/LibraryIncludes.h index 18ad9c6c8c8..aa0191d61b7 100644 --- a/src/inc/LibraryIncludes.h +++ b/src/inc/LibraryIncludes.h @@ -30,6 +30,7 @@ #include #include #include +#include #include #include #include diff --git a/src/inc/til/point.h b/src/inc/til/point.h index 4ac3bd7cfc2..887d03b4893 100644 --- a/src/inc/til/point.h +++ b/src/inc/til/point.h @@ -374,6 +374,32 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" }; } +// til::point is ordered (lexicographically, by y then x), so the extremes are +// well defined. Generic code that needs a "smaller/larger than anything" seed +// value reaches for std::numeric_limits, and the unspecialized primary template +// silently answers til::point{} for both max() and min() -- see GH#20486. +template<> +class std::numeric_limits +{ +public: + static constexpr bool is_specialized = true; + + static constexpr til::point min() noexcept + { + return { til::CoordTypeMin, til::CoordTypeMin }; + } + + static constexpr til::point max() noexcept + { + return { til::CoordTypeMax, til::CoordTypeMax }; + } + + static constexpr til::point lowest() noexcept + { + return min(); + } +}; + #ifdef __WEX_COMMON_H__ namespace WEX::TestExecution { diff --git a/src/inc/til/size.h b/src/inc/til/size.h index 98800ad7539..88a6856ba29 100644 --- a/src/inc/til/size.h +++ b/src/inc/til/size.h @@ -206,6 +206,30 @@ namespace til // Terminal Implementation Library. Also: "Today I Learned" } }; +// See the equivalent specialization in point.h. Without this, generic code that +// seeds an accumulator with std::numeric_limits gets til::size{} for both ends. +template<> +class std::numeric_limits +{ +public: + static constexpr bool is_specialized = true; + + static constexpr til::size min() noexcept + { + return { til::CoordTypeMin, til::CoordTypeMin }; + } + + static constexpr til::size max() noexcept + { + return { til::CoordTypeMax, til::CoordTypeMax }; + } + + static constexpr til::size lowest() noexcept + { + return min(); + } +}; + #ifdef __WEX_COMMON_H__ namespace WEX::TestExecution { diff --git a/src/til/ut_til/IntervalTreeTests.cpp b/src/til/ut_til/IntervalTreeTests.cpp new file mode 100644 index 00000000000..0d0f4e3a3c5 --- /dev/null +++ b/src/til/ut_til/IntervalTreeTests.cpp @@ -0,0 +1,75 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT license. + +#include "precomp.h" + +#include "til/point.h" + +using namespace WEX::Common; +using namespace WEX::Logging; +using namespace WEX::TestExecution; + +// These tests cover our use of oss/interval_tree with a non-arithmetic Scalar. +// See oss/interval_tree/MAINTAINER_README.md for the local deviations involved. +class IntervalTreeTests +{ + TEST_CLASS(IntervalTreeTests); + + using PointTree = interval_tree::IntervalTree; + + // The tree only splits into subtrees once it holds at least 64 intervals (its + // default minimum bucket size), so anything below that never exercised the + // subtree constraints. + static PointTree _makeTree(size_t count) + { + PointTree::interval_vector intervals; + intervals.reserve(count); + for (size_t i = 0; i < count; ++i) + { + const auto y = gsl::narrow(i); + intervals.push_back(PointTree::interval({ 0, y }, { 10, y }, i)); + } + return PointTree{ std::move(intervals) }; + } + + // GH#20486: is_valid() seeds its bounds accumulators from + // std::numeric_limits. Without a specialization the primary template + // answers til::point{0,0} for both ends, which is not a usable sentinel. + TEST_METHOD(NumericLimitsAreUsableSentinels) + { + VERIFY_IS_TRUE(std::numeric_limits::is_specialized); + VERIFY_IS_TRUE(std::numeric_limits::min() < til::point{}); + VERIFY_IS_TRUE(std::numeric_limits::max() > til::point{}); + VERIFY_ARE_EQUAL(std::numeric_limits::min(), std::numeric_limits::lowest()); + + VERIFY_IS_TRUE(std::numeric_limits::is_specialized); + VERIFY_ARE_EQUAL(til::CoordTypeMax, std::numeric_limits::max().width); + VERIFY_ARE_EQUAL(til::CoordTypeMin, std::numeric_limits::min().height); + } + + // Before GH#20486 this asserted in Debug builds for any tree with 64 or more + // intervals -- i.e. whenever 64+ URLs were autodetected on screen. + TEST_METHOD(IsValidWithSubtrees) + { + for (const size_t count : { 1u, 63u, 64u, 65u, 512u }) + { + const auto tree = _makeTree(count); + VERIFY_IS_TRUE(tree.is_valid().first, NoThrowString().Format(L"%zu intervals", count)); + } + } + + // A tree that reports itself valid must also still find what it contains. + TEST_METHOD(FindsIntervalsAcrossSubtrees) + { + static constexpr size_t count = 200; + const auto tree = _makeTree(count); + + for (size_t i = 0; i < count; ++i) + { + const auto y = gsl::narrow(i); + const auto results = tree.findOverlapping({ 5, y }, { 5, y }); + VERIFY_ARE_EQUAL(1u, results.size(), NoThrowString().Format(L"row %zu", i)); + VERIFY_ARE_EQUAL(i, results.front().value); + } + } +}; diff --git a/src/til/ut_til/sources b/src/til/ut_til/sources index ec02bb53cb0..1be320297b8 100644 --- a/src/til/ut_til/sources +++ b/src/til/ut_til/sources @@ -20,6 +20,7 @@ SOURCES = \ EnumSetTests.cpp \ EnvTests.cpp \ HashTests.cpp \ + IntervalTreeTests.cpp \ MathTests.cpp \ mutex.cpp \ OperatorTests.cpp \ diff --git a/src/til/ut_til/til.unit.tests.vcxproj b/src/til/ut_til/til.unit.tests.vcxproj index e3910f82182..c67036fa302 100644 --- a/src/til/ut_til/til.unit.tests.vcxproj +++ b/src/til/ut_til/til.unit.tests.vcxproj @@ -22,6 +22,7 @@ + diff --git a/src/til/ut_til/til.unit.tests.vcxproj.filters b/src/til/ut_til/til.unit.tests.vcxproj.filters index a3a38f11276..80958390710 100644 --- a/src/til/ut_til/til.unit.tests.vcxproj.filters +++ b/src/til/ut_til/til.unit.tests.vcxproj.filters @@ -12,9 +12,11 @@ + +