[PATCH] D25249: [libc++] Many any test fixes

[Casey Carter via cfe-commits]

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