<div dir="ltr">I was about to revert this too but Galina beat me. The problem is that in MSVC's STL, set<T>::const_iterator is not trivially copy constructible.</div><br><div class="gmail_quote"><div dir="ltr">On Wed, Jul 11, 2018 at 11:06 AM Galina Kistanova via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</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">Hello Florian,<br><br>This commit broke build step on one of our builders:<br><a href="http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/10903/steps/build-unified-tree/logs/stdio" target="_blank">http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/10903/steps/build-unified-tree/logs/stdio</a><br><br>. . .<br>FAILED: unittests/ADT/CMakeFiles/ADTTests.dir/SmallSetTest.cpp.obj <br>C:\PROGRA~2\MICROS~1.0\VC\bin\amd64\cl.exe /nologo /TP -DEXPENSIVE_CHECKS -DGTEST_HAS_RTTI=0 -DGTEST_HAS_TR1_TUPLE=0 -DGTEST_LANG_CXX11=1 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GLIBCXX_DEBUG -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Iunittests\ADT -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\unittests\ADT -Iinclude -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\include -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\utils\unittest\googletest\include -IC:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\utils\unittest\googlemock\include /DWIN32 /D_WINDOWS /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /MDd /Zi /Ob0 /Od /RTC1 /EHs-c- /GR- /showIncludes /Founittests\ADT\CMakeFiles\ADTTests.dir\SmallSetTest.cpp.obj /Fdunittests\ADT\CMakeFiles\ADTTests.dir\ /FS -c C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\unittests\ADT\SmallSetTest.cpp<br>C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\unittests\ADT\SmallSetTest.cpp(80): error C2280: 'llvm::SmallSetIterator<T,4,C>::SmallSetIterator(const llvm::SmallSetIterator<T,4,C> &)': attempting to reference a deleted function<br> with<br> [<br> T=int,<br> C=std::less<int><br> ]<br>C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\llvm\include\llvm/ADT/SmallSet.h(80): note: compiler has generated 'llvm::SmallSetIterator<T,4,C>::SmallSetIterator' here<br> with<br> [<br> T=int,<br> C=std::less<int><br> ]<br><br><br>Please have a look?<br>The builder was already red and did not send notifications.<br><br>Thanks</div><div dir="ltr"><br><br>Galina<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Jul 11, 2018 at 6:39 AM, Florian Hahn 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: fhahn<br>
Date: Wed Jul 11 06:39:59 2018<br>
New Revision: 336805<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=336805&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=336805&view=rev</a><br>
Log:<br>
Recommit r334887: [SmallSet] Add SmallSetIterator.<br>
<br>
This version now uses the subset of is_trivially_XXX provided by<br>
GCC 4.8 and llvm/Support/type_traits.h<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/ADT/SmallSet.h<br>
llvm/trunk/unittests/ADT/SmallSetTest.cpp<br>
<br>
Modified: llvm/trunk/include/llvm/ADT/SmallSet.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallSet.h?rev=336805&r1=336804&r2=336805&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ADT/SmallSet.h?rev=336805&r1=336804&r2=336805&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/include/llvm/ADT/SmallSet.h (original)<br>
+++ llvm/trunk/include/llvm/ADT/SmallSet.h Wed Jul 11 06:39:59 2018<br>
@@ -17,14 +17,68 @@<br>
#include "llvm/ADT/None.h"<br>
#include "llvm/ADT/SmallPtrSet.h"<br>
#include "llvm/ADT/SmallVector.h"<br>
+#include "llvm/ADT/iterator.h"<br>
#include "llvm/Support/Compiler.h"<br>
+#include "llvm/Support/type_traits.h"<br>
#include <cstddef><br>
#include <functional><br>
#include <set><br>
+#include <type_traits><br>
#include <utility><br>
<br>
namespace llvm {<br>
<br>
+/// SmallSetIterator - This class implements a const_iterator for SmallSet by<br>
+/// delegating to the underlying SmallVector or Set iterators.<br>
+template <typename T, unsigned N, typename C><br>
+class SmallSetIterator<br>
+ : public iterator_facade_base<SmallSetIterator<T, N, C>,<br>
+ std::forward_iterator_tag, T> {<br>
+private:<br>
+ using SetIterTy = typename std::set<T, C>::const_iterator;<br>
+ using VecIterTy = typename SmallVector<T, N>::const_iterator;<br>
+ using SelfTy = SmallSetIterator<T, N, C>;<br>
+<br>
+ /// Iterators to the parts of the SmallSet containing the data. They are set<br>
+ /// depending on isSmall.<br>
+ union {<br>
+ SetIterTy SetIter;<br>
+ VecIterTy VecIter;<br>
+ };<br>
+<br>
+ bool isSmall;<br>
+<br>
+public:<br>
+ SmallSetIterator(SetIterTy SetIter) : SetIter(SetIter), isSmall(false) {<br>
+ // Use static_assert here, as the SmallSetIterator type is incomplete in the<br>
+ // class scope.<br>
+ static_assert(std::is_trivially_destructible<SelfTy>() &&<br>
+ llvm::is_trivially_copy_constructible<SelfTy>() &&<br>
+ llvm::is_trivially_move_constructible<SelfTy>(),<br>
+ "SelfTy needs to by trivially constructible and destructible");<br>
+ }<br>
+<br>
+ SmallSetIterator(VecIterTy VecIter) : VecIter(VecIter), isSmall(true) {}<br>
+<br>
+ bool operator==(const SmallSetIterator &RHS) const {<br>
+ if (isSmall != RHS.isSmall)<br>
+ return false;<br>
+ if (isSmall)<br>
+ return VecIter == RHS.VecIter;<br>
+ return SetIter == RHS.SetIter;<br>
+ }<br>
+<br>
+ SmallSetIterator &operator++() { // Preincrement<br>
+ if (isSmall)<br>
+ VecIter++;<br>
+ else<br>
+ SetIter++;<br>
+ return *this;<br>
+ }<br>
+<br>
+ const T &operator*() const { return isSmall ? *VecIter : *SetIter; }<br>
+};<br>
+<br>
/// SmallSet - This maintains a set of unique values, optimizing for the case<br>
/// when the set is small (less than N). In this case, the set can be<br>
/// maintained with no mallocs. If the set gets large, we expand to using an<br>
@@ -50,6 +104,7 @@ class SmallSet {<br>
<br>
public:<br>
using size_type = size_t;<br>
+ using const_iterator = SmallSetIterator<T, N, C>;<br>
<br>
SmallSet() = default;<br>
<br>
@@ -121,6 +176,18 @@ public:<br>
Set.clear();<br>
}<br>
<br>
+ const_iterator begin() const {<br>
+ if (isSmall())<br>
+ return {Vector.begin()};<br>
+ return {Set.begin()};<br>
+ }<br>
+<br>
+ const_iterator end() const {<br>
+ if (isSmall())<br>
+ return {Vector.end()};<br>
+ return {Set.end()};<br>
+ }<br>
+<br>
private:<br>
bool isSmall() const { return Set.empty(); }<br>
<br>
<br>
Modified: llvm/trunk/unittests/ADT/SmallSetTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SmallSetTest.cpp?rev=336805&r1=336804&r2=336805&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/ADT/SmallSetTest.cpp?rev=336805&r1=336804&r2=336805&view=diff</a><br>
==============================================================================<br>
--- llvm/trunk/unittests/ADT/SmallSetTest.cpp (original)<br>
+++ llvm/trunk/unittests/ADT/SmallSetTest.cpp Wed Jul 11 06:39:59 2018<br>
@@ -13,6 +13,7 @@<br>
<br>
#include "llvm/ADT/SmallSet.h"<br>
#include "gtest/gtest.h"<br>
+#include <string><br>
<br>
using namespace llvm;<br>
<br>
@@ -68,3 +69,57 @@ TEST(SmallSetTest, Erase) {<br>
<br>
EXPECT_EQ(0u, s1.count(8));<br>
}<br>
+<br>
+TEST(SmallSetTest, IteratorInt) {<br>
+ SmallSet<int, 4> s1;<br>
+<br>
+ // Test the 'small' case.<br>
+ for (int i = 0; i < 3; i++)<br>
+ s1.insert(i);<br>
+<br>
+ std::vector<int> V(s1.begin(), s1.end());<br>
+ // Make sure the elements are in the expected order.<br>
+ std::sort(V.begin(), V.end());<br>
+ for (int i = 0; i < 3; i++)<br>
+ EXPECT_EQ(i, V[i]);<br>
+<br>
+ // Test the 'big' case by adding a few more elements to switch to std::set<br>
+ // internally.<br>
+ for (int i = 3; i < 6; i++)<br>
+ s1.insert(i);<br>
+<br>
+ V.assign(s1.begin(), s1.end());<br>
+ // Make sure the elements are in the expected order.<br>
+ std::sort(V.begin(), V.end());<br>
+ for (int i = 0; i < 6; i++)<br>
+ EXPECT_EQ(i, V[i]);<br>
+}<br>
+<br>
+TEST(SmallSetTest, IteratorString) {<br>
+ // Test SmallSetIterator for SmallSet with a type with non-trivial<br>
+ // ctors/dtors.<br>
+ SmallSet<std::string, 2> s1;<br>
+<br>
+ s1.insert("str 1");<br>
+ s1.insert("str 2");<br>
+ s1.insert("str 1");<br>
+<br>
+ std::vector<std::string> V(s1.begin(), s1.end());<br>
+ std::sort(V.begin(), V.end());<br>
+ EXPECT_EQ(2u, s1.size());<br>
+ EXPECT_EQ("str 1", V[0]);<br>
+ EXPECT_EQ("str 2", V[1]);<br>
+<br>
+ s1.insert("str 4");<br>
+ s1.insert("str 0");<br>
+ s1.insert("str 4");<br>
+<br>
+ V.assign(s1.begin(), s1.end());<br>
+ // Make sure the elements are in the expected order.<br>
+ std::sort(V.begin(), V.end());<br>
+ EXPECT_EQ(4u, s1.size());<br>
+ EXPECT_EQ("str 0", V[0]);<br>
+ EXPECT_EQ("str 1", V[1]);<br>
+ EXPECT_EQ("str 2", V[2]);<br>
+ EXPECT_EQ("str 4", V[3]);<br>
+}<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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/mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">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/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>