[PATCH] [libcxx] Add <experimental/any> v2.
    Marshall Clow 
    mclow.lists at gmail.com
       
    Mon Mar 16 13:01:17 PDT 2015
    
    
  
Reviewed section any.cons
================
Comment at: test/std/experimental/any/any.class/any.cons/copy.pass.cpp:34
@@ +33,3 @@
+    {
+        small const s(1);
+        any const a(s);
----------------
I guess I'm OK with using `large` and `small` here (instead of `int` and `complex<long double>`, say), but I think that would be better.
If you're going to explicitly check small object optimizations, that should not be in the "test/std/" hierarchy.
================
Comment at: test/std/experimental/any/any.class/any.cons/copy_throws.pass.cpp:24
@@ +23,3 @@
+{
+    using std::experimental::any;
+    {
----------------
Again; `large` and `small`.
================
Comment at: test/std/experimental/any/any.class/any.cons/default.pass.cpp:35
@@ +34,3 @@
+        assert(a.type() == typeid(void));
+        assert(globalMemCounter.checkOutstandingNewEq(0));
+    }
----------------
Do you want to assert this before the `any` is created as well?
================
Comment at: test/std/experimental/any/any.class/any.cons/move.pass.cpp:42
@@ +41,3 @@
+    // small object
+    {
+        small s(1);
----------------
same comment as for copy-constructor for `large` vs. `small`
================
Comment at: test/std/experimental/any/any.class/any.cons/move_throws.pass.cpp:39
@@ +38,2 @@
+    assert(throws_on_move::count == 0);
+}
----------------
Looks good.
================
Comment at: test/std/experimental/any/any.class/any.cons/non_copyable_value.fail.cpp:26
@@ +25,3 @@
+    non_copyable() {}
+    non_copyable(non_copyable &&) {}
+};
----------------
Shouldn't this really be `non_copyable & non_copyable(non_copyable &&) { return *this; }`  ??  
and similar for the declaration of the copy constructor.
I find it hard to write good "compile failing" tests. Frequently they fail for the wrong reasons.
================
Comment at: test/std/experimental/any/any.class/any.cons/value.pass.cpp:55
@@ +54,3 @@
+        small s(1);
+        any const a(static_cast<small &&>(s));
+        assert(!a.empty());
----------------
Not `std::move(s)` ?
================
Comment at: test/std/experimental/any/any.class/any.cons/value.pass.cpp:98
@@ +97,3 @@
+        large l(1);
+        any const a(static_cast<large &&>(l));
+        assert(!a.empty());
----------------
See line #55
================
Comment at: test/std/experimental/any/any.class/any.cons/value_copy_throws.pass.cpp:53
@@ +52,2 @@
+    assert(large_throws_on_copy::count == 0);
+}
----------------
Looks good.
================
Comment at: test/std/experimental/any/any.class/any.cons/value_move_throws.pass.cpp:26
@@ +25,3 @@
+    {
+        throws_on_move v;
+        assert(throws_on_move::count == 1);
----------------
You have a `small_throws_on_copy` and a `large_throws_on_copy`; but only a single `throws_on_move`.
Why the difference?
http://reviews.llvm.org/D6762
EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/
    
    
More information about the cfe-commits
mailing list