[PATCH] D24941: [libc++][tests] Fixing some issues in unordered container tests

Eric Fiselier via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 26 16:10:28 PDT 2016


EricWF added a comment.

Thanks for cleaning up our debug mode. I've added a couple of inline comments which need to be addressed.

If your interested there is much more work to do. For example moving all debug tests from `test/std` to `test/libcxx`, or fixing the debug tests to manually `#define _LIBCPP_DEBUG 1`.


================
Comment at: test/libcxx/containers/unord/unord.map/db_local_iterators_7.pass.cpp:23
@@ -22,3 +22,3 @@
 #include <exception>
 #include <cstdlib>
 
----------------
Tests for debug mode should manually enable it, instead of requiring it already be enabled. If you want to do more cleanup please change this test to `#define _LIBCPP_DEBUG 1` at the start of the file, and then remove the redundant empty main.

================
Comment at: test/libcxx/containers/unord/unord.map/db_local_iterators_7.pass.cpp:31
@@ -30,3 +30,3 @@
     typedef std::unordered_map<int, std::string> C;
-    C c(1);
-    C::local_iterator i = c.begin(0);
+    C c({{1, std::string()}});
+    C::local_iterator i = c.begin(c.bucket(1));
----------------
This uses C++11 features but is required to work in C++03. Please make sure these changes don't break C++03 tests.

Side note: Wouldn't removing the second increment be simpler than changing the size? 

================
Comment at: test/libcxx/containers/unord/unord.map/db_local_iterators_7.pass.cpp:35
@@ -34,3 +34,3 @@
     ++i;
     assert(false);
     }
----------------
All of the code below this is logically dead. Please either (A) remove it, or (B) define `_LIBCPP_ASSERT` so it doesn't exit when invoked. For example:

```
#define _LIBCPP_ASSERT(p, m) ((p) ? (void)0 : throw 0)
int main()
{
    typedef std::unordered_map<int, std::string> C;
    C c(1);
    C::local_iterator i = c.begin(0);
    try {
      ++i;
      assert(false);
    } catch(int) {}
    // More tests can be written below
}
```

================
Comment at: test/std/containers/unord/unord.multiset/db_iterators_7.pass.cpp:41
@@ -40,3 +40,3 @@
     typedef int T;
-    typedef std::unordered_multiset<T, min_allocator<T>> C;
-    C c(1);
+    typedef std::unordered_multiset<T, std::hash<T>, std::equal_to<T>, min_allocator<T>> C;
+    C c({1});
----------------
Nice fix. Thanks!

================
Comment at: test/std/containers/unord/unord.set/insert_hint_const_lvalue.pass.cpp:39
@@ -38,3 +38,3 @@
 
-        r = c.insert(e, P(3.5));
+        r = c.insert(c.end(), P(3.5));
         assert(c.size() == 1);
----------------
These fixes LGTM. Thanks.


Repository:
  rL LLVM

https://reviews.llvm.org/D24941





More information about the llvm-commits mailing list