Skip to content

Conversation

philnik777
Copy link
Contributor

We've modified the algorithm of __tree::find in #152370, which can change the return value. Since we're always returned the lower bound before some users started relying on it. This patch adds a release note so users are aware that this might break their code.

@philnik777 philnik777 requested a review from a team as a code owner August 25, 2025 14:49
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 25, 2025
@llvmbot
Copy link
Member

llvmbot commented Aug 25, 2025

@llvm/pr-subscribers-libcxx

Author: Nikolas Klauser (philnik777)

Changes

We've modified the algorithm of __tree::find in #152370, which can change the return value. Since we're always returned the lower bound before some users started relying on it. This patch adds a release note so users are aware that this might break their code.


Full diff: https://github.com/llvm/llvm-project/pull/155252.diff

1 Files Affected:

  • (modified) libcxx/docs/ReleaseNotes/22.rst (+4)
diff --git a/libcxx/docs/ReleaseNotes/22.rst b/libcxx/docs/ReleaseNotes/22.rst
index a26c5476d421b..6be269cfbbc84 100644
--- a/libcxx/docs/ReleaseNotes/22.rst
+++ b/libcxx/docs/ReleaseNotes/22.rst
@@ -61,6 +61,10 @@ Deprecations and Removals
 Potentially breaking changes
 ----------------------------
 
+- The algorithm for ``multi{map,set}::find`` has been modified, which doesn't guarantee that the first element is
+  returned. If code relies on the first element being returned from ``find`` it may be broken. To fix it use
+  ``lower_bound`` or ``equal_range`` instead.
+
 Announcements About Future Releases
 -----------------------------------
 

@nico
Copy link
Contributor

nico commented Aug 25, 2025

Thanks!

As far as I can tell, libstdc++ also returns the first element from find():

https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/stl_tree.h#L3166
https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/bits/stl_tree.h#L1936

As does MSVC's STL:

https://github.com/microsoft/STL/blob/main/stl/inc/xtree#L1395

So maybe say something like "libc++ is the first mainstream C++ standard library not returning the first element" to call this out more? I imagine it's going to be fairly ingrained.

For what it's worth, in Chromium all calls of find() I've found so far (so far just 2) wanted the first element. So maybe it might even make sense to change the spec to require this instead of changing implementation behavior 😅 (update: I now also found 2 instances where find() returning any element is fine.)

(…for Chromium, it looks like it'll be feasible to just audit all callers, so I personally don't care all that much. But I imagine people with bigger code bases might have a harder time.)

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with rewording suggestion

Co-authored-by: Louis Dionne <ldionne.2@gmail.com>
@philnik777 philnik777 merged commit 62da805 into llvm:main Aug 26, 2025
12 of 16 checks passed
@philnik777 philnik777 deleted the release_note_tree_find branch August 26, 2025 06:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants