[PATCH] [libcxx] Add <experimental/any> v2.

Eric Fiselier eric at efcs.ca
Tue Mar 17 08:43:42 PDT 2015


Address @mclow.lists comments.


================
Comment at: test/std/experimental/any/any.class/any.cons/copy.pass.cpp:34
@@ +33,3 @@
+    {
+        small const s(1);
+        any const a(s);
----------------
mclow.lists wrote:
> 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.
All of the tests in the `test/std` hierarchy should pass regardless of whether the implementation provides the small object optimization. I test both cases inside `test/std` because if I didn't the tests here would only provides about ~50% coverage. Any test cases that actually 

There are two reasons I prefer using the names `large` and `small` (even if they alias int and complex<long double>). 

1. If the conditions for the SOO change then it is easy to change either `large` or `small` to match.
2. It's more clear why there is duplication in the tests for the two different types.

The reason I like actually using the `large` and `small` classes is that they provide a little extra support for testing things like construction, copying and moving.





================
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));
+    }
----------------
mclow.lists wrote:
> Do you want to assert this before the `any` is created as well?
Probably. Thanks.

================
Comment at: test/std/experimental/any/any.class/any.cons/non_copyable_value.fail.cpp:26
@@ +25,3 @@
+    non_copyable() {}
+    non_copyable(non_copyable &&) {}
+};
----------------
mclow.lists wrote:
> 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.
I don't think so... Could you explain?

================
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());
----------------
mclow.lists wrote:
> Not `std::move(s)` ?
Fixed.

================
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());
----------------
mclow.lists wrote:
> See line #55
Fixed.

================
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);
----------------
mclow.lists wrote:
> You have a `small_throws_on_copy` and a `large_throws_on_copy`; but only a single `throws_on_move`.
> 
> Why the difference?
Any type that throws on move construction cannot have the SOO applied to it.

http://reviews.llvm.org/D6762

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list