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