<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 21 March 2017 at 14:23, Zachary Turner via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: zturner<br>
Date: Tue Mar 21 16:23:57 2017<br>
New Revision: 298440<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=298440&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=298440&view=rev</a><br>
Log:<br>
Revert "Improve StringMap iterator support."<br>
<br>
This is causing crashes in clang, so reverting until the problem<br>
is figured out.<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ADT/<wbr>STLExtras.h<br>
llvm/trunk/include/llvm/ADT/<wbr>StringMap.h<br>
llvm/trunk/unittests/ADT/<wbr>StringMapTest.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/<wbr>STLExtras.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/STLExtras.h?rev=298440&r1=298439&r2=298440&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/ADT/STLExtras.h?rev=<wbr>298440&r1=298439&r2=298440&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/include/llvm/ADT/<wbr>STLExtras.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/<wbr>STLExtras.h Tue Mar 21 16:23:57 2017<br>
@@ -893,8 +893,7 @@ auto partition(R &&Range, UnaryPredicate<br>
/// SmallVector with elements of the vector. This is useful, for example,<br>
/// when you want to iterate a range and then sort the results.<br>
template <unsigned Size, typename R><br>
-SmallVector<typename std::remove_const<detail::<wbr>ValueOfRange<R>>::type, Size><br>
-to_vector(R &&Range) {<br>
+SmallVector<detail::<wbr>ValueOfRange<R>, Size> to_vector(R &&Range) {<br>
return {std::begin(Range), std::end(Range)};<br>
}<br>
<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/<wbr>StringMap.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/StringMap.h?rev=298440&r1=298439&r2=298440&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/ADT/StringMap.h?rev=<wbr>298440&r1=298439&r2=298440&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/include/llvm/ADT/<wbr>StringMap.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/<wbr>StringMap.h Tue Mar 21 16:23:57 2017<br>
@@ -15,13 +15,13 @@<br>
#define LLVM_ADT_STRINGMAP_H<br>
<br>
#include "llvm/ADT/StringRef.h"<br>
-#include "llvm/ADT/iterator.h"<br>
#include "llvm/Support/Allocator.h"<br>
#include "llvm/Support/<wbr>PointerLikeTypeTraits.h"<br>
#include <cassert><br>
#include <cstdint><br>
#include <cstdlib><br>
#include <cstring><br>
+#include <utility><br>
#include <initializer_list><br>
#include <new><br>
#include <utility><br>
@@ -32,7 +32,6 @@ namespace llvm {<br>
class StringMapConstIterator;<br>
template<typename ValueT><br>
class StringMapIterator;<br>
- template <typename ValueT> class StringMapKeyIterator;<br>
template<typename ValueTy><br>
class StringMapEntry;<br>
<br>
@@ -313,11 +312,6 @@ public:<br>
return const_iterator(TheTable+<wbr>NumBuckets, true);<br>
}<br>
<br>
- llvm::iterator_range<<wbr>StringMapKeyIterator<ValueTy>> keys() const {<br>
- return make_range(<wbr>StringMapKeyIterator<ValueTy>(<wbr>begin()),<br>
- StringMapKeyIterator<ValueTy>(<wbr>end()));<br>
- }<br>
-<br>
iterator find(StringRef Key) {<br>
int Bucket = FindKey(Key);<br>
if (Bucket == -1) return end();<br>
@@ -450,39 +444,42 @@ public:<br>
}<br>
};<br>
<br>
-template <typename DerivedTy, typename ValueTy><br>
-class StringMapIterBase<br>
- : public iterator_facade_base<<wbr>DerivedTy, std::forward_iterator_tag,<br>
- ValueTy> {<br>
+template <typename ValueTy> class StringMapConstIterator {<br>
protected:<br>
StringMapEntryBase **Ptr = nullptr;<br>
<br>
public:<br>
- StringMapIterBase() = default;<br>
+ typedef StringMapEntry<ValueTy> value_type;<br>
<br>
- explicit StringMapIterBase(<wbr>StringMapEntryBase **Bucket,<br>
- bool NoAdvance = false)<br>
- : Ptr(Bucket) {<br>
+ StringMapConstIterator() = default;<br>
+<br>
+ explicit StringMapConstIterator(<wbr>StringMapEntryBase **Bucket,<br>
+ bool NoAdvance = false)<br>
+ : Ptr(Bucket) {<br>
if (!NoAdvance) AdvancePastEmptyBuckets();<br>
}<br>
<br>
- DerivedTy &operator=(const DerivedTy &Other) {<br>
- Ptr = Other.Ptr;<br>
- return static_cast<DerivedTy &>(*this);<br>
+ const value_type &operator*() const {<br>
+ return *static_cast<StringMapEntry<<wbr>ValueTy>*>(*Ptr);<br>
+ }<br>
+ const value_type *operator->() const {<br>
+ return static_cast<StringMapEntry<<wbr>ValueTy>*>(*Ptr);<br>
}<br>
<br>
- bool operator==(const DerivedTy &RHS) const { return Ptr == RHS.Ptr; }<br>
+ bool operator==(const StringMapConstIterator &RHS) const {<br>
+ return Ptr == RHS.Ptr;<br>
+ }<br>
+ bool operator!=(const StringMapConstIterator &RHS) const {<br>
+ return Ptr != RHS.Ptr;<br>
+ }<br>
<br>
- DerivedTy &operator++() { // Preincrement<br>
+ inline StringMapConstIterator& operator++() { // Preincrement<br>
++Ptr;<br>
AdvancePastEmptyBuckets();<br>
- return static_cast<DerivedTy &>(*this);<br>
+ return *this;<br>
}<br>
-<br>
- DerivedTy operator++(int) { // Post-increment<br>
- DerivedTy Tmp(Ptr);<br>
- ++*this;<br>
- return Tmp;<br>
+ StringMapConstIterator operator++(int) { // Postincrement<br>
+ StringMapConstIterator tmp = *this; ++*this; return tmp;<br>
}<br>
<br>
private:<br>
@@ -492,67 +489,22 @@ private:<br>
}<br>
};<br>
<br>
-template <typename ValueTy><br>
-class StringMapConstIterator<br>
- : public StringMapIterBase<<wbr>StringMapConstIterator<<wbr>ValueTy>,<br>
- const StringMapEntry<ValueTy>> {<br>
- using base = StringMapIterBase<<wbr>StringMapConstIterator<<wbr>ValueTy>,<br>
- const StringMapEntry<ValueTy>>;<br>
-<br>
-public:<br>
- StringMapConstIterator() = default;<br>
- explicit StringMapConstIterator(<wbr>StringMapEntryBase **Bucket,<br>
- bool NoAdvance = false)<br>
- : base(Bucket, NoAdvance) {}<br>
-<br>
- const StringMapEntry<ValueTy> &operator*() const {<br>
- return *static_cast<const StringMapEntry<ValueTy> *>(*this->Ptr);<br>
- }<br>
-};<br>
-<br>
-template <typename ValueTy><br>
-class StringMapIterator : public StringMapIterBase<<wbr>StringMapIterator<ValueTy>,<br>
- StringMapEntry<ValueTy>> {<br>
- using base =<br>
- StringMapIterBase<<wbr>StringMapIterator<ValueTy>, StringMapEntry<ValueTy>>;<br>
-<br>
+template<typename ValueTy><br>
+class StringMapIterator : public StringMapConstIterator<<wbr>ValueTy> {<br>
public:<br>
StringMapIterator() = default;<br>
+<br>
explicit StringMapIterator(<wbr>StringMapEntryBase **Bucket,<br>
bool NoAdvance = false)<br>
- : base(Bucket, NoAdvance) {}<br>
-<br>
- StringMapEntry<ValueTy> &operator*() const {<br>
- return *static_cast<StringMapEntry<<wbr>ValueTy> *>(*this->Ptr);<br>
+ : StringMapConstIterator<<wbr>ValueTy>(Bucket, NoAdvance) {<br>
}<br>
<br>
- operator StringMapConstIterator<<wbr>ValueTy>() const {<br>
- return StringMapConstIterator<<wbr>ValueTy>(this->Ptr, false);<br></blockquote><div><br></div><div>The problem is that this conversion tries to "advance" the iterator, which doesn't work if it's the end iterator. You should probably pass 'true' here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ StringMapEntry<ValueTy> &operator*() const {<br>
+ return *static_cast<StringMapEntry<<wbr>ValueTy>*>(*this->Ptr);<br>
}<br>
-};<br>
-<br>
-template <typename ValueTy><br>
-class StringMapKeyIterator<br>
- : public iterator_adaptor_base<<wbr>StringMapKeyIterator<ValueTy>,<br>
- StringMapConstIterator<<wbr>ValueTy>,<br>
- std::forward_iterator_tag, StringRef> {<br>
- using base = iterator_adaptor_base<<wbr>StringMapKeyIterator<ValueTy>,<br>
- StringMapConstIterator<<wbr>ValueTy>,<br>
- std::forward_iterator_tag, StringRef>;<br>
-<br>
-public:<br>
- StringMapKeyIterator() = default;<br>
-<br>
- explicit StringMapKeyIterator(<wbr>StringMapConstIterator<<wbr>ValueTy> Iter)<br>
- : base(std::move(Iter)) {}<br>
-<br>
- StringRef &operator*() {<br>
- Key = this->wrapped()->getKey();<br>
- return Key;<br>
+ StringMapEntry<ValueTy> *operator->() const {<br>
+ return static_cast<StringMapEntry<<wbr>ValueTy>*>(*this->Ptr);<br>
}<br>
-<br>
-private:<br>
- StringRef Key;<br>
};<br>
<br>
} // end namespace llvm<br>
<br>
Modified: llvm/trunk/unittests/ADT/<wbr>StringMapTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/StringMapTest.cpp?rev=298440&r1=298439&r2=298440&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/unittests/<wbr>ADT/StringMapTest.cpp?rev=<wbr>298440&r1=298439&r2=298440&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/unittests/ADT/<wbr>StringMapTest.cpp (original)<br>
+++ llvm/trunk/unittests/ADT/<wbr>StringMapTest.cpp Tue Mar 21 16:23:57 2017<br>
@@ -8,7 +8,6 @@<br>
//===-------------------------<wbr>------------------------------<wbr>---------------===//<br>
<br>
#include "llvm/ADT/StringMap.h"<br>
-#include "llvm/ADT/StringSet.h"<br>
#include "llvm/ADT/Twine.h"<br>
#include "llvm/Support/DataTypes.h"<br>
#include "gtest/gtest.h"<br>
@@ -270,34 +269,6 @@ TEST_F(StringMapTest, InsertRehashingPai<br>
EXPECT_EQ(42u, It->second);<br>
}<br>
<br>
-TEST_F(StringMapTest, IterMapKeys) {<br>
- StringMap<int> Map;<br>
- Map["A"] = 1;<br>
- Map["B"] = 2;<br>
- Map["C"] = 3;<br>
- Map["D"] = 3;<br>
-<br>
- auto Keys = to_vector<4>(Map.keys());<br>
- std::sort(Keys.begin(), Keys.end());<br>
-<br>
- SmallVector<StringRef, 4> Expected = {"A", "B", "C", "D"};<br>
- EXPECT_EQ(Expected, Keys);<br>
-}<br>
-<br>
-TEST_F(StringMapTest, IterSetKeys) {<br>
- StringSet<> Set;<br>
- Set.insert("A");<br>
- Set.insert("B");<br>
- Set.insert("C");<br>
- Set.insert("D");<br>
-<br>
- auto Keys = to_vector<4>(Set.keys());<br>
- std::sort(Keys.begin(), Keys.end());<br>
-<br>
- SmallVector<StringRef, 4> Expected = {"A", "B", "C", "D"};<br>
- EXPECT_EQ(Expected, Keys);<br>
-}<br>
-<br>
// Create a non-default constructable value<br>
struct StringMapTestStruct {<br>
StringMapTestStruct(int i) : i(i) {}<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div>