[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