Allow user-defined structs as keys to the interval tree - #31
Open
PankajBhojwani wants to merge 4 commits into
Open
Allow user-defined structs as keys to the interval tree#31PankajBhojwani wants to merge 4 commits into
PankajBhojwani wants to merge 4 commits into
Conversation
DHowett
reviewed
Oct 1, 2020
DHowett
reviewed
Oct 5, 2020
steffen-heil-secforge
added a commit
to secforge/terminal
that referenced
this pull request
Jul 30, 2026
is_valid() seeded its bounds accumulators with std::numeric_limits<Scalar>
::max()/min(). We instantiate the tree with Scalar = til::point, which has
no std::numeric_limits specialization, so the primary template returned a
default-constructed til::point{0,0} for *both* sentinels. The right-subtree
constraint check then evaluated {0,0} <= center, which holds for essentially
any real tree, so is_valid() returned false and assert(is_valid().first)
fired.
The tree only splits into subtrees once it holds 64 intervals, its default
minimum bucket size, so the threshold was exactly 64: Terminal::_getPatterns
builds the pattern tree from autodetected URLs over roughly three viewport-
heights of buffer, making 64 matches easy to reach. Any Debug build showing
a screenful of links aborted. Release builds were unaffected, and the tree
itself was structurally sound -- only the checker was wrong.
Track emptiness explicitly so no sentinel value is needed. Also accumulate
the maximum stop with std::max rather than std::min; upstream's std::min
pinned that accumulator at numeric_limits<Scalar>::min() forever, silently
reducing both center checks to no-ops, so a numeric_limits<til::point>
specialization alone would have left the check dead.
This is the second local deviation from ekg/intervaltree, alongside the
struct-key support carried since microsoft#7691 (upstream PR ekg/intervaltree#31,
still unmerged). Both are now recorded in MAINTAINER_README.md, together
with a warning that re-importing the file wholesale breaks the build.
Closes microsoft#20486
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pr3nsYBYoJ66jgRidRHyJF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hello! Thank you for this implementation - it is extremely useful!
I'd like to propose a change that would allow custom structs to be used as keys to the tree as long as those structs have the
<,<=,>,>=,==operators defined.The only code change this involves is instead of checking for equality with 0, we check for equality with the default constructor. For integral values, this does not change the behaviour.
UPDATE:
This also now adds a default constructor and the equality/inequality operators just for ease of use