<div dir="rtl"><div dir="ltr">Thanks!<br><br></div><div dir="ltr">Would it make sense to have erase(Key) in addition to erase(Iterator) ?</div><div dir="ltr"><br></div><div dir="ltr">Yaron</div><div dir="ltr"><br></div></div>
<div class="gmail_extra"><br><br><div class="gmail_quote"><div dir="ltr">2014-07-15 21:32 GMT+03:00 Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span>:</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: dexonsmith<br>
Date: Tue Jul 15 13:32:30 2014<br>
New Revision: 213084<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=213084&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=213084&view=rev</a><br>
Log:<br>
ADT: Fix MapVector::erase()<br>
<br>
Actually update the changed indexes in the map portion of `MapVector`<br>
when erasing from the middle. Add a unit test that checks for this.<br>
<br>
Note that `MapVector::erase()` is a linear time operation (it was and<br>
still is). I'll commit a new method in a moment called<br>
`MapVector::remove_if()` that deletes multiple entries in linear time,<br>
which should be slightly less painful.<br>
<br>
Modified:<br>
llvm/trunk/docs/ProgrammersManual.rst<br>
llvm/trunk/include/llvm/ADT/MapVector.h<br>
llvm/trunk/unittests/ADT/MapVectorTest.cpp<br>
<br>
Modified: llvm/trunk/docs/ProgrammersManual.rst<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/ProgrammersManual.rst?rev=213084&r1=213083&r2=213084&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/ProgrammersManual.rst?rev=213084&r1=213083&r2=213084&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/docs/ProgrammersManual.rst (original)<br>
+++ llvm/trunk/docs/ProgrammersManual.rst Tue Jul 15 13:32:30 2014<br>
@@ -1442,7 +1442,7 @@ iteration over maps of pointers.<br>
<br>
It is implemented by mapping from key to an index in a vector of key,value<br>
pairs. This provides fast lookup and iteration, but has two main drawbacks: The<br>
-key is stored twice and it doesn't support removing elements.<br>
+key is stored twice and removing elements takes linear time.<br>
<br>
.. _dss_inteqclasses:<br>
<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/MapVector.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/MapVector.h?rev=213084&r1=213083&r2=213084&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/MapVector.h?rev=213084&r1=213083&r2=213084&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/MapVector.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/MapVector.h Tue Jul 15 13:32:30 2014<br>
@@ -125,12 +125,26 @@ public:<br>
}<br>
<br>
/// \brief Remove the element given by Iterator.<br>
+ ///<br>
/// Returns an iterator to the element following the one which was removed,<br>
/// which may be end().<br>
+ ///<br>
+ /// \note This is a deceivingly expensive operation (linear time). It's<br>
+ /// usually better to use \a remove_if() if possible.<br>
typename VectorType::iterator erase(typename VectorType::iterator Iterator) {<br>
- typename MapType::iterator MapIterator = Map.find(Iterator->first);<br>
- Map.erase(MapIterator);<br>
- return Vector.erase(Iterator);<br>
+ Map.erase(Iterator->first);<br>
+ auto Next = Vector.erase(Iterator);<br>
+ if (Next == Vector.end())<br>
+ return Next;<br>
+<br>
+ // Update indices in the map.<br>
+ size_t Index = Next - Vector.begin();<br>
+ for (auto &I : Map) {<br>
+ assert(I.second != Index && "Index was already erased!");<br>
+ if (I.second > Index)<br>
+ --I.second;<br>
+ }<br>
+ return Next;<br>
}<br>
};<br>
<br>
<br>
Modified: llvm/trunk/unittests/ADT/MapVectorTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/MapVectorTest.cpp?rev=213084&r1=213083&r2=213084&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/MapVectorTest.cpp?rev=213084&r1=213083&r2=213084&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ADT/MapVectorTest.cpp (original)<br>
+++ llvm/trunk/unittests/ADT/MapVectorTest.cpp Tue Jul 15 13:32:30 2014<br>
@@ -53,3 +53,18 @@ TEST(MapVectorTest, insert_pop) {<br>
EXPECT_EQ(MV[1], 2);<br>
EXPECT_EQ(MV[4], 7);<br>
}<br>
+<br>
+TEST(MapVectorTest, erase) {<br>
+ MapVector<int, int> MV;<br>
+<br>
+ MV.insert(std::make_pair(1, 2));<br>
+ MV.insert(std::make_pair(3, 4));<br>
+ MV.insert(std::make_pair(5, 6));<br>
+ ASSERT_EQ(MV.size(), 3u);<br>
+<br>
+ MV.erase(MV.find(1));<br>
+ ASSERT_EQ(MV.size(), 2u);<br>
+ ASSERT_EQ(MV.find(1), MV.end());<br>
+ ASSERT_EQ(MV[3], 4);<br>
+ ASSERT_EQ(MV[5], 6);<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>