<div dir="ltr">Yep - this is essentially impossible to test for using raw pointers. The test will be unreliable in either forward or reverse direction. Please commit a change to delete the tests that test containers of pointers. (no need to wait for code review, etc)<br><br>In terms of finding a more reliable/good way to test this functionality... - perhaps it'd be possible to make a custom type that's pointer-like, but not a pointer & has known comparisons. (eg: struct foo { int i; }; with an IsPointerLike specialization, comparison/hashing using the member int so it's reliably ordered)</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Aug 24, 2017 at 5:36 PM Grang, Mandeep Singh <<a href="mailto:mgrang@codeaurora.org">mgrang@codeaurora.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div text="#000000" bgcolor="#FFFFFF">
<p><font size="+1">Compare these two:</font></p>
<p><font size="+1"><a class="m_-4266230998048083906moz-txt-link-freetext" href="http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/9064/steps/ninja%20check%201/logs/FAIL%3A%20LLVM-Unit%3A%3AReverseIterationTest.DenseMapTest1" target="_blank">http://lab.llvm.org:8011/builders/clang-ppc64le-linux/builds/9064/steps/ninja%20check%201/logs/FAIL%3A%20LLVM-Unit%3A%3AReverseIterationTest.DenseMapTest1</a></font></p>
<p><font size="+1"><a class="m_-4266230998048083906moz-txt-link-freetext" href="http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/4861/steps/test-stage1-compiler/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/clang-with-thin-lto-ubuntu/builds/4861/steps/test-stage1-compiler/logs/stdio</a></font></p>
<p><font size="+1">In one case it expects "3" while in the other it
expects "4".<br>
</font></p></div><div text="#000000" bgcolor="#FFFFFF">
<br>
<div class="m_-4266230998048083906moz-cite-prefix">On 8/24/2017 5:31 PM, Grang, Mandeep
Singh via llvm-commits wrote:<br>
</div>
<blockquote type="cite">
<font size="+1">I think maintaining an *expected* order for
pointer-like keys may not be the correct way of doing this. We
would need a way to make the expected order independent of the
hashing function.<br>
<br>
</font>
<div class="m_-4266230998048083906moz-cite-prefix">On 8/24/2017 5:26 PM, Grang, Mandeep
Singh via llvm-commits wrote:<br>
</div>
<blockquote type="cite">
<p><font size="+1">The problem is not the reversal of the
expected order. The problem is that the *expected* order for
pointer-like keys may differ based on how they are hashed.</font></p>
<p><font size="+1">The hashing order may be
machine/platform/environment dependent.</font></p>
<p><font size="+1">--Mandeep</font><br>
</p>
<br>
<div class="m_-4266230998048083906moz-cite-prefix">On 8/24/2017 5:17 PM, David Blaikie
wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Doesn't the test only reverse the expected
order if reverse iteration is enabled? So why is this
failing on a forward iteration build?<br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Thu, Aug 24, 2017 at 5:13 PM Mandeep
Singh Grang via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">mgrang
created this revision.<br>
<br>
This test causes failures in forward iteration builds.
This is because we have hard-coded<br>
the expected order of iteration of supported containers.<br>
<br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D37128" rel="noreferrer" target="_blank">https://reviews.llvm.org/D37128</a><br>
<br>
Files:<br>
unittests/Support/ReverseIterationTest.cpp<br>
<br>
<br>
Index: unittests/Support/ReverseIterationTest.cpp<br>
===================================================================<br>
--- unittests/Support/ReverseIterationTest.cpp<br>
+++ unittests/Support/ReverseIterationTest.cpp<br>
@@ -16,6 +16,8 @@<br>
#include "llvm/Support/ReverseIteration.h"<br>
#include "gtest/gtest.h"<br>
<br>
+#if LLVM_ENABLE_REVERSE_ITERATION<br>
+<br>
using namespace llvm;<br>
<br>
TEST(ReverseIterationTest, DenseMapTest1) {<br>
@@ -109,3 +111,5 @@<br>
for (auto iter = Set.begin(), end = Set.end(); iter
!= end; iter++, ++i)<br>
ASSERT_EQ(*iter, IterPtrs[i]);<br>
}<br>
+<br>
+#endif<br>
<br>
<br>
</blockquote>
</div>
</div>
</blockquote>
<br>
<br>
<fieldset class="m_-4266230998048083906mimeAttachmentHeader"></fieldset>
<br>
<pre>_______________________________________________
llvm-commits mailing list
<a class="m_-4266230998048083906moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>
<a class="m_-4266230998048083906moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
<br>
<fieldset class="m_-4266230998048083906mimeAttachmentHeader"></fieldset>
<br>
<pre>_______________________________________________
llvm-commits mailing list
<a class="m_-4266230998048083906moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>
<a class="m_-4266230998048083906moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
<br>
</div></blockquote></div>