CaseyCarter added a comment.
I'll push a revision with the whitespace changes reverted soon.
> .gitignore:59
> keep.lst
> +.vscode/
This is an editor byproduct, I suppose, not "MSVC libraries test harness". Let
me know if anyone cares and I'll add a comment.
> any:553
> + __tmp.__call(_Action::_Move, this);
> + }
> }
Don't blow up on self swap. This is the obvious (naive) fix; it's possible that
someone who has looked at more of the implementation than this single function
could devise a better fix.
> value.pass.cpp:25
> +// Test that any& operator=(ValueType&&) is *never* selected for:
> +// * std::in_place type.
> + {
This behavior is not required by N4606, so I moved this test to the libcxx tree.
> value.pass.cpp:23
> +int main() {
> + // test construction from INSANE copy-but-not-movable types.
> + using Type = deleted_move;
Again, not required behavior (I am, of course, totally unbiased about whether
this behavior is a good idea.)
> any_cast_reference.pass.cpp:34
> +
> +// Test that I can retrieve INSANE copy-but-not-movable type from an any
> +void test_cast_to_value_deleted_move()
ibid.
> move.pass.cpp:43
> assert(LHS::count == 1);
> - assert(RHS::count == 2);
> + assert(RHS::count == 2 + a2.has_value());
>
Portability fix: don't rely on a moved-from `any` being empty. (Occurs more
below)
> move.pass.cpp:93
> + , "any & operator=(any &&) must be noexcept"
> + );
> }
Woops - I forgot to review the diff without -w. Will correct this unintended
whitespace change.
> value.pass.cpp:69
> assert(LHS::count == 0);
> - assert(RHS::count == 1);
> + assert(RHS::count == 1 + rhs.has_value());
>
Again, don't rely on `any`'s moved-from state being empty.
> value.pass.cpp:123
> Move ? lhs = std::move(rhs)
> - : lhs = rhs;
> + : lhs = rhs;
> assert(false);
Whitespace again (will fix).
> value.pass.cpp:172
> void test_sfinae_constraints() {
> - {
> - using Tag = std::in_place_type_t<int>;
I split out these non-portable tests into
test/libcxx/utilities/any.class/any.assign/value.pass.cpp
> copy.pass.cpp:74
> assertContains<Type>(a, 42);
> - assertContains<Type>(a, 42);
> + assertContains<Type>(a2, 42);
>
Typo in the original test.
> move.pass.cpp:80
>
> - assert(Type::moved >= 1); // zero or more move operations can be
> performed.
> + assert(Type::moved == 1 || Type::moved == 2); // zero or one move
> operations can be performed.
> assert(Type::copied == 0); // no copies can be performed.
More move behavior.
> move.pass.cpp:97
> + , "any must be nothrow move constructible"
> + );
> }
Another inadvertent whitespace change.
> value.pass.cpp:110
>
> -void test_non_moveable_type()
> -{
Non-portable behavior split into
test/libcxx/utilities/any.class/any.cons/value.pass.cpp
> value.pass.cpp:143
> + NoCopy(NoCopy const&) = delete;
> + NoCopy(int) {}
> };
Inadvertent whitespace change.
> value.pass.cpp:182
> test_move_value_throws();
> - test_non_moveable_type();
> test_sfinae_constraints();
See above.
> emplace.pass.cpp:132
> LargeThrows(std::initializer_list<int>, int) { throw 42; }
> - int data[10];
> + int data[sizeof(std::any)];
> };
Ensure that `LargeThrows` is large, regardless of how bloated `std::any` may
become.
> emplace.pass.cpp:234
> + NoCopy(int) {}
> + NoCopy(std::initializer_list<int>, int, int) {}
> };
Inadvertent whitespace change.
> swap.pass.cpp:89
> + , "any::swap(any&) must be noexcept"
> + );
> +}
ibid.
> swap.pass.cpp:92
> +
> +void test_self_swap()
> +{
New test: self-`swap` must have no effect.
> any_cast_reference.pass.cpp:71
>
> -template <class Type, class ConstT = Type>
> -void checkThrows(any& a)
> +template <class Type>
> +void checkThrowsRvalue(any&, std::true_type)
Possibly controversial: I'm going to propose that `template<class T>
any_cast(any&&)` should require that `T` is not an lvalue reference type, so
that it cannot return potentially dangling references. That means the portion
of this test that performs `any_cast<T>(any&&)` must be conditioned on whether
or not `T` is an lvalue reference type.
> any_cast_reference.pass.cpp:146
> - // Check getting a type by reference from a non-const rvalue
> - {
> - Type& v = any_cast<Type&>(std::move(a));
This code becomes ill-formed with the above proposed change.
> any_cast_reference.pass.cpp:198
> any a((Type(42)));
> - any const& ca = a;
> assert(Type::count == 1);
Remove unused variable `ca`.
> any_cast_reference.pass.cpp:306
>
> -void test_cast_to_value_deleted_move()
> -{
Non-standard behavior split out into
test/libcxx/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp
> make_any.pass.cpp:102
> LargeThrows(std::initializer_list<int>, int) { throw 42; }
> - int data[10];
> + int data[sizeof(std::any)];
> };
Again, ensure that `LargeThrows` is large independent of the size of `any`.
https://reviews.llvm.org/D25249
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
