[llvm] [ADT] Teach set_intersect to erase with iterators (PR #99569)

Kazu Hirata via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 18 14:19:37 PDT 2024


https://github.com/kazutakahirata created https://github.com/llvm/llvm-project/pull/99569

Without this patch, we erase an element in S1 by value even though we
have an interator pointing to it.  This patch tries to use erase(iter)
to avoid redundant lookups.

Existing tests cover the new code path.  Somewhat worrying is that the
existing "erase-by-value" code path is no longer covered.  I am not
aware of any container that has erase(value) but not remove_if or
erase(iter).


>From 860efd218ca9c2620cd921f1f932df77becd4358 Mon Sep 17 00:00:00 2001
From: Kazu Hirata <kazu at google.com>
Date: Thu, 18 Jul 2024 02:54:48 -0700
Subject: [PATCH] [ADT] Teach set_intersect to erase with iterators

Without this patch, we erase an element in S1 by value even though we
have an interator pointing to it.  This patch tries to use erase(iter)
to avoid redundant lookups.

Existing tests cover the new code path.  Somewhat worrying is that the
existing "erase-by-value" code path is no longer covered.  I am not
aware of any container that has erase(value) but not remove_if or
erase(iter).
---
 llvm/include/llvm/ADT/SetOperations.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/llvm/include/llvm/ADT/SetOperations.h b/llvm/include/llvm/ADT/SetOperations.h
index 2b1a103565f7d..feb3561aca61f 100644
--- a/llvm/include/llvm/ADT/SetOperations.h
+++ b/llvm/include/llvm/ADT/SetOperations.h
@@ -60,6 +60,13 @@ template <class S1Ty, class S2Ty> void set_intersect(S1Ty &S1, const S2Ty &S2) {
   auto Pred = [&S2](const auto &E) { return !S2.count(E); };
   if constexpr (detail::HasMemberRemoveIf<S1Ty, decltype(Pred)>) {
     S1.remove_if(Pred);
+  } else if constexpr (detail::HasMemberEraseIter<S1Ty>) {
+    typename S1Ty::iterator Next;
+    for (typename S1Ty::iterator I = S1.begin(); I != S1.end(); I = Next) {
+      Next = std::next(I);
+      if (!S2.count(*I))
+        S1.erase(I); // Erase element if not in S2
+    }
   } else {
     for (typename S1Ty::iterator I = S1.begin(); I != S1.end();) {
       const auto &E = *I;



More information about the llvm-commits mailing list