[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D87974#2439043 , @jwakely wrote:

> In D87974#2438723 , @zoecarver wrote:
>
>> In D87974#2438682 , @BillyONeal 
>> wrote:
>>
 Are they actually the same, with the same handling of corner cases like 
 unions and tail padding?
 There's more to this than just the name, and if they aren't the same, it 
 seems better to have two names.
>>>
>>> They are both implementing the same C++ feature, with the same desired 
>>> semantics of zeroing out any bits in the object representation that aren't 
>>> in the value representation. If they differ, one or the other would have a 
>>> bug.
>
> Do they support non-trivially copyable types? That isn't required for the 
> atomic compare exchange feature, but is relevant for a feature exposed to 
> users. What about extensions like zero-sized arrays or C99 flexible array 
> members?

As far as MSVC is concerned this isn't "exposed to users".

> What about extensions like zero-sized arrays or C99 flexible array members?

We don't have those extensions at all so it's irrelevant to talk about what the 
builtin would do with them in our case. (And at such time we would add such 
extensions presumably we would match gcc's behavior since again, there's no 
reason for the behavior to differ here)

>> I agree, they either need to be identical (including corner cases) or there 
>> needs to be two of them (i.e., GCC ships both 
>> `__builtin_zero_non_value_bits` and `__builtin_clear_padding` and the first 
>> is the same as MSVC, Clang, and NVCC).
>
> GCC doesn't need to support both. It only works with libstdc++ so it only 
> needs to support the one used by libstdc++ (although there is a patch to add 
> `-stdlib=libc++` to GCC).
>
> If libstdc++ uses `__has_builtin` to check what the compiler supports then 
> Clang doesn't even need to support GCC's built-in, because libstdc++ wouldn't 
> use it if not supported (and could use `__builtin_zero_non_value_bits` 
> instead when supported).
>
> The Intel compiler would need to support both though.
>
 Is there a specification for __builtin_zero_non_value_bits available 
 somewhere?
>>>
>>> I don't know if there is a formal spec for it beyond the actual C++ 
>>> standard.
>>
>> I think P0528 is the relevant paper but other than that, no, there's not a 
>> spec. I think that's going to be the most time sensitive part of 
>> implementing this: coming up with the spec and making sure all the tests 
>> pass on all the implementations.
>
> GCC has publicly available documentation describing its built-in, and 
> publicly available tests for it. That's the kind of spec I'm looking for.

We don't consider it "publicly available" so there isn't going to be that kind 
of documentation for it. I don't see a serious problem with the gcc version of 
that builtin supporting a superset of the functionality of the equivalent msvc 
builtin.

Of course if it's already publicly documented for you the horse has presumably 
already left the barn which makes the discussion moot?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

> Are they actually the same, with the same handling of corner cases like 
> unions and tail padding?
> There's more to this than just the name, and if they aren't the same, it 
> seems better to have two names.

They are both implementing the same C++ feature, with the same desired 
semantics of zeroing out any bits in the object representation that aren't in 
the value representation. If they differ, one or the other would have a bug.

> Is there a specification for __builtin_zero_non_value_bits available 
> somewhere?

I don't know if there is a formal spec for it beyond the actual C++ standard.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D87974: [Builtin] Add __builtin_zero_non_value_bits.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D87974#2432793 , @zoecarver wrote:

> So, it looks like GCC already uses `__builtin_clear_padding` and MSVC already 
> uses `__builtin_zero_non_value_bits`. This patch (obviously) is currently 
> implementing `__builtin_zero_non_value_bits ` but, I had planned to update it 
> to use `__builtin_clear_padding`. Maybe that's not the best course of action, 
> though.
>
> We should all try to agree on _one_ name. CC @BillyONeal @jwakely thoughts?

The name MSFT is already shipping in production is 
`__builtin_zero_non_value_bits`. If gcc is already shipping another name in 
production I think clang is stuck supporting both names, if gcc has not yet 
shipped their implementation perhaps we can choose one. That seems to be more 
on gcc than it is on clang given clang's desire to be more or less a drop in 
replacement for either gcc or msvc.

The MSFT STL implementation can of course use a different builtin when we 
detect clang.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87974/new/

https://reviews.llvm.org/D87974

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73138: [libcxx] [test] Correct asserted type in subspan test; subspan with count should never produce dynamic_extent

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73138/new/

https://reviews.llvm.org/D73138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73138: [libcxx] [test] Correct asserted type in subspan test; subspan with count should never produce dynamic_extent

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

@ldionne I think you're correct, though the test there is more complex than it 
needs to be (because the ==Count cases are the only ever encountered here).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73138/new/

https://reviews.llvm.org/D73138



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

https://github.com/llvm/llvm-project/commit/45f630d729e2cce044ed48e6eaf4b8e61e06fede
 Committed


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61366/new/

https://reviews.llvm.org/D61366



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49530: [libc++] Implement Directory Entry Caching -- Sort of.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.
Herald added subscribers: libcxx-commits, dexonsmith.

> @BillyONeal has proposed a more reasonable set of requirements

I believe that was you. I wanted the throws/fails behavior.


Repository:
  rCXX libc++

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D49530/new/

https://reviews.llvm.org/D49530



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61365: [libcxx] [test] Suppress float->int narrowing warning in vector range-construction test.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal abandoned this revision.
BillyONeal added a comment.

It looks like someone already fixed this.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61365/new/

https://reviews.llvm.org/D61365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D73138: [libcxx] [test] Correct asserted type in subspan test; subspan with count should never produce dynamic_extent

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: ldionne, EricWF, mclow.lists.
Herald added subscribers: dexonsmith, christof.

I'm not sure if this was a change in the standard behavior or if it was a typo 
in libcxx from the beginning. http://eel.is/c++draft/span.sub#10 says that if a 
count is supplied, the returned span is always of that count, never of 
dynamic_extent, so these tests were asserting incorrectly.

I'm not sure what to do about this as fixing the test to assert correctly (and 
thus make msvc++ pass) is going to make libcxx fail.


https://reviews.llvm.org/D73138

Files:
  libcxx/test/std/containers/views/span.sub/subspan.pass.cpp


Index: libcxx/test/std/containers/views/span.sub/subspan.pass.cpp
===
--- libcxx/test/std/containers/views/span.sub/subspan.pass.cpp
+++ libcxx/test/std/containers/views/span.sub/subspan.pass.cpp
@@ -37,7 +37,7 @@
 using S2 = decltype(s2);
 ASSERT_SAME_TYPE(typename Span::value_type, typename S1::value_type);
 ASSERT_SAME_TYPE(typename Span::value_type, typename S2::value_type);
-static_assert(S1::extent == (Span::extent == std::dynamic_extent ? 
std::dynamic_extent : Count), "");
+static_assert(S1::extent == Count, "");
 static_assert(S2::extent == std::dynamic_extent, "");
 return
 s1.data() == s2.data()
@@ -76,7 +76,7 @@
 using S2 = decltype(s2);
 ASSERT_SAME_TYPE(typename Span::value_type, typename S1::value_type);
 ASSERT_SAME_TYPE(typename Span::value_type, typename S2::value_type);
-static_assert(S1::extent == (Span::extent == std::dynamic_extent ? 
std::dynamic_extent : Count), "");
+static_assert(S1::extent == Count, "");
 static_assert(S2::extent == std::dynamic_extent, "");
 assert(s1.data() == s2.data());
 assert(s1.size() == s2.size());


Index: libcxx/test/std/containers/views/span.sub/subspan.pass.cpp
===
--- libcxx/test/std/containers/views/span.sub/subspan.pass.cpp
+++ libcxx/test/std/containers/views/span.sub/subspan.pass.cpp
@@ -37,7 +37,7 @@
 using S2 = decltype(s2);
 ASSERT_SAME_TYPE(typename Span::value_type, typename S1::value_type);
 ASSERT_SAME_TYPE(typename Span::value_type, typename S2::value_type);
-static_assert(S1::extent == (Span::extent == std::dynamic_extent ? std::dynamic_extent : Count), "");
+static_assert(S1::extent == Count, "");
 static_assert(S2::extent == std::dynamic_extent, "");
 return
 s1.data() == s2.data()
@@ -76,7 +76,7 @@
 using S2 = decltype(s2);
 ASSERT_SAME_TYPE(typename Span::value_type, typename S1::value_type);
 ASSERT_SAME_TYPE(typename Span::value_type, typename S2::value_type);
-static_assert(S1::extent == (Span::extent == std::dynamic_extent ? std::dynamic_extent : Count), "");
+static_assert(S1::extent == Count, "");
 static_assert(S2::extent == std::dynamic_extent, "");
 assert(s1.data() == s2.data());
 assert(s1.size() == s2.size());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65249: [NFC] use C++11 in AlignOf.h

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

(In fact I observe many patterns in this review like:

enum { Foo = alignof(void*); }
aligned_storage_t<1234, Foo> x;

and then a bunch of casting to treat it as a char buffer; if it was just born 
as a char buffer you can remove both the casts and the enum hack:

alignas(void*) char x[1234];


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65249/new/

https://reviews.llvm.org/D65249



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65249: [NFC] use C++11 in AlignOf.h

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D65249#1603335 , @jfb wrote:

> @rnk: how about I add a bit of code that wraps `aligned_storage` on all 
> platforms except MSVC (where I'd implement it as Billy suggests). That would 
> mean updating all the uses of `aligned_storage` to this LLVM one, but that's 
> not a big deal.


Why not just always use the alignas version? Or even better, not involve a 
library for something you can say in the core language at all?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65249/new/

https://reviews.llvm.org/D65249



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65249: [NFC] use C++11 in AlignOf.h

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

> @BillyONeal do you know if 19.11 has the aligned_storage issue on x86?

aligned_storage is still capped at what the library can fake with unions. It 
would be an ABI break to change it to use alignas, so things where the x86 
stack temporarily breaks T's usual alignment rules will affect it (and for all 
releases using VS2015's ABI). We recommend customers use `alignas(T) unsigned 
char space[sizeof(T)]` if they can't tolerate such limitations.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65249/new/

https://reviews.llvm.org/D65249



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D61366#1569151 , @mclow.lists wrote:

> Is there a reason this hasn't been committed?


Because it needs that one change Casey requested I was going to do that next 
time we take a libcxx test harness update, and I was hoping we'd get some 
closure on what happens with https://reviews.llvm.org/D61364 before doing that. 
Would you like me to expedite this change specifically?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61366/new/

https://reviews.llvm.org/D61366



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal marked an inline comment as done.
BillyONeal added inline comments.



Comment at: test/std/containers/map_allocator_requirement_test_templates.h:236
 const ValueTp v(42, 1);
-cc->expect();
+cc->expect();
 It ret = c.insert(c.end(), std::move(v));

EricWF wrote:
> I really think the current behavior libc++ has here is correct.
> 
> 
File a DR to retroactively make libc++'s behavior standard then? :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61364/new/

https://reviews.llvm.org/D61364



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D61364#1502200 , @EricWF wrote:

> From Billy and my last discussion, I think we came to the agreement that it's 
> not clear exactly what the "standard behavior" is.


No, I don't think so. I think there was agreement that the behavior in the 
standard may not have been what was intended by the original move semantics 
proposals, but I don't think there's any doubt about what the standard 
currently requires.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61364/new/

https://reviews.llvm.org/D61364



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D61364#1502172 , @ldionne wrote:

> Do you know *why* the tests are failing with libc++? I see this overload for 
> `insert` and it seems like it should be a better match?


No, I haven't investigated. I avoid looking at libc++ product code too much for 
now because I want to avoid any appearance or reality of licensing conflicts... 
for now  ;) .

In D61364#1502172 , @ldionne wrote:

> I think we should just fix libc++ to do the right thing (according to the 
> standard).


That's what I'd recommend :)

In D61364#1502172 , @ldionne wrote:

> Separately, I'm not sure it's worth changing the Standard to ensure that the 
> `T const&` overload is selected -- I don't see a huge benefit here.


Would be a code size win if both overloads are called in the same program.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61364/new/

https://reviews.llvm.org/D61364



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D61366#1502170 , @Quuxplusone wrote:

> you are indeed allowed to //move// the //elements//


And indeed we *must* do that :).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61366/new/

https://reviews.llvm.org/D61366



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In D61366#1502073 , @EricWF wrote:

> I'm not sure I agree with your design decision, but this patch LGTM.


I wouldn't object to a standards change to make this the case; though it is 
suboptimal to destroy all the elements needlessly.

> Are you not allowed to move the containers elements in this case?

Correct. The allocator is not POCMA and not equal, so it's functionally the 
same as doing assign(make_move_iterator(begin()), make_move_iterator(end())).

Billy3


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61366/new/

https://reviews.llvm.org/D61366



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61365: [libcxx] [test] Suppress float->int narrowing warning in vector range-construction test.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal marked an inline comment as done.
BillyONeal added inline comments.



Comment at: 
test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp:159
 float array[3] = {0.0f, 1.0f, 2.0f};
+#pragma warning(suppress: 4244) // narrowing float to int
 std::vector v(array, array + 3);

CaseyCarter wrote:
> This will blow up non-MSVC-ish when running the test suite with `-Wall -W 
> -Werror` (which is typical). I suggest wrapping in `#ifdef _MSC_VER`.
Why didn't it blow up on Contest then? clang-cl is happy with it?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61365/new/

https://reviews.llvm.org/D61365



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 197653.
BillyONeal added a comment.

Also fixed vector test.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61366/new/

https://reviews.llvm.org/D61366

Files:
  test/std/containers/associative/map/map.cons/move_assign.pass.cpp
  test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
  test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
  test/std/containers/associative/set/set.cons/move_assign.pass.cpp
  test/std/containers/sequences/vector.bool/assign_move.pass.cpp


Index: test/std/containers/sequences/vector.bool/assign_move.pass.cpp
===
--- test/std/containers/sequences/vector.bool/assign_move.pass.cpp
+++ test/std/containers/sequences/vector.bool/assign_move.pass.cpp
@@ -30,7 +30,7 @@
 std::vector > l2(test_allocator(5));
 l2 = std::move(l);
 assert(l2 == lo);
-assert(l.empty());
+LIBCPP_ASSERT(l.empty());
 assert(l2.get_allocator() == lo.get_allocator());
 }
 {
Index: test/std/containers/associative/set/set.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/set/set.cons/move_assign.pass.cpp
+++ test/std/containers/associative/set/set.cons/move_assign.pass.cpp
@@ -100,7 +100,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef MoveOnly V;
Index: 
test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
+++ test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
@@ -100,7 +100,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef MoveOnly V;
Index: 
test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
+++ test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
@@ -102,7 +102,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef std::pair V;
Index: test/std/containers/associative/map/map.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/map/map.cons/move_assign.pass.cpp
+++ test/std/containers/associative/map/map.cons/move_assign.pass.cpp
@@ -102,7 +102,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef std::pair V;


Index: test/std/containers/sequences/vector.bool/assign_move.pass.cpp
===
--- test/std/containers/sequences/vector.bool/assign_move.pass.cpp
+++ test/std/containers/sequences/vector.bool/assign_move.pass.cpp
@@ -30,7 +30,7 @@
 std::vector > l2(test_allocator(5));
 l2 = std::move(l);
 assert(l2 == lo);
-assert(l.empty());
+LIBCPP_ASSERT(l.empty());
 assert(l2.get_allocator() == lo.get_allocator());
 }
 {
Index: test/std/containers/associative/set/set.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/set/set.cons/move_assign.pass.cpp
+++ test/std/containers/associative/set/set.cons/move_assign.pass.cpp
@@ -100,7 +100,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef MoveOnly V;
Index: test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
+++ test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
@@ -100,7 +100,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef MoveOnly V;
Index: test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
===

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF, ldionne.
Herald added a subscriber: dexonsmith.

The standard only requires that moved-from standard library types are in a 
'valid but unspecified state', not that any moved-from container is empty. When 
the container is using a POCMA allocator, asserting that the moved-from 
container is empty is reasonable because the target container needs to take 
ownership of the memory buffers allocated from the source container's 
allocator. However, when the allocator is non-POCMA, the destination container 
must not take over any buffers, and effectively must copy the contents of the 
source container.

In the MSVC++ implementation, in this non-POCMA case, we do not clear() the 
source container, so that subsequent operations can reuse memory if the 
container is not immediately destroyed.


https://reviews.llvm.org/D61366

Files:
  test/std/containers/associative/map/map.cons/move_assign.pass.cpp
  test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
  test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
  test/std/containers/associative/set/set.cons/move_assign.pass.cpp


Index: test/std/containers/associative/set/set.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/set/set.cons/move_assign.pass.cpp
+++ test/std/containers/associative/set/set.cons/move_assign.pass.cpp
@@ -100,7 +100,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef MoveOnly V;
Index: 
test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
+++ test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
@@ -100,7 +100,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef MoveOnly V;
Index: 
test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
+++ test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
@@ -102,7 +102,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef std::pair V;
Index: test/std/containers/associative/map/map.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/map/map.cons/move_assign.pass.cpp
+++ test/std/containers/associative/map/map.cons/move_assign.pass.cpp
@@ -102,7 +102,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef std::pair V;


Index: test/std/containers/associative/set/set.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/set/set.cons/move_assign.pass.cpp
+++ test/std/containers/associative/set/set.cons/move_assign.pass.cpp
@@ -100,7 +100,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef MoveOnly V;
Index: test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
+++ test/std/containers/associative/multiset/multiset.cons/move_assign.pass.cpp
@@ -100,7 +100,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef MoveOnly V;
Index: test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
===
--- test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
+++ test/std/containers/associative/multimap/multimap.cons/move_assign.pass.cpp
@@ -102,7 +102,7 @@
 assert(m3 == m2);
 assert(m3.get_allocator() == A(5));
 assert(m3.key_comp() == C(5));
-assert(m1.empty());
+LIBCPP_ASSERT(m1.empty());
 }
 {
 typedef std::pair V;
Index: 

[PATCH] D61365: [libcxx] [test] Suppress float->int narrowing warning in vector range-construction test.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF, ldionne.
Herald added a subscriber: dexonsmith.

https://reviews.llvm.org/D61365

Files:
  test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp


Index: 
test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
===
--- 
test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
+++ 
test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
@@ -156,6 +156,7 @@
 // Make sure initialization is performed with each element value, not with
 // a memory blob.
 float array[3] = {0.0f, 1.0f, 2.0f};
+#pragma warning(suppress: 4244) // narrowing float to int
 std::vector v(array, array + 3);
 assert(v[0] == 0);
 assert(v[1] == 1);


Index: test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
===
--- test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
+++ test/std/containers/sequences/vector/vector.cons/construct_iter_iter.pass.cpp
@@ -156,6 +156,7 @@
 // Make sure initialization is performed with each element value, not with
 // a memory blob.
 float array[3] = {0.0f, 1.0f, 2.0f};
+#pragma warning(suppress: 4244) // narrowing float to int
 std::vector v(array, array + 3);
 assert(v[0] == 0);
 assert(v[1] == 1);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists, ldionne.
Herald added a subscriber: dexonsmith.

In 
$:\Dev\msvc\src\qa\VC\Libs\libcxx\upstream\test\std\containers\map_allocator_requirement_test_templates.h
 and $\test\std\containers\set_allocator_requirement_test_templates.h, libcxx 
tests are asserting that the container's insert operation passes an incorrect 
type down to allocator::construct. Specifically, given something like:

  {
CHECKPOINT("Testing C::insert(value_type&)");
Container c;
ValueTp v(42, 1);

- cc->expect();** assert(c.insert(v).second); 
assert(!cc->unchecked()); { DisableAllocationGuard g; ValueTp v2(42, 1); 
assert(c.insert(v2).second == false); } }

This call to ::insert ends up calling the insert(P&&), not insert(const 
value_type&), because P is deduced to value_type&, meaning the insert(P&&) 
overload is an exact match, while insert(const value_type&) requires adding 
const. See http://eel.is/c++draft/associative#map.modifiers

When the insert(P&&) is called, it delegates to emplace, which only gets 
Cpp17EmplaceConstructible from the supplied parameters, not 
Cpp17EmplaceConstructible from add_const_t of the parameters. Thus, libcxx's 
test is asserting a condition the standard explicitly does not allow at this 
time. (Though it may be reasonable to fix the standard to make what libcxx is 
doing okay)

Unfortunately with the tests fixed to assert the correct allocator::construct 
call, libc++ will fail these tests. I'm looking for guidance on how libc++ 
maintainers would like to handle this.


https://reviews.llvm.org/D61364

Files:
  test/std/containers/map_allocator_requirement_test_templates.h
  test/std/containers/set_allocator_requirement_test_templates.h


Index: test/std/containers/set_allocator_requirement_test_templates.h
===
--- test/std/containers/set_allocator_requirement_test_templates.h
+++ test/std/containers/set_allocator_requirement_test_templates.h
@@ -128,7 +128,7 @@
 CHECKPOINT("Testing C::insert(Iter, Iter) for *Iter = value_type&");
 Container c;
 ValueTp ValueList[] = { ValueTp(1), ValueTp(2) , ValueTp(3) };
-cc->expect(3);
+cc->expect(3);
 c.insert(std::begin(ValueList), std::end(ValueList));
 assert(!cc->unchecked());
 {
Index: test/std/containers/map_allocator_requirement_test_templates.h
===
--- test/std/containers/map_allocator_requirement_test_templates.h
+++ test/std/containers/map_allocator_requirement_test_templates.h
@@ -51,7 +51,7 @@
 CHECKPOINT("Testing C::insert(value_type&)");
 Container c;
 ValueTp v(42, 1);
-cc->expect();
+cc->expect();
 assert(c.insert(v).second);
 assert(!cc->unchecked());
 {
@@ -77,7 +77,7 @@
 CHECKPOINT("Testing C::insert(const value_type&&)");
 Container c;
 const ValueTp v(42, 1);
-cc->expect();
+cc->expect();
 assert(c.insert(std::move(v)).second);
 assert(!cc->unchecked());
 {
@@ -141,7 +141,7 @@
 CHECKPOINT("Testing C::insert(Iter, Iter) for *Iter = value_type&");
 Container c;
 ValueTp ValueList[] = { ValueTp(1, 1), ValueTp(2, 1) , ValueTp(3, 1) };
-cc->expect(3);
+cc->expect(3);
 c.insert(std::begin(ValueList), std::end(ValueList));
 assert(!cc->unchecked());
 {
@@ -184,7 +184,7 @@
 CHECKPOINT("Testing C::insert(p, value_type&)");
 Container c;
 ValueTp v(42, 1);
-cc->expect();
+cc->expect();
 It ret = c.insert(c.end(), v);
 assert(ret != c.end());
 assert(c.size() == 1);
@@ -233,7 +233,7 @@
 CHECKPOINT("Testing C::insert(p, const value_type&&)");
 Container c;
 const ValueTp v(42, 1);
-cc->expect();
+cc->expect();
 It ret = c.insert(c.end(), std::move(v));
 assert(ret != c.end());
 assert(c.size() == 1);


Index: test/std/containers/set_allocator_requirement_test_templates.h
===
--- test/std/containers/set_allocator_requirement_test_templates.h
+++ test/std/containers/set_allocator_requirement_test_templates.h
@@ -128,7 +128,7 @@
 CHECKPOINT("Testing C::insert(Iter, Iter) for *Iter = value_type&");
 Container c;
 ValueTp ValueList[] = { ValueTp(1), ValueTp(2) , ValueTp(3) };
-cc->expect(3);
+cc->expect(3);
 c.insert(std::begin(ValueList), std::end(ValueList));
 assert(!cc->unchecked());
 {
Index: test/std/containers/map_allocator_requirement_test_templates.h
===
--- test/std/containers/map_allocator_requirement_test_templates.h
+++ test/std/containers/map_allocator_requirement_test_templates.h
@@ -51,7 +51,7 @@
 CHECKPOINT("Testing C::insert(value_type&)");
 Container c;
 ValueTp v(42, 1);
-cc->expect();
+cc->expect();
 assert(c.insert(v).second);
 

[PATCH] D60104: [libcxx] [test] Use ptrdiff_t rather than int in splice_after_range.pass.cpp to avoid narrowing from pointer subtraction to int warnings.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

Committed r357546


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60104/new/

https://reviews.llvm.org/D60104



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix test bugs in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 193393.
BillyONeal retitled this revision from "[libcxx] [test] Fix inability to rebind 
poca_alloc in string.cons/copy_alloc.pass.cpp." to "[libcxx] [test] Fix test 
bugs in string.cons/copy_alloc.pass.cpp.".
BillyONeal edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60023/new/

https://reviews.llvm.org/D60023

Files:
  test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp


Index: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
@@ -18,12 +18,12 @@
 #include "min_allocator.h"
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
-template 
 struct alloc_imp {
 bool active;
 
 alloc_imp() : active(true) {}
 
+template 
 T* allocate(std::size_t n)
 {
 if (active)
@@ -32,6 +32,7 @@
 throw std::bad_alloc();
 }
 
+template 
 void deallocate(T* p, std::size_t) { std::free(p); }
 void activate  ()  { active = true; }
 void deactivate()  { active = false; }
@@ -42,14 +43,14 @@
 typedef T value_type;
 typedef std::true_type propagate_on_container_copy_assignment;
 
-alloc_imp *imp;
+alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp_) : imp (imp_) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
 
-T*   allocate  (std::size_t n)   { return imp->allocate(n);}
+T*   allocate  (std::size_t n)   { return imp->allocate(n);}
 void deallocate(T* p, std::size_t n) { imp->deallocate(p, n); }
 };
 
@@ -112,8 +113,8 @@
 const char * p1 = "This is my first string";
 const char * p2 = "This is my second string";
 
-alloc_imp imp1;
-alloc_imp imp2;
+alloc_imp imp1;
+alloc_imp imp2;
 S s1(p1, A());
 S s2(p2, A());
 


Index: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
@@ -18,12 +18,12 @@
 #include "min_allocator.h"
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
-template 
 struct alloc_imp {
 bool active;
 
 alloc_imp() : active(true) {}
 
+template 
 T* allocate(std::size_t n)
 {
 if (active)
@@ -32,6 +32,7 @@
 throw std::bad_alloc();
 }
 
+template 
 void deallocate(T* p, std::size_t) { std::free(p); }
 void activate  ()  { active = true; }
 void deactivate()  { active = false; }
@@ -42,14 +43,14 @@
 typedef T value_type;
 typedef std::true_type propagate_on_container_copy_assignment;
 
-alloc_imp *imp;
+alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp_) : imp (imp_) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
 
-T*   allocate  (std::size_t n)   { return imp->allocate(n);}
+T*   allocate  (std::size_t n)   { return imp->allocate(n);}
 void deallocate(T* p, std::size_t n) { imp->deallocate(p, n); }
 };
 
@@ -112,8 +113,8 @@
 const char * p1 = "This is my first string";
 const char * p2 = "This is my second string";
 
-alloc_imp imp1;
-alloc_imp imp2;
+alloc_imp imp1;
+alloc_imp imp2;
 S s1(p1, A());
 S s2(p2, A());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix test bugs in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

Committed r357545


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60023/new/

https://reviews.llvm.org/D60023



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix inability to rebind poca_alloc in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

Tim Song suggests that http://eel.is/c++draft/string.require#2 indicates that 
basic_string actually does need to provide the strong guarantee here. While I 
think that wording is a mess I think I'll fix our basic_string to do that 
instead of that part of this change.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60023/new/

https://reviews.llvm.org/D60023



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix inability to rebind poca_alloc in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 193216.
BillyONeal added a comment.

Fixed misspelled test macro.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60023/new/

https://reviews.llvm.org/D60023

Files:
  test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp


Index: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
@@ -18,12 +18,12 @@
 #include "min_allocator.h"
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
-template 
 struct alloc_imp {
 bool active;
 
 alloc_imp() : active(true) {}
 
+template 
 T* allocate(std::size_t n)
 {
 if (active)
@@ -32,6 +32,7 @@
 throw std::bad_alloc();
 }
 
+template 
 void deallocate(T* p, std::size_t) { std::free(p); }
 void activate  ()  { active = true; }
 void deactivate()  { active = false; }
@@ -42,14 +43,14 @@
 typedef T value_type;
 typedef std::true_type propagate_on_container_copy_assignment;
 
-alloc_imp *imp;
+alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp_) : imp (imp_) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
 
-T*   allocate  (std::size_t n)   { return imp->allocate(n);}
+T*   allocate  (std::size_t n)   { return imp->allocate(n);}
 void deallocate(T* p, std::size_t n) { imp->deallocate(p, n); }
 };
 
@@ -112,8 +113,8 @@
 const char * p1 = "This is my first string";
 const char * p2 = "This is my second string";
 
-alloc_imp imp1;
-alloc_imp imp2;
+alloc_imp imp1;
+alloc_imp imp2;
 S s1(p1, A());
 S s2(p2, A());
 
@@ -122,7 +123,11 @@
 
 imp2.deactivate();
 test_assign(s1, s2);
-assert(s1 == p1);
+// libc++ provides the strong exception safety guarantee on the copy 
assignment operator,
+// but the standard only requires the basic guarantee:
+LIBCPP_ASSERT(s1 == p1);
+s1.clear(); // under the basic guarantee, s1 must still be a valid string 
object.
+assert(s1.empty());
 assert(s2 == p2);
 }
 #endif


Index: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
@@ -18,12 +18,12 @@
 #include "min_allocator.h"
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
-template 
 struct alloc_imp {
 bool active;
 
 alloc_imp() : active(true) {}
 
+template 
 T* allocate(std::size_t n)
 {
 if (active)
@@ -32,6 +32,7 @@
 throw std::bad_alloc();
 }
 
+template 
 void deallocate(T* p, std::size_t) { std::free(p); }
 void activate  ()  { active = true; }
 void deactivate()  { active = false; }
@@ -42,14 +43,14 @@
 typedef T value_type;
 typedef std::true_type propagate_on_container_copy_assignment;
 
-alloc_imp *imp;
+alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp_) : imp (imp_) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
 
-T*   allocate  (std::size_t n)   { return imp->allocate(n);}
+T*   allocate  (std::size_t n)   { return imp->allocate(n);}
 void deallocate(T* p, std::size_t n) { imp->deallocate(p, n); }
 };
 
@@ -112,8 +113,8 @@
 const char * p1 = "This is my first string";
 const char * p2 = "This is my second string";
 
-alloc_imp imp1;
-alloc_imp imp2;
+alloc_imp imp1;
+alloc_imp imp2;
 S s1(p1, A());
 S s2(p2, A());
 
@@ -122,7 +123,11 @@
 
 imp2.deactivate();
 test_assign(s1, s2);
-assert(s1 == p1);
+// libc++ provides the strong exception safety guarantee on the copy assignment operator,
+// but the standard only requires the basic guarantee:
+LIBCPP_ASSERT(s1 == p1);
+s1.clear(); // under the basic guarantee, s1 must still be a valid string object.
+assert(s1.empty());
 assert(s2 == p2);
 }
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60104: [libcxx] [test] Use ptrdiff_t rather than int in splice_after_range.pass.cpp to avoid narrowing from pointer subtraction to int warnings.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: ldionne, mclow.lists, EricWF.
Herald added a subscriber: dexonsmith.

https://reviews.llvm.org/D60104

Files:
  
test/std/containers/sequences/forwardlist/forwardlist.ops/splice_after_range.pass.cpp

Index: test/std/containers/sequences/forwardlist/forwardlist.ops/splice_after_range.pass.cpp
===
--- test/std/containers/sequences/forwardlist/forwardlist.ops/splice_after_range.pass.cpp
+++ test/std/containers/sequences/forwardlist/forwardlist.ops/splice_after_range.pass.cpp
@@ -11,27 +11,28 @@
 // void splice_after(const_iterator p, forward_list&& x,
 //   const_iterator first, const_iterator last);
 
+#include 
 #include 
 #include 
 #include 
 
 #include "min_allocator.h"
 
-typedef int T;
+typedef ptrdiff_t T;
 const T t1[] = {0, 1, 2, 3, 4, 5, 6, 7};
 const T t2[] = {10, 11, 12, 13, 14, 15};
-const int size_t1 = std::end(t1) - std::begin(t1);
-const int size_t2 = std::end(t2) - std::begin(t2);
+const ptrdiff_t size_t1 = std::end(t1) - std::begin(t1);
+const ptrdiff_t size_t2 = std::end(t2) - std::begin(t2);
 
 template 
 void
-testd(const C& c, int p, int f, int l)
+testd(const C& c, ptrdiff_t p, ptrdiff_t f, ptrdiff_t l)
 {
 typename C::const_iterator i = c.begin();
-int n1 = 0;
+ptrdiff_t n1 = 0;
 for (; n1 < p; ++n1, ++i)
 assert(*i == t1[n1]);
-for (int n2 = f; n2 < l-1; ++n2, ++i)
+for (ptrdiff_t n2 = f; n2 < l-1; ++n2, ++i)
 assert(*i == t2[n2]);
 for (; n1 < size_t1; ++n1, ++i)
 assert(*i == t1[n1]);
@@ -40,11 +41,11 @@
 
 template 
 void
-tests(const C& c, int p, int f, int l)
+tests(const C& c, ptrdiff_t p, ptrdiff_t f, ptrdiff_t l)
 {
 typename C::const_iterator i = c.begin();
-int n = 0;
-int d = l > f+1 ? l-1-f : 0;
+ptrdiff_t n = 0;
+ptrdiff_t d = l > f+1 ? l-1-f : 0;
 if (d == 0 || p == f)
 {
 for (n = 0; n < size_t1; ++n, ++i)
@@ -80,11 +81,11 @@
 {
 // splicing different containers
 typedef std::forward_list C;
-for (int f = 0; f <= size_t2+1; ++f)
+for (ptrdiff_t f = 0; f <= size_t2+1; ++f)
 {
-for (int l = f; l <= size_t2+1; ++l)
+for (ptrdiff_t l = f; l <= size_t2+1; ++l)
 {
-for (int p = 0; p <= size_t1; ++p)
+for (ptrdiff_t p = 0; p <= size_t1; ++p)
 {
 C c1(std::begin(t1), std::end(t1));
 C c2(std::begin(t2), std::end(t2));
@@ -97,11 +98,11 @@
 }
 
 // splicing within same container
-for (int f = 0; f <= size_t1+1; ++f)
+for (ptrdiff_t f = 0; f <= size_t1+1; ++f)
 {
-for (int l = f; l <= size_t1; ++l)
+for (ptrdiff_t l = f; l <= size_t1; ++l)
 {
-for (int p = 0; p <= f; ++p)
+for (ptrdiff_t p = 0; p <= f; ++p)
 {
 C c1(std::begin(t1), std::end(t1));
 
@@ -109,7 +110,7 @@
   next(c1.cbefore_begin(), f), next(c1.cbefore_begin(), l));
 tests(c1, p, f, l);
 }
-for (int p = l; p <= size_t1; ++p)
+for (ptrdiff_t p = l; p <= size_t1; ++p)
 {
 C c1(std::begin(t1), std::end(t1));
 
@@ -124,11 +125,11 @@
 {
 // splicing different containers
 typedef std::forward_list> C;
-for (int f = 0; f <= size_t2+1; ++f)
+for (ptrdiff_t f = 0; f <= size_t2+1; ++f)
 {
-for (int l = f; l <= size_t2+1; ++l)
+for (ptrdiff_t l = f; l <= size_t2+1; ++l)
 {
-for (int p = 0; p <= size_t1; ++p)
+for (ptrdiff_t p = 0; p <= size_t1; ++p)
 {
 C c1(std::begin(t1), std::end(t1));
 C c2(std::begin(t2), std::end(t2));
@@ -141,11 +142,11 @@
 }
 
 // splicing within same container
-for (int f = 0; f <= size_t1+1; ++f)
+for (ptrdiff_t f = 0; f <= size_t1+1; ++f)
 {
-for (int l = f; l <= size_t1; ++l)
+for (ptrdiff_t l = f; l <= size_t1; ++l)
 {
-for (int p = 0; p <= f; ++p)
+for (ptrdiff_t p = 0; p <= f; ++p)
 {
 C c1(std::begin(t1), std::end(t1));
 
@@ -153,7 +154,7 @@
   next(c1.cbefore_begin(), f), next(c1.cbefore_begin(), l));
 tests(c1, p, f, l);
 }
-for (int p = l; p <= size_t1; ++p)
+for (ptrdiff_t p = l; p <= size_t1; ++p)
 {
 C c1(std::begin(t1), std::end(t1));
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix inability to rebind poca_alloc in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 193157.
BillyONeal edited the summary of this revision.
BillyONeal added a comment.

Fix asserts for the strong EH guarantee.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60023/new/

https://reviews.llvm.org/D60023

Files:
  test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp


Index: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
@@ -18,12 +18,12 @@
 #include "min_allocator.h"
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
-template 
 struct alloc_imp {
 bool active;
 
 alloc_imp() : active(true) {}
 
+template 
 T* allocate(std::size_t n)
 {
 if (active)
@@ -32,6 +32,7 @@
 throw std::bad_alloc();
 }
 
+template 
 void deallocate(T* p, std::size_t) { std::free(p); }
 void activate  ()  { active = true; }
 void deactivate()  { active = false; }
@@ -42,14 +43,14 @@
 typedef T value_type;
 typedef std::true_type propagate_on_container_copy_assignment;
 
-alloc_imp *imp;
+alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp_) : imp (imp_) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
 
-T*   allocate  (std::size_t n)   { return imp->allocate(n);}
+T*   allocate  (std::size_t n)   { return imp->allocate(n);}
 void deallocate(T* p, std::size_t n) { imp->deallocate(p, n); }
 };
 
@@ -112,8 +113,8 @@
 const char * p1 = "This is my first string";
 const char * p2 = "This is my second string";
 
-alloc_imp imp1;
-alloc_imp imp2;
+alloc_imp imp1;
+alloc_imp imp2;
 S s1(p1, A());
 S s2(p2, A());
 
@@ -122,7 +123,11 @@
 
 imp2.deactivate();
 test_assign(s1, s2);
-assert(s1 == p1);
+// libc++ provides the strong exception safety guarantee on the copy 
assignment operator,
+// but the standard only requires the basic guarantee:
+_LIBCXX_ASSERT(s1 == p1);
+s1.clear(); // under the basic guarantee, s1 must still be a valid string 
object.
+assert(s1.empty());
 assert(s2 == p2);
 }
 #endif


Index: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
@@ -18,12 +18,12 @@
 #include "min_allocator.h"
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
-template 
 struct alloc_imp {
 bool active;
 
 alloc_imp() : active(true) {}
 
+template 
 T* allocate(std::size_t n)
 {
 if (active)
@@ -32,6 +32,7 @@
 throw std::bad_alloc();
 }
 
+template 
 void deallocate(T* p, std::size_t) { std::free(p); }
 void activate  ()  { active = true; }
 void deactivate()  { active = false; }
@@ -42,14 +43,14 @@
 typedef T value_type;
 typedef std::true_type propagate_on_container_copy_assignment;
 
-alloc_imp *imp;
+alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp_) : imp (imp_) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
 
-T*   allocate  (std::size_t n)   { return imp->allocate(n);}
+T*   allocate  (std::size_t n)   { return imp->allocate(n);}
 void deallocate(T* p, std::size_t n) { imp->deallocate(p, n); }
 };
 
@@ -112,8 +113,8 @@
 const char * p1 = "This is my first string";
 const char * p2 = "This is my second string";
 
-alloc_imp imp1;
-alloc_imp imp2;
+alloc_imp imp1;
+alloc_imp imp2;
 S s1(p1, A());
 S s2(p2, A());
 
@@ -122,7 +123,11 @@
 
 imp2.deactivate();
 test_assign(s1, s2);
-assert(s1 == p1);
+// libc++ provides the strong exception safety guarantee on the copy assignment operator,
+// but the standard only requires the basic guarantee:
+_LIBCXX_ASSERT(s1 == p1);
+s1.clear(); // under the basic guarantee, s1 must still be a valid string object.
+assert(s1.empty());
 assert(s2 == p2);
 }
 #endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix inability to rebind poca_alloc in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal marked an inline comment as done.
BillyONeal added inline comments.



Comment at: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp:125
 test_assign(s1, s2);
 assert(s1 == p1);
 assert(s2 == p2);

This particular assert tests that the copy assignment operator provides the 
strong exception safety guarantee, but the standard requires only the basic 
guarantee. Should this be _LIBCXX_ASSERT?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60023/new/

https://reviews.llvm.org/D60023



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix inability to rebind poca_alloc in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal marked 2 inline comments as done.
BillyONeal added inline comments.



Comment at: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp:123
 
 imp2.deactivate();
 test_assign(s1, s2);

BillyONeal wrote:
> We still fail this test in debug mode because we need to allocate a new 
> _Container_proxy from imp2 in this assignment. Should this be libcxx_assert?
Hmmm this is copy assignment, not move assignment, so we still have a bug. I 
should not try writing CR comments at 2 in the morning.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60023/new/

https://reviews.llvm.org/D60023



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix inability to rebind poca_alloc in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp:123
 
 imp2.deactivate();
 test_assign(s1, s2);

We still fail this test in debug mode because we need to allocate a new 
_Container_proxy from imp2 in this assignment. Should this be libcxx_assert?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60023/new/

https://reviews.llvm.org/D60023



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D60023: [libcxx] [test] Fix inability to rebind poca_alloc in string.cons/copy_alloc.pass.cpp.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, ldionne, mclow.lists.
BillyONeal added a project: libc++.
Herald added subscribers: jdoerfert, dexonsmith.

copy_alloc.pass.cpp contains an allocator that tries to be rebindable; but if 
it actually does that it fails because the alloc_imp type is not rebindable (so 
you get failure to compile because alloc_imp* isn't convertible to 
alloc_imp*).

The fix is to make the members of alloc_impl templates instead of making 
alloc_imp itself a template.

This allows the test for MSVC++ because we need to rebind for _Container_proxy.


https://reviews.llvm.org/D60023

Files:
  test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp


Index: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
@@ -18,12 +18,12 @@
 #include "min_allocator.h"
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
-template 
 struct alloc_imp {
 bool active;
 
 alloc_imp() : active(true) {}
 
+template 
 T* allocate(std::size_t n)
 {
 if (active)
@@ -32,6 +32,7 @@
 throw std::bad_alloc();
 }
 
+template 
 void deallocate(T* p, std::size_t) { std::free(p); }
 void activate  ()  { active = true; }
 void deactivate()  { active = false; }
@@ -42,14 +43,14 @@
 typedef T value_type;
 typedef std::true_type propagate_on_container_copy_assignment;
 
-alloc_imp *imp;
+alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp_) : imp (imp_) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
 
-T*   allocate  (std::size_t n)   { return imp->allocate(n);}
+T*   allocate  (std::size_t n)   { return imp->allocate(n);}
 void deallocate(T* p, std::size_t n) { imp->deallocate(p, n); }
 };
 
@@ -112,8 +113,8 @@
 const char * p1 = "This is my first string";
 const char * p2 = "This is my second string";
 
-alloc_imp imp1;
-alloc_imp imp2;
+alloc_imp imp1;
+alloc_imp imp2;
 S s1(p1, A());
 S s2(p2, A());
 


Index: test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
===
--- test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
+++ test/std/strings/basic.string/string.cons/copy_alloc.pass.cpp
@@ -18,12 +18,12 @@
 #include "min_allocator.h"
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
-template 
 struct alloc_imp {
 bool active;
 
 alloc_imp() : active(true) {}
 
+template 
 T* allocate(std::size_t n)
 {
 if (active)
@@ -32,6 +32,7 @@
 throw std::bad_alloc();
 }
 
+template 
 void deallocate(T* p, std::size_t) { std::free(p); }
 void activate  ()  { active = true; }
 void deactivate()  { active = false; }
@@ -42,14 +43,14 @@
 typedef T value_type;
 typedef std::true_type propagate_on_container_copy_assignment;
 
-alloc_imp *imp;
+alloc_imp *imp;
 
-poca_alloc(alloc_imp *imp_) : imp (imp_) {}
+poca_alloc(alloc_imp *imp_) : imp (imp_) {}
 
 template 
 poca_alloc(const poca_alloc& other) : imp(other.imp) {}
 
-T*   allocate  (std::size_t n)   { return imp->allocate(n);}
+T*   allocate  (std::size_t n)   { return imp->allocate(n);}
 void deallocate(T* p, std::size_t n) { imp->deallocate(p, n); }
 };
 
@@ -112,8 +113,8 @@
 const char * p1 = "This is my first string";
 const char * p2 = "This is my second string";
 
-alloc_imp imp1;
-alloc_imp imp2;
+alloc_imp imp1;
+alloc_imp imp2;
 S s1(p1, A());
 S s2(p2, A());
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59621: [libcxx] [test] Add (void) casts to operator new calls, to suppress warnings generated by [[nodiscard]].

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal abandoned this revision.
BillyONeal added a comment.

This didn't work out, we'll just skip the tests for MSVC++ for now.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59621/new/

https://reviews.llvm.org/D59621



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59621: [libcxx] [test] Add (void) casts to operator new calls, to suppress warnings generated by [[nodiscard]].

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

This broke all the build bots, so I reverted the SVN change. I thought the 
nodiscard-ness was tested elsewhere but I'm not sure about interaction between 
this and -faligned-allocation.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59621/new/

https://reviews.llvm.org/D59621



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59621: [libcxx] [test] Add (void) casts to operator new calls, to suppress warnings generated by [[nodiscard]].

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

As an FYI, I committed this to subversion as r356632 since I haven't figured 
out the new git world order yet.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59621/new/

https://reviews.llvm.org/D59621



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59621: [libcxx] [test] Add (void) casts to operator new calls, to suppress warnings generated by [[nodiscard]].

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists, ldionne.
Herald added a subscriber: jdoerfert.

Add (void) casts to operator new calls, to suppress warnings generated by 
[[nodiscard]].

This allows these tests to pass when compiled by MSVC++.


https://reviews.llvm.org/D59621

Files:
  
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align_nothrow.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_nothrow.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align.sh.cpp
  
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp


Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new(4, std::align_val_t{4}, std::nothrow);  // expected-warning 
{{ignoring return value of function declared with 'nodiscard' attribute}}
+(void)::operator new(4, std::align_val_t{4}, std::nothrow);
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new(4, std::align_val_t{4});  // expected-warning {{ignoring 
return value of function declared with 'nodiscard' attribute}}
+(void)::operator new(4, std::align_val_t{4});
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_nothrow.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_nothrow.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_nothrow.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new[](4, std::nothrow);  // expected-warning {{ignoring return 
value of function declared with 'nodiscard' attribute}}
+(void)::operator new[](4, std::nothrow);
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align_nothrow.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align_nothrow.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align_nothrow.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new[](4, std::align_val_t{4}, std::nothrow);  // 
expected-warning {{ignoring return value of function declared with 'nodiscard' 
attribute}}
+(void)::operator new[](4, std::align_val_t{4}, std::nothrow);
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size_align.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new[](4, std::align_val_t{4});  // expected-warning {{ignoring 
return value of function declared with 'nodiscard' attribute}}
+(void)::operator new[](4, std::align_val_t{4});
 }
Index: 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size.sh.cpp
===
--- 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size.sh.cpp
+++ 
test/std/language.support/support.dynamic/new.delete/new.delete.array/new_size.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new[](4);  // expected-warning {{ignoring return value of 
function declared with 'nodiscard' attribute}}
+(void)::operator new[](4);
 }


Index: test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
===
--- test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
+++ test/std/language.support/support.dynamic/new.delete/new.delete.single/new_size_align_nothrow.sh.cpp
@@ -21,5 +21,5 @@
 
 int main ()
 {
-::operator new(4, std::align_val_t{4}, 

[PATCH] D53466: [libcxx] [test] Don't detect Windows' UCRT with TEST_COMPILER_C1XX

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

Thank you! :D

Committed r344829


https://reviews.llvm.org/D53466



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53468: [libcxx] [test] Don't silence MSVC STL deprecation warnings when testing _LIBCPP_ENABLE_DEPRECATION_WARNINGS

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal abandoned this revision.
BillyONeal added a comment.

Actually this doesn't work -- because we /FI this thing, this file is included 
before the test gets to say _LIBCPP_ENABLE_DEPRECATION_WARNINGS.


https://reviews.llvm.org/D53468



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53468: [libcxx] [test] Don't silence MSVC STL deprecation warnings when testing _LIBCPP_ENABLE_DEPRECATION_WARNINGS

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.

https://reviews.llvm.org/D53468

Files:
  test/support/msvc_stdlib_force_include.hpp


Index: test/support/msvc_stdlib_force_include.hpp
===
--- test/support/msvc_stdlib_force_include.hpp
+++ test/support/msvc_stdlib_force_include.hpp
@@ -72,10 +72,12 @@
 #ifndef _LIBCXX_IN_DEVCRT
 // atomic_is_lock_free.pass.cpp needs this VS 2015 Update 2 fix.
 #define _ENABLE_ATOMIC_ALIGNMENT_FIX
+#endif // _LIBCXX_IN_DEVCRT
 
+#if !defined(_LIBCXX_IN_DEVCRT) && 
!defined(_LIBCPP_ENABLE_DEPRECATION_WARNINGS)
 // Silence warnings about features that are deprecated in C++17.
 #define _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS
-#endif // _LIBCXX_IN_DEVCRT
+#endif // !defined(_LIBCXX_IN_DEVCRT) && 
!defined(_LIBCPP_ENABLE_DEPRECATION_WARNINGS)
 
 #include 
 


Index: test/support/msvc_stdlib_force_include.hpp
===
--- test/support/msvc_stdlib_force_include.hpp
+++ test/support/msvc_stdlib_force_include.hpp
@@ -72,10 +72,12 @@
 #ifndef _LIBCXX_IN_DEVCRT
 // atomic_is_lock_free.pass.cpp needs this VS 2015 Update 2 fix.
 #define _ENABLE_ATOMIC_ALIGNMENT_FIX
+#endif // _LIBCXX_IN_DEVCRT
 
+#if !defined(_LIBCXX_IN_DEVCRT) && !defined(_LIBCPP_ENABLE_DEPRECATION_WARNINGS)
 // Silence warnings about features that are deprecated in C++17.
 #define _SILENCE_ALL_CXX17_DEPRECATION_WARNINGS
-#endif // _LIBCXX_IN_DEVCRT
+#endif // !defined(_LIBCXX_IN_DEVCRT) && !defined(_LIBCPP_ENABLE_DEPRECATION_WARNINGS)
 
 #include 
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53466: [libcxx] [test] Don't detect Windows' UCRT with TEST_COMPILER_C1XX

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 170294.
BillyONeal added a comment.

Also fixed cstdlib tests.


https://reviews.llvm.org/D53466

Files:
  test/std/depr/depr.c.headers/stdlib_h.pass.cpp
  test/std/language.support/support.runtime/cstdlib.pass.cpp


Index: test/std/language.support/support.runtime/cstdlib.pass.cpp
===
--- test/std/language.support/support.runtime/cstdlib.pass.cpp
+++ test/std/language.support/support.runtime/cstdlib.pass.cpp
@@ -75,10 +75,8 @@
 static_assert((std::is_same::value), "");
 
 //  Microsoft does not implement aligned_alloc in their C library
-#ifndef TEST_COMPILER_C1XX
-#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES)
-static_assert((std::is_same::value), "");
-#endif
+#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES) && !defined(_WIN32)
+static_assert((std::is_same::value), 
"");
 #endif
 
 static_assert((std::is_same::value), 
"");
Index: test/std/depr/depr.c.headers/stdlib_h.pass.cpp
===
--- test/std/depr/depr.c.headers/stdlib_h.pass.cpp
+++ test/std/depr/depr.c.headers/stdlib_h.pass.cpp
@@ -87,10 +87,8 @@
 static_assert((std::is_same::value), "");
 
 //  Microsoft does not implement aligned_alloc in their C library
-#ifndef TEST_COMPILER_C1XX
-#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES)
+#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES) && !defined(_WIN32)
 static_assert((std::is_same::value), 
"");
-#endif
 #endif
 
 static_assert((std::is_same::value), "");


Index: test/std/language.support/support.runtime/cstdlib.pass.cpp
===
--- test/std/language.support/support.runtime/cstdlib.pass.cpp
+++ test/std/language.support/support.runtime/cstdlib.pass.cpp
@@ -75,10 +75,8 @@
 static_assert((std::is_same::value), "");
 
 //  Microsoft does not implement aligned_alloc in their C library
-#ifndef TEST_COMPILER_C1XX
-#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES)
-static_assert((std::is_same::value), "");
-#endif
+#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES) && !defined(_WIN32)
+static_assert((std::is_same::value), "");
 #endif
 
 static_assert((std::is_same::value), "");
Index: test/std/depr/depr.c.headers/stdlib_h.pass.cpp
===
--- test/std/depr/depr.c.headers/stdlib_h.pass.cpp
+++ test/std/depr/depr.c.headers/stdlib_h.pass.cpp
@@ -87,10 +87,8 @@
 static_assert((std::is_same::value), "");
 
 //  Microsoft does not implement aligned_alloc in their C library
-#ifndef TEST_COMPILER_C1XX
-#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES)
+#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES) && !defined(_WIN32)
 static_assert((std::is_same::value), "");
-#endif
 #endif
 
 static_assert((std::is_same::value), "");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53466: [libcxx] [test] Don't detect Windows' UCRT with TEST_COMPILER_C1XX

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.

The test is trying to avoid saying aligned_alloc on Windows' UCRT, which does 
not (and can not) implement aligned_alloc. However, it's testing for c1xx, 
meaning clang on Windows will fail this test when using the UCRT.


https://reviews.llvm.org/D53466

Files:
  test/std/depr/depr.c.headers/stdlib_h.pass.cpp


Index: test/std/depr/depr.c.headers/stdlib_h.pass.cpp
===
--- test/std/depr/depr.c.headers/stdlib_h.pass.cpp
+++ test/std/depr/depr.c.headers/stdlib_h.pass.cpp
@@ -87,10 +87,8 @@
 static_assert((std::is_same::value), "");
 
 //  Microsoft does not implement aligned_alloc in their C library
-#ifndef TEST_COMPILER_C1XX
-#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES)
+#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES) && !defined(_WIN32)
 static_assert((std::is_same::value), 
"");
-#endif
 #endif
 
 static_assert((std::is_same::value), "");


Index: test/std/depr/depr.c.headers/stdlib_h.pass.cpp
===
--- test/std/depr/depr.c.headers/stdlib_h.pass.cpp
+++ test/std/depr/depr.c.headers/stdlib_h.pass.cpp
@@ -87,10 +87,8 @@
 static_assert((std::is_same::value), "");
 
 //  Microsoft does not implement aligned_alloc in their C library
-#ifndef TEST_COMPILER_C1XX
-#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES)
+#if TEST_STD_VER > 14 && defined(TEST_HAS_C11_FEATURES) && !defined(_WIN32)
 static_assert((std::is_same::value), "");
-#endif
 #endif
 
 static_assert((std::is_same::value), "");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50551: [libcxx] [test] Add missing to several tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

Committed r344821


https://reviews.llvm.org/D50551



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

Committed r344820


https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 160498.
BillyONeal edited the summary of this revision.
BillyONeal added a comment.

Remove changes to detach tests.


https://reviews.llvm.org/D50549

Files:
  
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp


Index: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
===
--- 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
+++ 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
@@ -157,8 +157,11 @@
 {
 assert(G::n_alive == 0);
 assert(!G::op_run);
-std::thread t((G()));
-t.join();
+{
+G g;
+std::thread t(g);
+t.join();
+}
 assert(G::n_alive == 0);
 assert(G::op_run);
 }
@@ -185,8 +188,11 @@
 {
 assert(G::n_alive == 0);
 assert(!G::op_run);
-std::thread t(G(), 5, 5.5);
-t.join();
+{
+G g;
+std::thread t(g, 5, 5.5);
+t.join();
+}
 assert(G::n_alive == 0);
 assert(G::op_run);
 }


Index: test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
===
--- test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
+++ test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
@@ -157,8 +157,11 @@
 {
 assert(G::n_alive == 0);
 assert(!G::op_run);
-std::thread t((G()));
-t.join();
+{
+G g;
+std::thread t(g);
+t.join();
+}
 assert(G::n_alive == 0);
 assert(G::op_run);
 }
@@ -185,8 +188,11 @@
 {
 assert(G::n_alive == 0);
 assert(!G::op_run);
-std::thread t(G(), 5, 5.5);
-t.join();
+{
+G g;
+std::thread t(g, 5, 5.5);
+t.join();
+}
 assert(G::n_alive == 0);
 assert(G::op_run);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

dvyukov wrote:
> BillyONeal wrote:
> > dvyukov wrote:
> > > BillyONeal wrote:
> > > > BillyONeal wrote:
> > > > > dvyukov wrote:
> > > > > > BillyONeal wrote:
> > > > > > > BillyONeal wrote:
> > > > > > > > dvyukov wrote:
> > > > > > > > > I don't immediately see how the race on n_alive/op_run 
> > > > > > > > > happens. It seems that the modifications in the thread happen 
> > > > > > > > > before this line, and modifications in main happen after this 
> > > > > > > > > line. How can both of them modify the variables at the same 
> > > > > > > > > time?
> > > > > > > > The destruction of g here races with the destruction of the 
> > > > > > > > DECAY_COPY'd copy of G used as the parameter of operator(). 
> > > > > > > > That is, line 69 creates a copy of g, passes that to the 
> > > > > > > > started thread, the started thread calls gCopy(). gCopy() 
> > > > > > > > doesn't return until the done flag is set, but the destruction 
> > > > > > > > of the object on which op() is being called is not so 
> > > > > > > > synchronized. Most of the other thread tests avoid this problem 
> > > > > > > > by joining with the thread; joining waits for the destruction 
> > > > > > > > of the DECAY_COPY'd parameters, but this does not.
> > > > > > > > 
> > > > > > > > (This is one of the reasons detach() should basically never be 
> > > > > > > > called anywhere)
> > > > > > > > 
> > > > > > > (That is to say, there's nothing to prevent both threads from 
> > > > > > > executing G::!G() on the two different copies of g... making 
> > > > > > > op_run atomic is probably avoidable but I'm being paranoid given 
> > > > > > > that there was already thread unsafety here...)
> > > > > > What is gCopy? I don't see anything named gCopy in this file...
> > > > > > 
> > > > > > Do we care about completion of destruction? Why? We wait for done 
> > > > > > to be set, and other variables are already updated at that point. 
> > > > > > Why does it matter that "the destruction of the object on which 
> > > > > > op() is being called is not so synchronized."?
> > > > > Because the destructor does `--n_alive;`
> > > > >What is gCopy? I don't see anything named gCopy in this file...
> > > > 
> > > > The copy is made in the constructor of std::thread. std::thread makes a 
> > > > copy of all the input parameters, gives the copy to the started thread, 
> > > > and then std::invoke is called there.
> > > > 
> > > > >Why does it matter that "the destruction of the object on which op() 
> > > > >is being called is not so synchronized."?
> > > > 
> > > > Because the two dtors race on `--n_alive;` when `n_alive` is not atomic.
> > > But the first dtor runs before "while (!done) {}" line and the second 
> > > dtor runs after "while (!done) {}" line, no?
> > > Or there is third object involved? But then I don't see how joining the  
> > > thread would help either.
> > >But the first dtor runs before "while (!done) {}" line
> > 
> > No, both dtors are run after the while (!done) {} line. The first dtor runs 
> > on line 76 (when the local variable g is destroyed), and the second dtor 
> > runs after operator() returns in the constructed thread.  The constructed 
> > thread is morally doing:
> > 
> > ```
> > void threadproc(G * g) {
> > g->operator(); // setting done happens in here
> > delete g; // dtor of second copy runs here
> > }
> > ```
> > 
> > > I don't see how joining the thread would help either.
> > 
> > Joining with the thread would wait for the second dtor -- the one after 
> > op() returns -- to complete. Of course joining with the thread isn't doable 
> > here given that the point is to test thread::detach :)
> > No, both dtors are run after the while (!done) {} line.
> 
> But how do we get past while (!done) line before the desctructor in the 
> thread has finished? The destructor sets done. So after while (!done) line 
> the destructor is effectively finished. What am I missing?
>The destructor sets done.

Hmmm I thought done was getting set on 56 but that's done_ (with a trailing 
underscore). :sigh:


https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:75
 assert(G::op_run);
 assert(G::n_alive == 1);
 }

Actually I just realized that this assert is bogus too; there's no 
synchronization to ensure the copied g is destroyed here.


https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

dvyukov wrote:
> BillyONeal wrote:
> > BillyONeal wrote:
> > > dvyukov wrote:
> > > > BillyONeal wrote:
> > > > > BillyONeal wrote:
> > > > > > dvyukov wrote:
> > > > > > > I don't immediately see how the race on n_alive/op_run happens. 
> > > > > > > It seems that the modifications in the thread happen before this 
> > > > > > > line, and modifications in main happen after this line. How can 
> > > > > > > both of them modify the variables at the same time?
> > > > > > The destruction of g here races with the destruction of the 
> > > > > > DECAY_COPY'd copy of G used as the parameter of operator(). That 
> > > > > > is, line 69 creates a copy of g, passes that to the started thread, 
> > > > > > the started thread calls gCopy(). gCopy() doesn't return until the 
> > > > > > done flag is set, but the destruction of the object on which op() 
> > > > > > is being called is not so synchronized. Most of the other thread 
> > > > > > tests avoid this problem by joining with the thread; joining waits 
> > > > > > for the destruction of the DECAY_COPY'd parameters, but this does 
> > > > > > not.
> > > > > > 
> > > > > > (This is one of the reasons detach() should basically never be 
> > > > > > called anywhere)
> > > > > > 
> > > > > (That is to say, there's nothing to prevent both threads from 
> > > > > executing G::!G() on the two different copies of g... making op_run 
> > > > > atomic is probably avoidable but I'm being paranoid given that there 
> > > > > was already thread unsafety here...)
> > > > What is gCopy? I don't see anything named gCopy in this file...
> > > > 
> > > > Do we care about completion of destruction? Why? We wait for done to be 
> > > > set, and other variables are already updated at that point. Why does it 
> > > > matter that "the destruction of the object on which op() is being 
> > > > called is not so synchronized."?
> > > Because the destructor does `--n_alive;`
> > >What is gCopy? I don't see anything named gCopy in this file...
> > 
> > The copy is made in the constructor of std::thread. std::thread makes a 
> > copy of all the input parameters, gives the copy to the started thread, and 
> > then std::invoke is called there.
> > 
> > >Why does it matter that "the destruction of the object on which op() is 
> > >being called is not so synchronized."?
> > 
> > Because the two dtors race on `--n_alive;` when `n_alive` is not atomic.
> But the first dtor runs before "while (!done) {}" line and the second dtor 
> runs after "while (!done) {}" line, no?
> Or there is third object involved? But then I don't see how joining the  
> thread would help either.
>But the first dtor runs before "while (!done) {}" line

No, both dtors are run after the while (!done) {} line. The first dtor runs on 
line 76 (when the local variable g is destroyed), and the second dtor runs 
after operator() returns in the constructed thread.  The constructed thread is 
morally doing:

```
void threadproc(G * g) {
g->operator(); // setting done happens in here
delete g; // dtor of second copy runs here
}
```

> I don't see how joining the thread would help either.

Joining with the thread would wait for the second dtor -- the one after op() 
returns -- to complete. Of course joining with the thread isn't doable here 
given that the point is to test thread::detach :)


https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

BillyONeal wrote:
> dvyukov wrote:
> > BillyONeal wrote:
> > > BillyONeal wrote:
> > > > dvyukov wrote:
> > > > > I don't immediately see how the race on n_alive/op_run happens. It 
> > > > > seems that the modifications in the thread happen before this line, 
> > > > > and modifications in main happen after this line. How can both of 
> > > > > them modify the variables at the same time?
> > > > The destruction of g here races with the destruction of the 
> > > > DECAY_COPY'd copy of G used as the parameter of operator(). That is, 
> > > > line 69 creates a copy of g, passes that to the started thread, the 
> > > > started thread calls gCopy(). gCopy() doesn't return until the done 
> > > > flag is set, but the destruction of the object on which op() is being 
> > > > called is not so synchronized. Most of the other thread tests avoid 
> > > > this problem by joining with the thread; joining waits for the 
> > > > destruction of the DECAY_COPY'd parameters, but this does not.
> > > > 
> > > > (This is one of the reasons detach() should basically never be called 
> > > > anywhere)
> > > > 
> > > (That is to say, there's nothing to prevent both threads from executing 
> > > G::!G() on the two different copies of g... making op_run atomic is 
> > > probably avoidable but I'm being paranoid given that there was already 
> > > thread unsafety here...)
> > What is gCopy? I don't see anything named gCopy in this file...
> > 
> > Do we care about completion of destruction? Why? We wait for done to be 
> > set, and other variables are already updated at that point. Why does it 
> > matter that "the destruction of the object on which op() is being called is 
> > not so synchronized."?
> Because the destructor does `--n_alive;`
>What is gCopy? I don't see anything named gCopy in this file...

The copy is made in the constructor of std::thread. std::thread makes a copy of 
all the input parameters, gives the copy to the started thread, and then 
std::invoke is called there.

>Why does it matter that "the destruction of the object on which op() is being 
>called is not so synchronized."?

Because the two dtors race on `--n_alive;` when `n_alive` is not atomic.


https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

dvyukov wrote:
> BillyONeal wrote:
> > BillyONeal wrote:
> > > dvyukov wrote:
> > > > I don't immediately see how the race on n_alive/op_run happens. It 
> > > > seems that the modifications in the thread happen before this line, and 
> > > > modifications in main happen after this line. How can both of them 
> > > > modify the variables at the same time?
> > > The destruction of g here races with the destruction of the DECAY_COPY'd 
> > > copy of G used as the parameter of operator(). That is, line 69 creates a 
> > > copy of g, passes that to the started thread, the started thread calls 
> > > gCopy(). gCopy() doesn't return until the done flag is set, but the 
> > > destruction of the object on which op() is being called is not so 
> > > synchronized. Most of the other thread tests avoid this problem by 
> > > joining with the thread; joining waits for the destruction of the 
> > > DECAY_COPY'd parameters, but this does not.
> > > 
> > > (This is one of the reasons detach() should basically never be called 
> > > anywhere)
> > > 
> > (That is to say, there's nothing to prevent both threads from executing 
> > G::!G() on the two different copies of g... making op_run atomic is 
> > probably avoidable but I'm being paranoid given that there was already 
> > thread unsafety here...)
> What is gCopy? I don't see anything named gCopy in this file...
> 
> Do we care about completion of destruction? Why? We wait for done to be set, 
> and other variables are already updated at that point. Why does it matter 
> that "the destruction of the object on which op() is being called is not so 
> synchronized."?
Because the destructor does `--n_alive;`


https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

BillyONeal wrote:
> dvyukov wrote:
> > I don't immediately see how the race on n_alive/op_run happens. It seems 
> > that the modifications in the thread happen before this line, and 
> > modifications in main happen after this line. How can both of them modify 
> > the variables at the same time?
> The destruction of g here races with the destruction of the DECAY_COPY'd copy 
> of G used as the parameter of operator(). That is, line 69 creates a copy of 
> g, passes that to the started thread, the started thread calls gCopy(). 
> gCopy() doesn't return until the done flag is set, but the destruction of the 
> object on which op() is being called is not so synchronized. Most of the 
> other thread tests avoid this problem by joining with the thread; joining 
> waits for the destruction of the DECAY_COPY'd parameters, but this does not.
> 
> (This is one of the reasons detach() should basically never be called 
> anywhere)
> 
(That is to say, there's nothing to prevent both threads from executing G::!G() 
on the two different copies of g... making op_run atomic is probably avoidable 
but I'm being paranoid given that there was already thread unsafety here...)


https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp:73
 assert(!t0.joinable());
 while (!done) {}
 assert(G::op_run);

dvyukov wrote:
> I don't immediately see how the race on n_alive/op_run happens. It seems that 
> the modifications in the thread happen before this line, and modifications in 
> main happen after this line. How can both of them modify the variables at the 
> same time?
The destruction of g here races with the destruction of the DECAY_COPY'd copy 
of G used as the parameter of operator(). That is, line 69 creates a copy of g, 
passes that to the started thread, the started thread calls gCopy(). gCopy() 
doesn't return until the done flag is set, but the destruction of the object on 
which op() is being called is not so synchronized. Most of the other thread 
tests avoid this problem by joining with the thread; joining waits for the 
destruction of the DECAY_COPY'd parameters, but this does not.

(This is one of the reasons detach() should basically never be called anywhere)



https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In https://reviews.llvm.org/D50549#1194852, @EricWF wrote:

> We should add some TSAN folks to this review, since I think these are also 
> TSAN false negatives; perhaps correctly, perhaps not.


Do you know who that is / can you add them?


https://reviews.llvm.org/D50549



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50551: [libcxx] [test] Add missing to several tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF.

https://reviews.llvm.org/D50551

Files:
  test/std/containers/associative/map/map.access/at.pass.cpp
  test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp


Index: test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp
===
--- test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp
+++ test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp
@@ -16,9 +16,10 @@
 // mapped_type&   at(const key_type& k);
 // const mapped_type& at(const key_type& k) const;
 
-#include 
-#include 
 #include 
+#include 
+#include 
+#include 
 
 #include "MoveOnly.h"
 #include "min_allocator.h"
Index: test/std/containers/associative/map/map.access/at.pass.cpp
===
--- test/std/containers/associative/map/map.access/at.pass.cpp
+++ test/std/containers/associative/map/map.access/at.pass.cpp
@@ -14,8 +14,9 @@
 //   mapped_type& at(const key_type& k);
 // const mapped_type& at(const key_type& k) const;
 
-#include 
 #include 
+#include 
+#include 
 
 #include "min_allocator.h"
 #include "test_macros.h"


Index: test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp
===
--- test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp
+++ test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp
@@ -16,9 +16,10 @@
 // mapped_type&   at(const key_type& k);
 // const mapped_type& at(const key_type& k) const;
 
-#include 
-#include 
 #include 
+#include 
+#include 
+#include 
 
 #include "MoveOnly.h"
 #include "min_allocator.h"
Index: test/std/containers/associative/map/map.access/at.pass.cpp
===
--- test/std/containers/associative/map/map.access/at.pass.cpp
+++ test/std/containers/associative/map/map.access/at.pass.cpp
@@ -14,8 +14,9 @@
 //   mapped_type& at(const key_type& k);
 // const mapped_type& at(const key_type& k) const;
 
-#include 
 #include 
+#include 
+#include 
 
 #include "min_allocator.h"
 #include "test_macros.h"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50549: [libcxx] [test] Repair thread unsafety in thread tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.
Herald added a subscriber: jfb.

- Repair thread-unsafe modifications of n_alive in F.pass.cpp

In this example, the ctor of G runs in the main thread in the expression G(), 
and also in the copy ctor of G() in the DECAY_COPY inside std::thread. The main 
thread destroys the G() instance at the semicolon, and the started thread 
destroys the G() after it returns. Thus there is a race between the threads on 
the n_alive variable.

The fix is to join with the background thread before attempting to destroy the 
G in the main thread.

- Repair a race in detach.pass.cpp by adding atomic.

This test thinks that setting done = true as the last line in the thread 
procedure is equivalent to joining with it, but that is not true. There is a 
race between destroying the G instance in main and destroying the *parameter* 
of the thread procedure. This problem is avoided by making n_alive and op_run 
atomics.


https://reviews.llvm.org/D50549

Files:
  
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
  
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp


Index: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
===
--- 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
+++ 
test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
@@ -29,8 +29,8 @@
 int alive_;
 bool done_;
 public:
-static int n_alive;
-static bool op_run;
+static std::atomic_int n_alive;
+static std::atomic_bool op_run;
 
 G() : alive_(1), done_(false)
 {
@@ -57,8 +57,8 @@
 }
 };
 
-int G::n_alive = 0;
-bool G::op_run = false;
+std::atomic_int G::n_alive = 0;
+std::atomic_bool G::op_run = false;
 
 void foo() {}
 
Index: 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
===
--- 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
+++ 
test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
@@ -157,8 +157,11 @@
 {
 assert(G::n_alive == 0);
 assert(!G::op_run);
-std::thread t((G()));
-t.join();
+{
+G g;
+std::thread t(g);
+t.join();
+}
 assert(G::n_alive == 0);
 assert(G::op_run);
 }
@@ -185,8 +188,11 @@
 {
 assert(G::n_alive == 0);
 assert(!G::op_run);
-std::thread t(G(), 5, 5.5);
-t.join();
+{
+G g;
+std::thread t(g, 5, 5.5);
+t.join();
+}
 assert(G::n_alive == 0);
 assert(G::op_run);
 }


Index: test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
===
--- test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
+++ test/std/thread/thread.threads/thread.thread.class/thread.thread.member/detach.pass.cpp
@@ -29,8 +29,8 @@
 int alive_;
 bool done_;
 public:
-static int n_alive;
-static bool op_run;
+static std::atomic_int n_alive;
+static std::atomic_bool op_run;
 
 G() : alive_(1), done_(false)
 {
@@ -57,8 +57,8 @@
 }
 };
 
-int G::n_alive = 0;
-bool G::op_run = false;
+std::atomic_int G::n_alive = 0;
+std::atomic_bool G::op_run = false;
 
 void foo() {}
 
Index: test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
===
--- test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
+++ test/std/thread/thread.threads/thread.thread.class/thread.thread.constr/F.pass.cpp
@@ -157,8 +157,11 @@
 {
 assert(G::n_alive == 0);
 assert(!G::op_run);
-std::thread t((G()));
-t.join();
+{
+G g;
+std::thread t(g);
+t.join();
+}
 assert(G::n_alive == 0);
 assert(G::op_run);
 }
@@ -185,8 +188,11 @@
 {
 assert(G::n_alive == 0);
 assert(!G::op_run);
-std::thread t(G(), 5, 5.5);
-t.join();
+{
+G g;
+std::thread t(g, 5, 5.5);
+t.join();
+}
 assert(G::n_alive == 0);
 assert(G::op_run);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47400: [libcxx] [test] Allow a standard library that implements LWG 1203 in istream.rvalue/rvalue.pass.cpp

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a reviewer: ldionne.
BillyONeal added a comment.

Adding ldionne as suggested by Eric.


https://reviews.llvm.org/D47400



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a reviewer: ldionne.
BillyONeal added a comment.

Adding ldionne as suggested by Eric.


https://reviews.llvm.org/D47395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47400: [libcxx] [test] Allow a standard library that implements LWG 1203 in istream.rvalue/rvalue.pass.cpp

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

Committed r339214

(If there are changes requested I'll do them in a follow up commit)


https://reviews.llvm.org/D47400



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

Committed r339213

(If there are changes requested I'll do them in a follow up commit)


https://reviews.llvm.org/D47395



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal marked 2 inline comments as done.
BillyONeal added a comment.

Committed r339212


https://reviews.llvm.org/D50421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 159632.
BillyONeal added a comment.

Remove comma.


https://reviews.llvm.org/D50421

Files:
  test/std/utilities/template.bitset/includes.pass.cpp


Index: test/std/utilities/template.bitset/includes.pass.cpp
===
--- test/std/utilities/template.bitset/includes.pass.cpp
+++ test/std/utilities/template.bitset/includes.pass.cpp
@@ -7,26 +7,17 @@
 //
 
//===--===//
 
-// test that  includes , ,  and 
+// test that  includes  and 
 
 #include 
 
 template  void test_typedef() {}
 
 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();


Index: test/std/utilities/template.bitset/includes.pass.cpp
===
--- test/std/utilities/template.bitset/includes.pass.cpp
+++ test/std/utilities/template.bitset/includes.pass.cpp
@@ -7,26 +7,17 @@
 //
 //===--===//
 
-// test that  includes , ,  and 
+// test that  includes  and 
 
 #include 
 
 template  void test_typedef() {}
 
 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50420: [libcxx] [test] Add missing to several tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

Committed r339209


https://reviews.llvm.org/D50420



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: test/std/utilities/template.bitset/includes.pass.cpp:11
+// test that  includes , and 

 #include 
 template  void test_typedef() {}

Quuxplusone wrote:
> grammar nit: comma
NP: Oxford Comma by Vampire Weekend


https://reviews.llvm.org/D50421



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50421: [libcxx] [test] Remove assertion that includes and .

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF.

[template.bitset] says only that  and  are included by 
, but libcxx tests are asserting that  and  are 
also included.

Note that libcxx's nonstandard assertion of this is already handled by 
test/libcxx/utilities/template.bitset/includes.pass.cpp


https://reviews.llvm.org/D50421

Files:
  test/std/utilities/template.bitset/includes.pass.cpp


Index: test/std/utilities/template.bitset/includes.pass.cpp
===
--- test/std/utilities/template.bitset/includes.pass.cpp
+++ test/std/utilities/template.bitset/includes.pass.cpp
@@ -1,32 +1,24 @@
+
 
//===--===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 
//===--===//

-// test that  includes , ,  and 
+// test that  includes , and 

 #include 

 template  void test_typedef() {}

 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();


Index: test/std/utilities/template.bitset/includes.pass.cpp
===
--- test/std/utilities/template.bitset/includes.pass.cpp
+++ test/std/utilities/template.bitset/includes.pass.cpp
@@ -1,32 +1,24 @@
+
 //===--===//
 //
 // The LLVM Compiler Infrastructure
 //
 // This file is dual licensed under the MIT and the University of Illinois Open
 // Source Licenses. See LICENSE.TXT for details.
 //
 //===--===//

-// test that  includes , ,  and 
+// test that  includes , and 

 #include 

 template  void test_typedef() {}

 int main()
 {
-  { // test for 
-std::ptrdiff_t p; ((void)p);
-std::size_t s; ((void)s);
-std::nullptr_t np; ((void)np);
-  }
   { // test for 
 std::string s; ((void)s);
   }
-  { // test for 
-std::logic_error le("blah"); ((void)le);
-std::runtime_error re("blah"); ((void)re);
-  }
   { // test for 
 test_typedef();
 test_typedef();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50420: [libcxx] [test] Add missing to several tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.

https://reviews.llvm.org/D50420

Files:
  test/std/strings/basic.string/string.capacity/over_max_size.pass.cpp
  test/std/strings/string.conversions/stod.pass.cpp
  test/std/strings/string.conversions/stof.pass.cpp
  test/std/strings/string.conversions/stoi.pass.cpp
  test/std/strings/string.conversions/stol.pass.cpp
  test/std/strings/string.conversions/stoll.pass.cpp
  test/std/strings/string.conversions/stoul.pass.cpp
  test/std/strings/string.conversions/stoull.pass.cpp
  test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp
  test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp
  
test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
  
test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
  test/std/strings/string.view/string.view.ops/copy.pass.cpp
  test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp
  test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp
  test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
  test/std/utilities/template.bitset/bitset.members/test.pass.cpp

Index: test/std/utilities/template.bitset/bitset.members/test.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/test.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/test.pass.cpp
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/set_one.pass.cpp
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/reset_one.pass.cpp
@@ -11,6 +11,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp
===
--- test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp
+++ test/std/utilities/template.bitset/bitset.members/flip_one.pass.cpp
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/strings/string.view/string.view.ops/copy.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/copy.pass.cpp
+++ test/std/strings/string.view/string.view.ops/copy.pass.cpp
@@ -21,6 +21,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 
Index: test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
+++ test/std/strings/string.view/string.view.ops/compare.size_size_sv_size_size.pass.cpp
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 #include "constexpr_char_traits.hpp"
Index: test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
+++ test/std/strings/string.view/string.view.ops/compare.size_size_sv_pointer_size.pass.cpp
@@ -14,6 +14,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 #include "constexpr_char_traits.hpp"
Index: test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp
+++ test/std/strings/string.view/string.view.ops/compare.size_size_sv.pass.cpp
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 #include "constexpr_char_traits.hpp"
Index: test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp
===
--- test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp
+++ test/std/strings/string.view/string.view.ops/compare.pointer_size.pass.cpp
@@ -13,6 +13,7 @@
 
 #include 
 #include 
+#include 
 
 #include "test_macros.h"
 #include "constexpr_char_traits.hpp"
Index: test/std/strings/string.conversions/stoull.pass.cpp
===
--- 

[PATCH] D49774: [libc++] Use __int128_t to represent file_time_type.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In https://reviews.llvm.org/D49774#1175650, @ldionne wrote:

> In https://reviews.llvm.org/D49774#1175543, @BillyONeal wrote:
>
> > In https://reviews.llvm.org/D49774#1175131, @mclow.lists wrote:
> >
> > > Another problem (that Eric and I discussed last night) is that filesystem 
> > > is part of C++17, and `file_clock` is C++20. So we need a solution for 
> > > C++17 as well.
> >
> >
> > It seems like we need to fix C++20 to allow that to be a typedef to a type 
> > in std::filesystem or that will be ABI breaking for MSVC++. IMO we should 
> > fix the spec to allow that rather than make libc++ jump through insane 
> > hoops.
>
>
> We could also just provide `file_clock` "early" in C++17. Strictly speaking 
> that may make our implementation non-conforming, but IDK how big of a deal 
> this would be?


Sure, libc++ could do that. But because msvc++ and libstdc++ have already 
shipped this thing I think the spec will have to change.

No real user cares about the associated namespaces of a clock anyway.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49774



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49774: [libc++] Use __int128_t to represent file_time_type.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In https://reviews.llvm.org/D49774#1175131, @mclow.lists wrote:

> Another problem (that Eric and I discussed last night) is that filesystem is 
> part of C++17, and `file_clock` is C++20. So we need a solution for C++17 as 
> well.


It seems like we need to fix C++20 to allow that to be a typedef to a type in 
std::filesystem or that will be ABI breaking for MSVC++. IMO we should fix the 
spec to allow that rather than make libc++ jump through insane hoops.


Repository:
  rCXX libc++

https://reviews.llvm.org/D49774



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47400: [libcxx] [test] Allow a standard library that implements LWG 1203 in istream.rvalue/rvalue.pass.cpp

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.

If a standard library implements LWG 1203 (which I am implementing in MSVC++ 
right now) the return value of (istringstream() << A{}) is an rvalue, so it 
can't have an lvalue reference bound to it. Change this test to bind a 
forwarding reference instead which allows both standard behavior and LWG 1203 
proposed behavior.

Note that LWG 1203 is presently not accepted.


https://reviews.llvm.org/D47400

Files:
  
test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp


Index: 
test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
===
--- 
test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
+++ 
test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
@@ -65,7 +65,7 @@
 { // test perfect forwarding
 assert(called == false);
 std::istringstream ss;
-auto& out = (std::move(ss) >> A{});
+auto&& out = (std::move(ss) >> A{});
 assert( == );
 assert(called);
 }


Index: test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
===
--- test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
+++ test/std/input.output/iostream.format/input.streams/istream.rvalue/rvalue.pass.cpp
@@ -65,7 +65,7 @@
 { // test perfect forwarding
 assert(called == false);
 std::istringstream ss;
-auto& out = (std::move(ss) >> A{});
+auto&& out = (std::move(ss) >> A{});
 assert( == );
 assert(called);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47395: [libcxx] [test] Remove nonportable locale assumption in basic.ios.members/narrow.pass.cpp

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: EricWF, mclow.lists.

I'm not sure if libcxx is asserting UTF-8 here; but on Windows the full char 
value is always passed through in its entirety, since the default codepage is 
something like Windows-1252. The replacement character is only used for 
non-chars there; and that should be a more portable test everywhere.


https://reviews.llvm.org/D47395

Files:
  test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp


Index: 
test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
===
--- test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
+++ test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
@@ -18,7 +18,7 @@
 
 int main()
 {
-const std::ios ios(0);
-assert(ios.narrow('c', '*') == 'c');
-assert(ios.narrow('\xFE', '*') == '*');
+const std::wios ios(0);
+assert(ios.narrow(L'c', '*') == 'c');
+assert(ios.narrow(L'\u203C', '*') == '*');
 }


Index: test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
===
--- test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
+++ test/std/input.output/iostreams.base/ios/basic.ios.members/narrow.pass.cpp
@@ -18,7 +18,7 @@
 
 int main()
 {
-const std::ios ios(0);
-assert(ios.narrow('c', '*') == 'c');
-assert(ios.narrow('\xFE', '*') == '*');
+const std::wios ios(0);
+assert(ios.narrow(L'c', '*') == 'c');
+assert(ios.narrow(L'\u203C', '*') == '*');
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D34331: func.wrap.func.con: Unset function before destroying anything

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added subscribers: STL_MSFT, BillyONeal.
BillyONeal added a comment.

@mclow.lists
@STL_MSFT

Why did tests for this this go into std? [reentrancy]/1 says this isn't 
required to work. Moreover, assignments in the dtor like this *can't* work in 
the general case because they would try to overwrite the SSO space. e.g. what 
do you expect this to do?

  std::function global;
  
  struct B {
  int data = 1729;
  void operator() {}
  };
  
  struct A {
  int data = 42;
  ~A() {
  global = std::function(B{}); // whoops, constructs a B on top 
of A if Small Functor Optimization engages
  assert(data == 42);
  }
  
  void operator() {}
  };
  
  int main() {
  global = std::function(A{});
  global = nullptr;
  }




Repository:
  rCXX libc++

https://reviews.llvm.org/D34331



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45294: [libcxx] [test] Remove non-portable assertions from filebuf tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

D:\msvc\src\qa\VC\Libs\libcxx\upstream>git svn dcommit
Committing to https://llvm.org/svn/llvm-project/libcxx/trunk ...
Use of uninitialized value $rec in scalar chomp at /mingw64/share/perl5/Git.pm 
line 557, <$fh> chunk 4.
Authentication realm: https://llvm.org:443 LLVM Subversion repository
Password for 'bion':

  M   
test/std/input.output/file.streams/fstreams/filebuf.virtuals/pbackfail.pass.cpp
  M   
test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp

Committed r330999

  M   
test/std/input.output/file.streams/fstreams/filebuf.virtuals/pbackfail.pass.cpp
  M   
test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp

r330999 = dc3bb72e3c9a47ef62d6288f73d164d3ba5fe941 (refs/remotes/llvm/master)
No changes between 706a727f5b068a7b5c6f43028cdbaf6ea610abe6 and 
refs/remotes/llvm/master
Resetting to the latest refs/remotes/llvm/master

D:\msvc\src\qa\VC\Libs\libcxx\upstream>


https://reviews.llvm.org/D45294



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45294: [libcxx] [test] Remove non-portable assertions from filebuf tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

@mclow.lists Are you OK with this one? It's conceptually similar to the 
previous review.


https://reviews.llvm.org/D45294



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45715: [libcxx] [test] Remove nonportable that errc::is_a_directory produces "Is a directory" from ios_base::failure tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

D:\msvc\src\qa\VC\Libs\libcxx\upstream>git svn dcommit
Committing to https://llvm.org/svn/llvm-project/libcxx/trunk ...
Authentication realm: https://llvm.org:443 LLVM Subversion repository
Password for 'bion':

  M   
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
  M   
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp

Committed r330791

  M   
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
  M   
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp

r330791 = efac6e5df9109748822bd3b5b533b35257a110f4 (refs/remotes/llvm/master)
No changes between e69aef2e1f9cb3ad085a9ddcc2f432f8f7c8c182 and 
refs/remotes/llvm/master
Resetting to the latest refs/remotes/llvm/master


https://reviews.llvm.org/D45715



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45715: [libcxx] [test] Remove nonportable that errc::is_a_directory produces "Is a directory" from ios_base::failure tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In https://reviews.llvm.org/D45715#1075665, @mclow.lists wrote:

> - if there are similar tests in the filesystem test suite, and you haven't 
> tripped over them because you spelled your error message the same as we did.


Almost certainly. But we aren't running your filesystem tests yet because they 
are still under experimental. There are probably a lot of POSIX assumptions 
everywhere in there given the root_name parsing business.

In https://reviews.llvm.org/D45715#1075665, @mclow.lists wrote:

> - Should part of this test be moved into test/libcxx


That one's up to you. I don't think the specific message matters to this test; 
clearly the existing asserts for `io_errc::stream` don't care about a specific 
message (which is why they worked on our implementation).


https://reviews.llvm.org/D45715



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45715: [libcxx] [test] Remove nonportable that errc::is_a_directory produces "Is a directory" from ios_base::failure tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 142733.
BillyONeal added a comment.

Added missing  include in one of the tests.


https://reviews.llvm.org/D45715

Files:
  
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
  
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp


Index: 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
===
--- 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
+++ 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
@@ -15,6 +15,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 int main()
@@ -28,7 +29,8 @@
 assert(se.code() == std::make_error_code(std::errc::is_a_directory));
 std::string what_message(se.what());
 assert(what_message.find(what_arg) != std::string::npos);
-assert(what_message.find("Is a directory") != std::string::npos);
+
assert(what_message.find(std::generic_category().message(static_cast
+(std::errc::is_a_directory))) != std::string::npos);
 }
 {
 std::string what_arg("io test message");
Index: 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
===
--- 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
+++ 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
@@ -15,6 +15,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 int main()
@@ -25,7 +26,8 @@
 assert(se.code() == std::make_error_code(std::errc::is_a_directory));
 std::string what_message(se.what());
 assert(what_message.find(what_arg) != std::string::npos);
-assert(what_message.find("Is a directory") != std::string::npos);
+
assert(what_message.find(std::generic_category().message(static_cast
+(std::errc::is_a_directory))) != std::string::npos);
 }
 {
 std::string what_arg("io test message");


Index: test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
===
--- test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
+++ test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
@@ -15,6 +15,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 int main()
@@ -28,7 +29,8 @@
 assert(se.code() == std::make_error_code(std::errc::is_a_directory));
 std::string what_message(se.what());
 assert(what_message.find(what_arg) != std::string::npos);
-assert(what_message.find("Is a directory") != std::string::npos);
+assert(what_message.find(std::generic_category().message(static_cast
+(std::errc::is_a_directory))) != std::string::npos);
 }
 {
 std::string what_arg("io test message");
Index: test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
===
--- test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
+++ test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
@@ -15,6 +15,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 int main()
@@ -25,7 +26,8 @@
 assert(se.code() == std::make_error_code(std::errc::is_a_directory));
 std::string what_message(se.what());
 assert(what_message.find(what_arg) != std::string::npos);
-assert(what_message.find("Is a directory") != std::string::npos);
+assert(what_message.find(std::generic_category().message(static_cast
+(std::errc::is_a_directory))) != std::string::npos);
 }
 {
 std::string what_arg("io test message");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45715: [libcxx] [test] Remove nonportable that errc::is_a_directory produces "Is a directory" from ios_base::failure tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF.

Remove nonportable assumption in io_error tests that std::errc::is_a_directory 
has message "Is a directory". On MSVC++ it reports "is a directory" (with a 
lowercase I).


https://reviews.llvm.org/D45715

Files:
  
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
  
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp


Index: 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
===
--- 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
+++ 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
@@ -28,7 +28,8 @@
 assert(se.code() == std::make_error_code(std::errc::is_a_directory));
 std::string what_message(se.what());
 assert(what_message.find(what_arg) != std::string::npos);
-assert(what_message.find("Is a directory") != std::string::npos);
+
assert(what_message.find(std::generic_category().message(static_cast
+(std::errc::is_a_directory))) != std::string::npos);
 }
 {
 std::string what_arg("io test message");
Index: 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
===
--- 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
+++ 
test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
@@ -15,6 +15,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 int main()
@@ -25,7 +26,8 @@
 assert(se.code() == std::make_error_code(std::errc::is_a_directory));
 std::string what_message(se.what());
 assert(what_message.find(what_arg) != std::string::npos);
-assert(what_message.find("Is a directory") != std::string::npos);
+
assert(what_message.find(std::generic_category().message(static_cast
+(std::errc::is_a_directory))) != std::string::npos);
 }
 {
 std::string what_arg("io test message");


Index: test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
===
--- test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
+++ test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_string_error_code.pass.cpp
@@ -28,7 +28,8 @@
 assert(se.code() == std::make_error_code(std::errc::is_a_directory));
 std::string what_message(se.what());
 assert(what_message.find(what_arg) != std::string::npos);
-assert(what_message.find("Is a directory") != std::string::npos);
+assert(what_message.find(std::generic_category().message(static_cast
+(std::errc::is_a_directory))) != std::string::npos);
 }
 {
 std::string what_arg("io test message");
Index: test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
===
--- test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
+++ test/std/input.output/iostreams.base/ios.base/ios.types/ios_failure/ctor_char_pointer_error_code.pass.cpp
@@ -15,6 +15,7 @@
 
 #include 
 #include 
+#include 
 #include 
 
 int main()
@@ -25,7 +26,8 @@
 assert(se.code() == std::make_error_code(std::errc::is_a_directory));
 std::string what_message(se.what());
 assert(what_message.find(what_arg) != std::string::npos);
-assert(what_message.find("Is a directory") != std::string::npos);
+assert(what_message.find(std::generic_category().message(static_cast
+(std::errc::is_a_directory))) != std::string::npos);
 }
 {
 std::string what_arg("io test message");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41748: [libcxx] [test] Fix Xxx_scan tests using nonstandard things and MSVC++ warnings

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal accepted this revision.
BillyONeal added a comment.
This revision is now accepted and ready to land.

It actually looks like I accidentally committed this back in January so I'm 
going to close this :)


https://reviews.llvm.org/D41748



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45294: [libcxx] [test] Remove non-portable assertions from filebuf tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 141433.
BillyONeal retitled this revision from "[libcxx] [test] Remove non-portable 
assertions from filebuf's seekoff.pass.cpp test" to "[libcxx] [test] Remove 
non-portable assertions from filebuf tests".
BillyONeal edited the summary of this revision.
BillyONeal added a comment.

Added pbackfail.pass.cpp changes.


https://reviews.llvm.org/D45294

Files:
  
test/std/input.output/file.streams/fstreams/filebuf.virtuals/pbackfail.pass.cpp
  test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp


Index: 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
===
--- 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
+++ 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
@@ -14,11 +14,11 @@
 // pos_type seekpos(pos_type sp,
 //  ios_base::openmode which = ios_base::in | ios_base::out);
 
-// This test is not entirely portable
-
 #include 
 #include 
 
+#include "test_macros.h"
+
 int main()
 {
 {
@@ -30,7 +30,7 @@
| std::ios_base::trunc) 
!= 0);
 assert(f.is_open());
 f.sputn("abcdefghijklmnopqrstuvwxyz", 26);
-assert(buf[0] == 'v');
+LIBCPP_ASSERT(buf[0] == 'v');
 pos_type p = f.pubseekoff(-15, std::ios_base::cur);
 assert(p == 11);
 assert(f.sgetc() == 'l');
@@ -51,7 +51,7 @@
| std::ios_base::trunc) 
!= 0);
 assert(f.is_open());
 f.sputn(L"abcdefghijklmnopqrstuvwxyz", 26);
-assert(buf[0] == L'v');
+LIBCPP_ASSERT(buf[0] == L'v');
 pos_type p = f.pubseekoff(-15, std::ios_base::cur);
 assert(p == 11);
 assert(f.sgetc() == L'l');
Index: 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/pbackfail.pass.cpp
===
--- 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/pbackfail.pass.cpp
+++ 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/pbackfail.pass.cpp
@@ -11,11 +11,11 @@
 
 // int_type pbackfail(int_type c = traits::eof());
 
-// This test is not entirely portable
-
 #include 
 #include 
 
+#include "test_macros.h"
+
 template 
 struct test_buf
 : public std::basic_filebuf
@@ -41,16 +41,24 @@
 assert(f.is_open());
 assert(f.sbumpc() == '1');
 assert(f.sgetc() == '2');
-assert(f.pbackfail('a') == -1);
+typename test_buf::int_type pbackResult = f.pbackfail('a');
+LIBCPP_ASSERT(pbackResult == -1);
+if (pbackResult != -1) {
+assert(f.sbumpc() == 'a');
+assert(f.sgetc() == '2');
+}
 }
 {
 test_buf f;
 assert(f.open("underflow.dat", std::ios_base::in | std::ios_base::out) 
!= 0);
 assert(f.is_open());
 assert(f.sbumpc() == '1');
 assert(f.sgetc() == '2');
-assert(f.pbackfail('a') == 'a');
-assert(f.sbumpc() == 'a');
-assert(f.sgetc() == '2');
+typename test_buf::int_type pbackResult = f.pbackfail('a');
+LIBCPP_ASSERT(pbackResult == 'a');
+if (pbackResult != -1) {
+assert(f.sbumpc() == 'a');
+assert(f.sgetc() == '2');
+}
 }
 }


Index: test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
===
--- test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
+++ test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
@@ -14,11 +14,11 @@
 // pos_type seekpos(pos_type sp,
 //  ios_base::openmode which = ios_base::in | ios_base::out);
 
-// This test is not entirely portable
-
 #include 
 #include 
 
+#include "test_macros.h"
+
 int main()
 {
 {
@@ -30,7 +30,7 @@
| std::ios_base::trunc) != 0);
 assert(f.is_open());
 f.sputn("abcdefghijklmnopqrstuvwxyz", 26);
-assert(buf[0] == 'v');
+LIBCPP_ASSERT(buf[0] == 'v');
 pos_type p = f.pubseekoff(-15, std::ios_base::cur);
 assert(p == 11);
 assert(f.sgetc() == 'l');
@@ -51,7 +51,7 @@
| std::ios_base::trunc) != 0);
 assert(f.is_open());
 f.sputn(L"abcdefghijklmnopqrstuvwxyz", 26);
-assert(buf[0] == L'v');
+LIBCPP_ASSERT(buf[0] == L'v');
 pos_type p = f.pubseekoff(-15, std::ios_base::cur);
 assert(p == 11);
 assert(f.sgetc() == L'l');
Index: test/std/input.output/file.streams/fstreams/filebuf.virtuals/pbackfail.pass.cpp
===
--- 

[PATCH] D41748: [libcxx] [test] Fix Xxx_scan tests using nonstandard things and MSVC++ warnings

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

@EricWF / @mclow.lists Any chance you could take a look sometime soon?


https://reviews.llvm.org/D41748



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45294: [libcxx] [test] Remove non-portable assertions from filebuf's seekoff.pass.cpp test

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF.

libc++'s tests are asserting things about the buffer passed to pubsetbuf. 
[filebuf.virtuals]/12 says that what the filebuf does with the buffer you give 
it is completely implementation defined. The MSVC++ implementation takes that 
buffer and hands it off to the CRT (by calling ::setvbuf) and the CRT doesn't 
necessarily follow the pattern this test wants.

This change simply makes asserts against the buffer's contents use 
LIBCPP_ASSERT instead of assert.


https://reviews.llvm.org/D45294

Files:
  test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp


Index: 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
===
--- 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
+++ 
test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
@@ -14,11 +14,11 @@
 // pos_type seekpos(pos_type sp,
 //  ios_base::openmode which = ios_base::in | ios_base::out);
 
-// This test is not entirely portable
-
 #include 
 #include 
 
+#include "test_macros.h"
+
 int main()
 {
 {
@@ -30,7 +30,7 @@
| std::ios_base::trunc) 
!= 0);
 assert(f.is_open());
 f.sputn("abcdefghijklmnopqrstuvwxyz", 26);
-assert(buf[0] == 'v');
+LIBCPP_ASSERT(buf[0] == 'v');
 pos_type p = f.pubseekoff(-15, std::ios_base::cur);
 assert(p == 11);
 assert(f.sgetc() == 'l');
@@ -51,7 +51,7 @@
| std::ios_base::trunc) 
!= 0);
 assert(f.is_open());
 f.sputn(L"abcdefghijklmnopqrstuvwxyz", 26);
-assert(buf[0] == L'v');
+LIBCPP_ASSERT(buf[0] == L'v');
 pos_type p = f.pubseekoff(-15, std::ios_base::cur);
 assert(p == 11);
 assert(f.sgetc() == L'l');


Index: test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
===
--- test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
+++ test/std/input.output/file.streams/fstreams/filebuf.virtuals/seekoff.pass.cpp
@@ -14,11 +14,11 @@
 // pos_type seekpos(pos_type sp,
 //  ios_base::openmode which = ios_base::in | ios_base::out);
 
-// This test is not entirely portable
-
 #include 
 #include 
 
+#include "test_macros.h"
+
 int main()
 {
 {
@@ -30,7 +30,7 @@
| std::ios_base::trunc) != 0);
 assert(f.is_open());
 f.sputn("abcdefghijklmnopqrstuvwxyz", 26);
-assert(buf[0] == 'v');
+LIBCPP_ASSERT(buf[0] == 'v');
 pos_type p = f.pubseekoff(-15, std::ios_base::cur);
 assert(p == 11);
 assert(f.sgetc() == 'l');
@@ -51,7 +51,7 @@
| std::ios_base::trunc) != 0);
 assert(f.is_open());
 f.sputn(L"abcdefghijklmnopqrstuvwxyz", 26);
-assert(buf[0] == L'v');
+LIBCPP_ASSERT(buf[0] == L'v');
 pos_type p = f.pubseekoff(-15, std::ios_base::cur);
 assert(p == 11);
 assert(f.sgetc() == L'l');
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41748: [libcxx] [test] Fix Xxx_scan tests using nonstandard things and MSVC++ warnings

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

> I hate your compiler.

Seems like a valid warning to me :)

Changed predicate to one that adds 10 to each input.


https://reviews.llvm.org/D41748



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41748: [libcxx] [test] Fix Xxx_scan tests using nonstandard things and MSVC++ warnings

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 128836.
BillyONeal marked 3 inline comments as done.
BillyONeal added a comment.

Also change predicate from identity to add_ten.


https://reviews.llvm.org/D41748

Files:
  test/std/numerics/numeric.ops/exclusive.scan/exclusive_scan.pass.cpp
  test/std/numerics/numeric.ops/exclusive.scan/exclusive_scan_init_op.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op_init.pass.cpp
  
test/std/numerics/numeric.ops/transform.exclusive.scan/transform_exclusive_scan_init_bop_uop.pass.cpp
  
test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop.pass.cpp
  
test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp

Index: test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
+++ test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
@@ -20,25 +20,19 @@
 
 
 #include 
-#include 
+#include 
 #include 
+#include 
+#include 
+#include 
 
 #include "test_iterators.h"
 
-template 
-struct identity : std::unary_function
-{
-constexpr const T& operator()(const T& x) const { return x;}
-};
-
-template <>
-struct identity
-{
-template 
-constexpr auto operator()(T&& x) const
-_NOEXCEPT_(noexcept(_VSTD::forward(x)))
--> decltype(_VSTD::forward(x))
-{ return_VSTD::forward(x); }
+struct add_ten {
+template 
+constexpr auto operator()(T x) const noexcept {
+return static_cast(x + 10);
+}
 };
 
 template 
@@ -63,12 +57,12 @@
 test()
 {
   int ia[] = {  1,  3,   5,7, 9};
-const int pResI0[] = {  1,  4,   9,   16,25};// with identity
+const int pResI0[] = { 11, 24,  39,   56,75};// with add_ten
 const int mResI0[] = {  0,  0,   0,0, 0};
 const int pResN0[] = { -1, -4,  -9,  -16,   -25};// with negate
 const int mResN0[] = {  0,  0,   0,0, 0};
-const int pResI2[] = {  3,  6,  11,   18,27};// with identity
-const int mResI2[] = {  2,  6,  30,  210,  1890};
+const int pResI2[] = { 13, 26,  41,   58,77};// with add_ten
+const int mResI2[] = { 22, 286, 4290, 72930, 1385670};
 const int pResN2[] = {  1, -2,  -7,  -14,   -23};// with negate
 const int mResN2[] = { -2,  6, -30,  210, -1890};
 const unsigned sa = sizeof(ia) / sizeof(ia[0]);
@@ -82,12 +76,12 @@
 static_assert(sa == sizeof(mResN2) / sizeof(mResN2[0]));   // just to be sure
 
 for (unsigned int i = 0; i < sa; ++i ) {
-test(Iter(ia), Iter(ia + i), std::plus<>(),   identity<>(),0, pResI0, pResI0 + i);
-test(Iter(ia), Iter(ia + i), std::multiplies<>(), identity<>(),0, mResI0, mResI0 + i);
+test(Iter(ia), Iter(ia + i), std::plus<>(),   add_ten{},   0, pResI0, pResI0 + i);
+test(Iter(ia), Iter(ia + i), std::multiplies<>(), add_ten{},   0, mResI0, mResI0 + i);
 test(Iter(ia), Iter(ia + i), std::plus<>(),   std::negate<>(), 0, pResN0, pResN0 + i);
 test(Iter(ia), Iter(ia + i), std::multiplies<>(), std::negate<>(), 0, mResN0, mResN0 + i);
-test(Iter(ia), Iter(ia + i), std::plus<>(),   identity<>(),2, pResI2, pResI2 + i);
-test(Iter(ia), Iter(ia + i), std::multiplies<>(), identity<>(),2, mResI2, mResI2 + i);
+test(Iter(ia), Iter(ia + i), std::plus<>(),   add_ten{},   2, pResI2, pResI2 + i);
+test(Iter(ia), Iter(ia + i), std::multiplies<>(), add_ten{},   2, mResI2, mResI2 + i);
 test(Iter(ia), Iter(ia + i), std::plus<>(),   std::negate<>(), 2, pResN2, pResN2 + i);
 test(Iter(ia), Iter(ia + i), std::multiplies<>(), std::negate<>(), 2, mResN2, mResN2 + i);
 }
@@ -101,46 +95,46 @@
 {
 std::vector v(10);
 std::fill(v.begin(), v.end(), 3);
-std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), identity<>(), 50);
+std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), add_ten{}, 50);
 for (size_t i = 0; i < v.size(); ++i)
-assert(v[i] == 50 + (int) (i + 1) * 3);
+assert(v[i] == 50 + (int) (i + 1) * 13);
 }
 
 {
 std::vector v(10);
 std::iota(v.begin(), v.end(), 0);
-std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), identity<>(), 30);
+std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), add_ten{}, 30);
 for (size_t i = 0; i < v.size(); ++i)
-assert(v[i] == 30 + triangle(i));
+assert(v[i] == 30 

[PATCH] D41372: [libcxx] Fix transform_reduce mishandling move-only types, and nonstandard macro use in tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 128695.
BillyONeal added a comment.

Moved things to MoveOnly.h.


https://reviews.llvm.org/D41372

Files:
  include/numeric
  
test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_init_bop_uop.pass.cpp
  
test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp
  
test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init_op_op.pass.cpp
  test/support/MoveOnly.h

Index: test/support/MoveOnly.h
===
--- test/support/MoveOnly.h
+++ test/support/MoveOnly.h
@@ -19,7 +19,6 @@
 
 class MoveOnly
 {
-friend class MoveOnly2;
 MoveOnly(const MoveOnly&);
 MoveOnly& operator=(const MoveOnly&);
 
@@ -35,6 +34,8 @@
 
 bool operator==(const MoveOnly& x) const {return data_ == x.data_;}
 bool operator< (const MoveOnly& x) const {return data_ <  x.data_;}
+MoveOnly operator+(const MoveOnly& x) const { return MoveOnly{data_ + x.data_}; }
+MoveOnly operator*(const MoveOnly& x) const { return MoveOnly{data_ * x.data_}; }
 };
 
 namespace std {
Index: test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init_op_op.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init_op_op.pass.cpp
+++ test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init_op_op.pass.cpp
@@ -19,7 +19,9 @@
 
 #include 
 #include 
+#include 
 
+#include "MoveOnly.h"
 #include "test_iterators.h"
 
 template 
@@ -58,6 +60,16 @@
decltype(std::transform_reduce(p, p, p, Init{}, std::plus<>(), std::multiplies<>()))> );
 }
 
+void test_move_only_types()
+{
+MoveOnly ia[] = {{1}, {2}, {3}};
+MoveOnly ib[] = {{1}, {2}, {3}};
+assert(14 ==
+std::transform_reduce(std::begin(ia), std::end(ia), std::begin(ib), MoveOnly{0},
+[](const MoveOnly& lhs, const MoveOnly& rhs) { return MoveOnly{lhs.get() + rhs.get()}; },
+[](const MoveOnly& lhs, const MoveOnly& rhs) { return MoveOnly{lhs.get() * rhs.get()}; }).get());
+}
+
 int main()
 {
 test_return_type();
@@ -94,4 +106,6 @@
 test();
 test<  int*, const unsigned int *>();
 test<  int*,   unsigned int *>();
+
+test_move_only_types();
 }
Index: test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp
+++ test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp
@@ -17,7 +17,9 @@
 
 #include 
 #include 
+#include 
 
+#include "MoveOnly.h"
 #include "test_iterators.h"
 
 template 
@@ -56,6 +58,14 @@
decltype(std::transform_reduce(p, p, p, Init{}))> );
 }
 
+void test_move_only_types()
+{
+MoveOnly ia[] = {{1}, {2}, {3}};
+MoveOnly ib[] = {{1}, {2}, {3}};
+assert(14 ==
+std::transform_reduce(std::begin(ia), std::end(ia), std::begin(ib), MoveOnly{0}).get());
+}
+
 int main()
 {
 test_return_type();
@@ -92,4 +102,6 @@
 test();
 test<  int*, const unsigned int *>();
 test<  int*,   unsigned int *>();
+
+test_move_only_types();
 }
Index: test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_init_bop_uop.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_init_bop_uop.pass.cpp
+++ test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_init_bop_uop.pass.cpp
@@ -18,40 +18,26 @@
 
 #include 
 #include 
+#include 
+#include 
 
+#include "MoveOnly.h"
 #include "test_iterators.h"
 
-template 
-struct identity : std::unary_function
-{
-constexpr const T& operator()(const T& x) const { return x;}
-};
-
-template <>
-struct identity
+struct identity
 {
 template 
-constexpr auto operator()(T&& x) const
-_NOEXCEPT_(noexcept(_VSTD::forward(x)))
--> decltype(_VSTD::forward(x))
-{ return_VSTD::forward(x); }
+constexpr decltype(auto) operator()(T&& x) const {
+return std::forward(x);
+}
 };
 
-
-template 
 struct twice
-{
-constexpr const T operator()(const T& x) const noexcept { return 2 * x; }
-};
-
-template <>
-struct twice
 {
 template 
-constexpr auto operator()(const T& x) const
-_NOEXCEPT_(noexcept(2 * x))
--> decltype(2 * x)
-{ return2 * x; }
+constexpr auto operator()(const T& x) const {
+return 2 * x;
+}
 };
 
 template 
@@ -70,31 +56,40 @@
 int ia[]  = {1, 2, 3, 4, 5, 6};
 unsigned sa = sizeof(ia) / sizeof(ia[0]);
 
-test(Iter(ia), Iter(ia),0, std::plus<>(),   identity<>(),

[PATCH] D41748: [libcxx] [test] Fix Xxx_scan tests using nonstandard things and MSVC++ warnings

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF.

- These tests use function objects from functional, back_inserter from 
iterator, and equal from algorithm, so add those headers.
- The use of iota targeting vector with an int parameter 
triggers warnings on MSVC++ assigning an into a unsigned char&; so change the 
parameter to unsigned char with a static_cast.
- Avoid naming unary_function in identity here as that is removed in '17. (This 
also fixes naming _VSTD, _NOEXCEPT_, and other libcxx-isms)


https://reviews.llvm.org/D41748

Files:
  test/std/numerics/numeric.ops/exclusive.scan/exclusive_scan.pass.cpp
  test/std/numerics/numeric.ops/exclusive.scan/exclusive_scan_init_op.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op.pass.cpp
  test/std/numerics/numeric.ops/inclusive.scan/inclusive_scan_op_init.pass.cpp
  
test/std/numerics/numeric.ops/transform.exclusive.scan/transform_exclusive_scan_init_bop_uop.pass.cpp
  
test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop.pass.cpp
  
test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp

Index: test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
+++ test/std/numerics/numeric.ops/transform.inclusive.scan/transform_inclusive_scan_bop_uop_init.pass.cpp
@@ -20,26 +20,15 @@
 
 
 #include 
-#include 
+#include 
 #include 
+#include 
+#include 
+#include 
 
 #include "test_iterators.h"
 
-template 
-struct identity : std::unary_function
-{
-constexpr const T& operator()(const T& x) const { return x;}
-};
-
-template <>
-struct identity
-{
-template 
-constexpr auto operator()(T&& x) const
-_NOEXCEPT_(noexcept(_VSTD::forward(x)))
--> decltype(_VSTD::forward(x))
-{ return_VSTD::forward(x); }
-};
+const auto identity = [](auto&& x) { return std::forward(x); };
 
 template 
 void
@@ -82,12 +71,12 @@
 static_assert(sa == sizeof(mResN2) / sizeof(mResN2[0]));   // just to be sure
 
 for (unsigned int i = 0; i < sa; ++i ) {
-test(Iter(ia), Iter(ia + i), std::plus<>(),   identity<>(),0, pResI0, pResI0 + i);
-test(Iter(ia), Iter(ia + i), std::multiplies<>(), identity<>(),0, mResI0, mResI0 + i);
+test(Iter(ia), Iter(ia + i), std::plus<>(),   identity,0, pResI0, pResI0 + i);
+test(Iter(ia), Iter(ia + i), std::multiplies<>(), identity,0, mResI0, mResI0 + i);
 test(Iter(ia), Iter(ia + i), std::plus<>(),   std::negate<>(), 0, pResN0, pResN0 + i);
 test(Iter(ia), Iter(ia + i), std::multiplies<>(), std::negate<>(), 0, mResN0, mResN0 + i);
-test(Iter(ia), Iter(ia + i), std::plus<>(),   identity<>(),2, pResI2, pResI2 + i);
-test(Iter(ia), Iter(ia + i), std::multiplies<>(), identity<>(),2, mResI2, mResI2 + i);
+test(Iter(ia), Iter(ia + i), std::plus<>(),   identity,2, pResI2, pResI2 + i);
+test(Iter(ia), Iter(ia + i), std::multiplies<>(), identity,2, mResI2, mResI2 + i);
 test(Iter(ia), Iter(ia + i), std::plus<>(),   std::negate<>(), 2, pResN2, pResN2 + i);
 test(Iter(ia), Iter(ia + i), std::multiplies<>(), std::negate<>(), 2, mResN2, mResN2 + i);
 }
@@ -101,39 +90,39 @@
 {
 std::vector v(10);
 std::fill(v.begin(), v.end(), 3);
-std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), identity<>(), 50);
+std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), identity, 50);
 for (size_t i = 0; i < v.size(); ++i)
 assert(v[i] == 50 + (int) (i + 1) * 3);
 }
 
 {
 std::vector v(10);
 std::iota(v.begin(), v.end(), 0);
-std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), identity<>(), 30);
+std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), identity, 30);
 for (size_t i = 0; i < v.size(); ++i)
 assert(v[i] == 30 + triangle(i));
 }
 
 {
 std::vector v(10);
 std::iota(v.begin(), v.end(), 1);
-std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), identity<>(), 40);
+std::transform_inclusive_scan(v.begin(), v.end(), v.begin(), std::plus<>(), identity, 40);
 for (size_t i = 0; i < v.size(); ++i)
 assert(v[i] == 40 + triangle(i + 1));
 }
 
 {
 std::vector v, res;
-std::transform_inclusive_scan(v.begin(), v.end(), std::back_inserter(res), std::plus<>(), identity<>(), 1);
+std::transform_inclusive_scan(v.begin(), v.end(), std::back_inserter(res), std::plus<>(), identity, 1);
 assert(res.empty());
 }

[PATCH] D41372: [libcxx] Fix transform_reduce mishandling move-only types, and nonstandard macro use in tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

Do you folks want me to port this test case for move-only T into libcxx? (I 
added this to our test harness) Note that discussion on validity is ongoing on 
lists.

  #define _SILENCE_PARALLEL_ALGORITHMS_EXPERIMENTAL_WARNING
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  #include 
  
  using namespace std;
  using namespace std::execution;
  
  void verify(const bool b) {
  if (!b) {
  exit(PM_TEST_FAIL);
  }
  }
  
  vector> get_move_only_test_data(const size_t 
testSize) {
  vector> testData;
  testData.reserve(testSize);
  for (size_t idx = 0; idx < testSize; ++idx) {
  testData.emplace_back(make_unique(1, 
static_cast(idx)));
  }
  
  return testData;
  }
  
  template
  void test_case_move_only(const size_t testSize, ExPo&& exec) {
  auto testData = get_move_only_test_data(testSize);
  unique_ptr result = 
transform_reduce(std::forward(exec),
  make_move_iterator(testData.begin()), 
make_move_iterator(testData.end()),
  make_unique(),
  [](unique_ptr lhs, unique_ptr rhs) {
  lhs->insert(lhs->end(), rhs->begin(), rhs->end());
  return lhs;
  },
  [](unique_ptr target) {
  verify(target->size() == 1); // should only be called directly on 
the input sequence
  target->back() *= 10;
  return target;
  });
  
  sort(result->begin(), result->end());
  for (size_t idx = 0; idx < testSize; ++idx) {
  verify((*result)[idx] == idx * 10);
  }
  }
  
  int main() {
  parallel_test_case(test_case_move_only, seq);
  parallel_test_case(test_case_move_only, par);
  return PM_TEST_PASS;
  }




Comment at: include/numeric:245
 for (; __first1 != __last1; ++__first1, (void) ++__first2)
 __init = __b1(__init, __b2(*__first1, *__first2));
 return __init;

BillyONeal wrote:
> It's arguable that this should be _VSTD::move(__init); I asked SG1 / LEWG 
> about that. (After all, if they're going to make inner_product do that this 
> should too)
Any comment on whether these should have extra moves added @mclow.lists  ?



Comment at: 
test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp:61
 
+inline MoveOnly operator+(const MoveOnly& lhs, const MoveOnly& rhs)
+{

mclow.lists wrote:
> BillyONeal wrote:
> > Should these go in MoveOnly.h ?
> I think so. While you're at it, you can remove the `friend` declaration in 
> `MoveOnly` - that's a remnant.
Should I also add op== and op< while I'm in there for testing things like sort?


Repository:
  rL LLVM

https://reviews.llvm.org/D41372



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41372: [libcxx] Fix transform_reduce mishandling move-only types, and nonstandard macro use in tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: include/numeric:245
 for (; __first1 != __last1; ++__first1, (void) ++__first2)
 __init = __b1(__init, __b2(*__first1, *__first2));
 return __init;

It's arguable that this should be _VSTD::move(__init); I asked SG1 / LEWG about 
that. (After all, if they're going to make inner_product do that this should 
too)


Repository:
  rL LLVM

https://reviews.llvm.org/D41372



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41372: [libcxx] Fix transform_reduce mishandling move-only types, and nonstandard macro use in tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added inline comments.



Comment at: 
test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp:61
 
+inline MoveOnly operator+(const MoveOnly& lhs, const MoveOnly& rhs)
+{

Should these go in MoveOnly.h ?


Repository:
  rL LLVM

https://reviews.llvm.org/D41372



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41372: [libcxx] Fix transform_reduce mishandling move-only types, and nonstandard macro use in tests.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.
BillyONeal added reviewers: mclow.lists, EricWF.

- Remove _VSTD use and remove C++03-isms in C++17 tests for transform_reduce.
- Add tests for all the transform_reduce overloads that pass in a move-only 
type.
- Repair missing move in libc++ product code.


Repository:
  rL LLVM

https://reviews.llvm.org/D41372

Files:
  include/numeric
  
test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_init_bop_uop.pass.cpp
  
test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp
  
test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init_op_op.pass.cpp

Index: test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init_op_op.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init_op_op.pass.cpp
+++ test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init_op_op.pass.cpp
@@ -19,7 +19,9 @@
 
 #include 
 #include 
+#include 
 
+#include "MoveOnly.h"
 #include "test_iterators.h"
 
 template 
@@ -58,6 +60,16 @@
decltype(std::transform_reduce(p, p, p, Init{}, std::plus<>(), std::multiplies<>()))> );
 }
 
+void test_move_only_types()
+{
+MoveOnly ia[] = {{1}, {2}, {3}};
+MoveOnly ib[] = {{1}, {2}, {3}};
+assert(14 ==
+std::transform_reduce(std::begin(ia), std::end(ia), std::begin(ib), MoveOnly{0},
+[](const MoveOnly& lhs, const MoveOnly& rhs) { return MoveOnly{lhs.get() + rhs.get()}; },
+[](const MoveOnly& lhs, const MoveOnly& rhs) { return MoveOnly{lhs.get() * rhs.get()}; }).get());
+}
+
 int main()
 {
 test_return_type();
@@ -94,4 +106,6 @@
 test();
 test<  int*, const unsigned int *>();
 test<  int*,   unsigned int *>();
+
+test_move_only_types();
 }
Index: test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp
+++ test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_iter_init.pass.cpp
@@ -17,7 +17,9 @@
 
 #include 
 #include 
+#include 
 
+#include "MoveOnly.h"
 #include "test_iterators.h"
 
 template 
@@ -56,6 +58,24 @@
decltype(std::transform_reduce(p, p, p, Init{}))> );
 }
 
+inline MoveOnly operator+(const MoveOnly& lhs, const MoveOnly& rhs)
+{
+return MoveOnly{lhs.get() + rhs.get()};
+}
+
+inline MoveOnly operator*(const MoveOnly& lhs, const MoveOnly& rhs)
+{
+return MoveOnly{lhs.get() * rhs.get()};
+}
+
+void test_move_only_types()
+{
+MoveOnly ia[] = {{1}, {2}, {3}};
+MoveOnly ib[] = {{1}, {2}, {3}};
+assert(14 ==
+std::transform_reduce(std::begin(ia), std::end(ia), std::begin(ib), MoveOnly{0}).get());
+}
+
 int main()
 {
 test_return_type();
@@ -92,4 +112,6 @@
 test();
 test<  int*, const unsigned int *>();
 test<  int*,   unsigned int *>();
+
+test_move_only_types();
 }
Index: test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_init_bop_uop.pass.cpp
===
--- test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_init_bop_uop.pass.cpp
+++ test/std/numerics/numeric.ops/transform.reduce/transform_reduce_iter_iter_init_bop_uop.pass.cpp
@@ -18,40 +18,26 @@
 
 #include 
 #include 
+#include 
+#include 
 
+#include "MoveOnly.h"
 #include "test_iterators.h"
 
-template 
-struct identity : std::unary_function
-{
-constexpr const T& operator()(const T& x) const { return x;}
-};
-
-template <>
-struct identity
+struct identity
 {
 template 
-constexpr auto operator()(T&& x) const
-_NOEXCEPT_(noexcept(_VSTD::forward(x)))
--> decltype(_VSTD::forward(x))
-{ return_VSTD::forward(x); }
+constexpr decltype(auto) operator()(T&& x) const {
+return std::forward(x);
+}
 };
 
-
-template 
 struct twice
-{
-constexpr const T operator()(const T& x) const noexcept { return 2 * x; }
-};
-
-template <>
-struct twice
 {
 template 
-constexpr auto operator()(const T& x) const
-_NOEXCEPT_(noexcept(2 * x))
--> decltype(2 * x)
-{ return2 * x; }
+constexpr auto operator()(const T& x) const {
+return 2 * x;
+}
 };
 
 template 
@@ -70,31 +56,40 @@
 int ia[]  = {1, 2, 3, 4, 5, 6};
 unsigned sa = sizeof(ia) / sizeof(ia[0]);
 
-test(Iter(ia), Iter(ia),0, std::plus<>(),   identity<>(),   0);
-test(Iter(ia), Iter(ia),1, std::multiplies<>(), identity<>(),   1);
-test(Iter(ia), Iter(ia+1),  0, std::multiplies<>(), identity<>(),   0);
-test(Iter(ia), Iter(ia+1),  2, std::plus<>(),   

[PATCH] D40324: [libcxx] [test] Resolve C1XX warning in new vector::size tests caused by assert(true)

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

Committed r318812


https://reviews.llvm.org/D40324



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40065: [libcxx] [test] Change (void)s to TEST_IGNORE_NODISCARD as requested by Eric.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

Committed r318804


https://reviews.llvm.org/D40065



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40065: [libcxx] [test] Change (void)s to TEST_IGNORE_NODISCARD as requested by Eric.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.

Added the TEST_IGNORE_NODISCARD macro to `test_macros.h`.
grep test/std for (void) casts that look like they're suppressing [[nodiscard]] 
and change them to TEST_IGNORE_NODISCARD.


https://reviews.llvm.org/D40065

Files:
  test/std/algorithms/alg.nonmodifying/alg.search/search_n_pred.pass.cpp
  test/std/containers/associative/map/map.access/at.pass.cpp
  test/std/containers/associative/map/map.ops/count1.fail.cpp
  test/std/containers/associative/map/map.ops/count2.fail.cpp
  test/std/containers/associative/map/map.ops/count3.fail.cpp
  test/std/containers/associative/map/map.ops/equal_range1.fail.cpp
  test/std/containers/associative/map/map.ops/equal_range2.fail.cpp
  test/std/containers/associative/map/map.ops/equal_range3.fail.cpp
  test/std/containers/associative/map/map.ops/find1.fail.cpp
  test/std/containers/associative/map/map.ops/find2.fail.cpp
  test/std/containers/associative/map/map.ops/find3.fail.cpp
  test/std/containers/associative/map/map.ops/lower_bound1.fail.cpp
  test/std/containers/associative/map/map.ops/lower_bound2.fail.cpp
  test/std/containers/associative/map/map.ops/lower_bound3.fail.cpp
  test/std/containers/associative/map/map.ops/upper_bound1.fail.cpp
  test/std/containers/associative/map/map.ops/upper_bound2.fail.cpp
  test/std/containers/associative/map/map.ops/upper_bound3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/count1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/count2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/count3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/equal_range1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/equal_range2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/equal_range3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/find1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/find2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/find3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/lower_bound1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/lower_bound2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/lower_bound3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/upper_bound1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/upper_bound2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/upper_bound3.fail.cpp
  test/std/containers/sequences/array/at.pass.cpp
  test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp
  
test/std/diagnostics/syserr/syserr.errcode/syserr.errcode.overview/types.pass.cpp
  
test/std/diagnostics/syserr/syserr.errcondition/syserr.errcondition.overview/types.pass.cpp
  test/std/iterators/iterator.range/begin-end.fail.cpp
  test/std/iterators/iterators.general/gcc_workaround.pass.cpp
  
test/std/iterators/predef.iterators/insert.iterators/insert.iterator/types.pass.cpp
  
test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.nonmember/make_move_iterator.pass.cpp
  test/std/numerics/numeric.ops/numeric.ops.lcm/lcm.pass.cpp
  test/std/re/re.alg/re.alg.search/grep.pass.cpp
  test/std/re/re.grammar/excessive_brace_count.pass.cpp
  test/std/strings/basic.string/string.access/at.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_T_size_size.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_pointer.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_pointer_size.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_string.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_string_size_size.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_string_view.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock_for.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock_until.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.locking/try_lock.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.locking/try_lock_for.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.locking/try_lock_until.pass.cpp
  test/std/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp
  test/std/utilities/any/any.nonmembers/make_any.pass.cpp
  
test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
  
test/std/utilities/memory/util.smartptr/util.smartptr.enab/enable_shared_from_this.pass.cpp
  
test/std/utilities/smartptr/unique.ptr/unique.ptr.class/unique.ptr.observers/dereference.runtime.fail.cpp
  

[PATCH] D39080: [libcxx] [test] Tolerate even more [[nodiscard]] in the STL.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.

STL reviewed my [[nodiscard]] changes and suggested a ton more places. These 
are the associated test changes in libcxx.


https://reviews.llvm.org/D39080

Files:
  test/std/algorithms/alg.nonmodifying/alg.search/search_n.pass.cpp
  test/std/algorithms/alg.nonmodifying/alg.search/search_n_pred.pass.cpp
  test/std/containers/associative/map/map.access/at.pass.cpp
  test/std/containers/unord/unord.map/unord.map.elem/at.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.disc/assign.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.disc/copy.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.disc/discard.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.ibits/assign.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.ibits/copy.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.ibits/discard.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.shuf/assign.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.shuf/copy.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.shuf/ctor_engine_copy.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.shuf/ctor_engine_move.pass.cpp
  test/std/numerics/rand/rand.adapt/rand.adapt.shuf/discard.pass.cpp
  test/std/numerics/rand/rand.device/eval.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.lcong/discard.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.mers/assign.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.mers/copy.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.mers/discard.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.sub/assign.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.sub/copy.pass.cpp
  test/std/numerics/rand/rand.eng/rand.eng.sub/discard.pass.cpp
  test/std/re/re.alg/re.alg.search/grep.pass.cpp
  test/std/thread/thread.mutex/thread.lock.algorithm/try_lock.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock_for.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock_until.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.locking/try_lock.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.locking/try_lock_for.pass.cpp
  
test/std/thread/thread.mutex/thread.lock/thread.lock.unique/thread.lock.unique.locking/try_lock_until.pass.cpp
  test/std/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp
  
test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
  
test/std/utilities/memory/util.smartptr/util.smartptr.enab/enable_shared_from_this.pass.cpp
  
test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.ownerless/owner_less.pass.cpp

Index: test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.ownerless/owner_less.pass.cpp
===
--- test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.ownerless/owner_less.pass.cpp
+++ test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.ownerless/owner_less.pass.cpp
@@ -112,11 +112,11 @@
 std::weak_ptr wp1;
 
 std::owner_less<> cmp;
-cmp(sp1, sp2);
-cmp(sp1, wp1);
-cmp(sp1, sp3);
-cmp(wp1, sp1);
-cmp(wp1, wp1);
+assert(!cmp(sp1, sp2));
+assert(!cmp(sp1, wp1));
+assert(!cmp(sp1, sp3));
+assert(!cmp(wp1, sp1));
+assert(!cmp(wp1, wp1));
 ASSERT_NOEXCEPT(cmp(sp1, sp1));
 ASSERT_NOEXCEPT(cmp(sp1, wp1));
 ASSERT_NOEXCEPT(cmp(wp1, sp1));
Index: test/std/utilities/memory/util.smartptr/util.smartptr.enab/enable_shared_from_this.pass.cpp
===
--- test/std/utilities/memory/util.smartptr/util.smartptr.enab/enable_shared_from_this.pass.cpp
+++ test/std/utilities/memory/util.smartptr/util.smartptr.enab/enable_shared_from_this.pass.cpp
@@ -134,7 +134,7 @@
 
 #ifndef TEST_HAS_NO_EXCEPTIONS
 try {
-ptr->shared_from_this();
+(void)ptr->shared_from_this();
 assert(false);
 } catch (std::bad_weak_ptr const&) {
 } catch (...) { assert(false); }
Index: test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
===
--- test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
+++ test/std/utilities/memory/default.allocator/allocator.members/allocate.size.pass.cpp
@@ -23,7 +23,7 @@
 {
 std::allocator a;
 try {
-a.allocate(count);
+(void)a.allocate(count);
 assert(false);
 } catch (const std::exception &) {
 }
Index: test/std/utilities/any/any.nonmembers/any.cast/any_cast_reference.pass.cpp

[PATCH] D39033: [libcxx] [test] Tolerate [[nodiscard]] on STL functions in libcxx tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.

I applied [[nodiscard]] to lots of things in MSVC++'s STL, which trips up some 
tests in libcxx. This change adds (void) casts or assertions as necessary to 
make [[nodiscard]] happy.


Repository:
  rL LLVM

https://reviews.llvm.org/D39033

Files:
  test/std/containers/associative/map/map.ops/count0.pass.cpp
  test/std/containers/associative/map/map.ops/count1.fail.cpp
  test/std/containers/associative/map/map.ops/count2.fail.cpp
  test/std/containers/associative/map/map.ops/count3.fail.cpp
  test/std/containers/associative/map/map.ops/equal_range0.pass.cpp
  test/std/containers/associative/map/map.ops/equal_range1.fail.cpp
  test/std/containers/associative/map/map.ops/equal_range2.fail.cpp
  test/std/containers/associative/map/map.ops/equal_range3.fail.cpp
  test/std/containers/associative/map/map.ops/find0.pass.cpp
  test/std/containers/associative/map/map.ops/find1.fail.cpp
  test/std/containers/associative/map/map.ops/find2.fail.cpp
  test/std/containers/associative/map/map.ops/find3.fail.cpp
  test/std/containers/associative/map/map.ops/lower_bound0.pass.cpp
  test/std/containers/associative/map/map.ops/lower_bound1.fail.cpp
  test/std/containers/associative/map/map.ops/lower_bound2.fail.cpp
  test/std/containers/associative/map/map.ops/lower_bound3.fail.cpp
  test/std/containers/associative/map/map.ops/upper_bound0.pass.cpp
  test/std/containers/associative/map/map.ops/upper_bound1.fail.cpp
  test/std/containers/associative/map/map.ops/upper_bound2.fail.cpp
  test/std/containers/associative/map/map.ops/upper_bound3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/count0.pass.cpp
  test/std/containers/associative/multimap/multimap.ops/count1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/count2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/count3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/equal_range0.pass.cpp
  test/std/containers/associative/multimap/multimap.ops/equal_range1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/equal_range2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/equal_range3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/find0.pass.cpp
  test/std/containers/associative/multimap/multimap.ops/find1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/find2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/find3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/lower_bound0.pass.cpp
  test/std/containers/associative/multimap/multimap.ops/lower_bound1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/lower_bound2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/lower_bound3.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/upper_bound0.pass.cpp
  test/std/containers/associative/multimap/multimap.ops/upper_bound1.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/upper_bound2.fail.cpp
  test/std/containers/associative/multimap/multimap.ops/upper_bound3.fail.cpp
  test/std/iterators/iterators.general/gcc_workaround.pass.cpp
  
test/std/iterators/predef.iterators/move.iterators/move.iter.ops/move.iter.nonmember/make_move_iterator.pass.cpp
  test/std/strings/basic.string/string.access/at.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_T_size_size.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_pointer.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_pointer_size.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_string.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_string_size_size.pass.cpp
  
test/std/strings/basic.string/string.ops/string_compare/size_size_string_view.pass.cpp
  test/std/strings/string.view/string.view.access/at.pass.cpp
  test/std/utilities/any/any.nonmembers/make_any.pass.cpp
  
test/std/utilities/memory/util.smartptr/util.smartptr.weak/util.smartptr.ownerless/owner_less.pass.cpp
  
test/std/utilities/optional/optional.object/optional.object.observe/value.pass.cpp
  
test/std/utilities/optional/optional.object/optional.object.observe/value_const.pass.cpp
  
test/std/utilities/optional/optional.object/optional.object.observe/value_const_rvalue.pass.cpp
  
test/std/utilities/optional/optional.object/optional.object.observe/value_rvalue.pass.cpp
  test/std/utilities/variant/variant.get/get_index.pass.cpp
  test/std/utilities/variant/variant.get/get_type.pass.cpp

Index: test/std/utilities/variant/variant.get/get_type.pass.cpp
===
--- test/std/utilities/variant/variant.get/get_type.pass.cpp
+++ test/std/utilities/variant/variant.get/get_type.pass.cpp
@@ -259,7 +259,7 @@
   auto test = [](auto idx, auto &) {
 using Idx = decltype(idx);
 try {
-  std::get(std::forward(v));
+  

[PATCH] D33708: [test] [libcxx] Disable MSVC++'s compare(a, b) implies !compare(b, a) assertion in predicate application count tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal abandoned this revision.
BillyONeal added a comment.

Stephan pointed out that the force include breaks this. Backing it out for now 
:(


https://reviews.llvm.org/D33708



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33708: [test] [libcxx] Disable MSVC++'s compare(a, b) implies !compare(b, a) assertion in predicate application count tests

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.

Turn off MSVC++'s comp(a, b) implies !comp(b, a) assertion in tests that are 
checking for predicate counts.

There should be no functionality change for libcxx as it does not look for the 
_ITERATOR_DEBUG_LEVEL macro.

While this assert is technically out of contract because the standard specifies 
exact operation counts for these algorithms, in practice users using !less t
o implement greater is a fairly common mistake, and this assert turns out to be 
highly compatible. Note also that in the inplace_merge case, users can't even 
rely on exact application counts here, so the complexity bound is probably 
overspecified in the standard.


https://reviews.llvm.org/D33708

Files:
  
test/std/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp
  test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
  test/std/algorithms/alg.sorting/alg.min.max/minmax_init_list_comp.pass.cpp


Index: 
test/std/algorithms/alg.sorting/alg.min.max/minmax_init_list_comp.pass.cpp
===
--- test/std/algorithms/alg.sorting/alg.min.max/minmax_init_list_comp.pass.cpp
+++ test/std/algorithms/alg.sorting/alg.min.max/minmax_init_list_comp.pass.cpp
@@ -17,6 +17,10 @@
 //
 //  Complexity: At most (3/2) * t.size() applications of the corresponding 
predicate.
 
+// Disable MSVC++'s "comp(a, b) implies !comp(b, a)" assertion:
+#undef _ITERATOR_DEBUG_LEVEL
+#define _ITERATOR_DEBUG_LEVEL 0
+
 #include 
 #include 
 #include 
Index: test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
===
--- test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
+++ test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
@@ -15,6 +15,10 @@
 //   void
 //   inplace_merge(Iter first, Iter middle, Iter last, Compare comp);
 
+// Disable MSVC++'s "comp(a, b) implies !comp(b, a)" assertion:
+#undef _ITERATOR_DEBUG_LEVEL
+#define _ITERATOR_DEBUG_LEVEL 0
+
 #include 
 #include 
 #include 
Index: 
test/std/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp
===
--- 
test/std/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp
+++ 
test/std/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp
@@ -14,6 +14,10 @@
 //   void
 //   make_heap(Iter first, Iter last, Compare comp);
 
+// Disable MSVC++'s "comp(a, b) implies !comp(b, a)" assertion:
+#undef _ITERATOR_DEBUG_LEVEL
+#define _ITERATOR_DEBUG_LEVEL 0
+
 #include 
 #include 
 #include 


Index: test/std/algorithms/alg.sorting/alg.min.max/minmax_init_list_comp.pass.cpp
===
--- test/std/algorithms/alg.sorting/alg.min.max/minmax_init_list_comp.pass.cpp
+++ test/std/algorithms/alg.sorting/alg.min.max/minmax_init_list_comp.pass.cpp
@@ -17,6 +17,10 @@
 //
 //  Complexity: At most (3/2) * t.size() applications of the corresponding predicate.
 
+// Disable MSVC++'s "comp(a, b) implies !comp(b, a)" assertion:
+#undef _ITERATOR_DEBUG_LEVEL
+#define _ITERATOR_DEBUG_LEVEL 0
+
 #include 
 #include 
 #include 
Index: test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
===
--- test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
+++ test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp
@@ -15,6 +15,10 @@
 //   void
 //   inplace_merge(Iter first, Iter middle, Iter last, Compare comp);
 
+// Disable MSVC++'s "comp(a, b) implies !comp(b, a)" assertion:
+#undef _ITERATOR_DEBUG_LEVEL
+#define _ITERATOR_DEBUG_LEVEL 0
+
 #include 
 #include 
 #include 
Index: test/std/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp
===
--- test/std/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp
+++ test/std/algorithms/alg.sorting/alg.heap.operations/make.heap/make_heap_comp.pass.cpp
@@ -14,6 +14,10 @@
 //   void
 //   make_heap(Iter first, Iter last, Compare comp);
 
+// Disable MSVC++'s "comp(a, b) implies !comp(b, a)" assertion:
+#undef _ITERATOR_DEBUG_LEVEL
+#define _ITERATOR_DEBUG_LEVEL 0
+
 #include 
 #include 
 #include 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33340: [libcxx] [test] Add string nullptr asserts to erase functions.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal closed this revision.
BillyONeal added a comment.

In https://reviews.llvm.org/D33340#764206, @EricWF wrote:

> Sorry for the delay reviewing such a simple change.


No problem! Their simplicity also means no likely merge conflicts :)

In https://reviews.llvm.org/D33340#764213, @EricWF wrote:

> @BillyONeal Changes like this are good for post-commit review (ie just commit 
> it). Adding clearly correct assertions and test cases is never an 
> objectionable thing.


Sounds good, thanks!


https://reviews.llvm.org/D33340



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33340: [libcxx] [test] Add string nullptr asserts to erase functions.

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal created this revision.

In my perf overhaul for MSVC++'s basic_string I wrote the null at data[oldsize] 
instead of data[newsize]; adding asserts to catch that bug.


https://reviews.llvm.org/D33340

Files:
  test/std/strings/basic.string/string.modifiers/string_erase/iter.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_erase/iter_iter.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_erase/pop_back.pass.cpp
  test/std/strings/basic.string/string.modifiers/string_erase/size_size.pass.cpp


Index: 
test/std/strings/basic.string/string.modifiers/string_erase/size_size.pass.cpp
===
--- 
test/std/strings/basic.string/string.modifiers/string_erase/size_size.pass.cpp
+++ 
test/std/strings/basic.string/string.modifiers/string_erase/size_size.pass.cpp
@@ -29,6 +29,7 @@
 {
 s.erase(pos, n);
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 }
 #ifndef TEST_HAS_NO_EXCEPTIONS
@@ -58,6 +59,7 @@
 {
 s.erase(pos);
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 }
 #ifndef TEST_HAS_NO_EXCEPTIONS
@@ -83,6 +85,7 @@
 {
 s.erase();
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 }
 
Index: 
test/std/strings/basic.string/string.modifiers/string_erase/pop_back.pass.cpp
===
--- 
test/std/strings/basic.string/string.modifiers/string_erase/pop_back.pass.cpp
+++ 
test/std/strings/basic.string/string.modifiers/string_erase/pop_back.pass.cpp
@@ -23,6 +23,7 @@
 {
 s.pop_back();
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 }
 
Index: 
test/std/strings/basic.string/string.modifiers/string_erase/iter_iter.pass.cpp
===
--- 
test/std/strings/basic.string/string.modifiers/string_erase/iter_iter.pass.cpp
+++ 
test/std/strings/basic.string/string.modifiers/string_erase/iter_iter.pass.cpp
@@ -25,6 +25,7 @@
 typename S::const_iterator last = s.cbegin() + pos + n;
 typename S::iterator i = s.erase(first, last);
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 assert(i - s.begin() == pos);
 }
Index: test/std/strings/basic.string/string.modifiers/string_erase/iter.pass.cpp
===
--- test/std/strings/basic.string/string.modifiers/string_erase/iter.pass.cpp
+++ test/std/strings/basic.string/string.modifiers/string_erase/iter.pass.cpp
@@ -24,6 +24,7 @@
 typename S::const_iterator p = s.begin() + pos;
 typename S::iterator i = s.erase(p);
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 assert(i - s.begin() == pos);
 }


Index: test/std/strings/basic.string/string.modifiers/string_erase/size_size.pass.cpp
===
--- test/std/strings/basic.string/string.modifiers/string_erase/size_size.pass.cpp
+++ test/std/strings/basic.string/string.modifiers/string_erase/size_size.pass.cpp
@@ -29,6 +29,7 @@
 {
 s.erase(pos, n);
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 }
 #ifndef TEST_HAS_NO_EXCEPTIONS
@@ -58,6 +59,7 @@
 {
 s.erase(pos);
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 }
 #ifndef TEST_HAS_NO_EXCEPTIONS
@@ -83,6 +85,7 @@
 {
 s.erase();
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 }
 
Index: test/std/strings/basic.string/string.modifiers/string_erase/pop_back.pass.cpp
===
--- test/std/strings/basic.string/string.modifiers/string_erase/pop_back.pass.cpp
+++ test/std/strings/basic.string/string.modifiers/string_erase/pop_back.pass.cpp
@@ -23,6 +23,7 @@
 {
 s.pop_back();
 LIBCPP_ASSERT(s.__invariants());
+assert(s[s.size()] == typename S::value_type());
 assert(s == expected);
 }
 
Index: test/std/strings/basic.string/string.modifiers/string_erase/iter_iter.pass.cpp
===
--- test/std/strings/basic.string/string.modifiers/string_erase/iter_iter.pass.cpp
+++ test/std/strings/basic.string/string.modifiers/string_erase/iter_iter.pass.cpp
@@ -25,6 +25,7 @@
 typename S::const_iterator last = s.cbegin() + pos + n;
 typename S::iterator i = s.erase(first, last);

[PATCH] D33021: [libcxx] [test] libc++ test changes for CWG 2094

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal updated this revision to Diff 98506.
BillyONeal added a comment.

Added spaces requested by Marshal.


https://reviews.llvm.org/D33021

Files:
  
test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_copyable.pass.cpp


Index: 
test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_copyable.pass.cpp
===
--- 
test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_copyable.pass.cpp
+++ 
test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_copyable.pass.cpp
@@ -22,13 +22,13 @@
 {
 static_assert( std::is_trivially_copyable::value, "");
 static_assert( std::is_trivially_copyable::value, "");
-static_assert(!std::is_trivially_copyable::value, "");
-static_assert(!std::is_trivially_copyable::value, "");
+static_assert( std::is_trivially_copyable::value, "");
+static_assert( std::is_trivially_copyable::value, "");
 #if TEST_STD_VER > 14
 static_assert( std::is_trivially_copyable_v, "");
 static_assert( std::is_trivially_copyable_v, "");
-static_assert(!std::is_trivially_copyable_v, "");
-static_assert(!std::is_trivially_copyable_v, "");
+static_assert( std::is_trivially_copyable_v, "");
+static_assert( std::is_trivially_copyable_v, "");
 #endif
 }
 


Index: test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_copyable.pass.cpp
===
--- test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_copyable.pass.cpp
+++ test/std/utilities/meta/meta.unary/meta.unary.prop/is_trivially_copyable.pass.cpp
@@ -22,13 +22,13 @@
 {
 static_assert( std::is_trivially_copyable::value, "");
 static_assert( std::is_trivially_copyable::value, "");
-static_assert(!std::is_trivially_copyable::value, "");
-static_assert(!std::is_trivially_copyable::value, "");
+static_assert( std::is_trivially_copyable::value, "");
+static_assert( std::is_trivially_copyable::value, "");
 #if TEST_STD_VER > 14
 static_assert( std::is_trivially_copyable_v, "");
 static_assert( std::is_trivially_copyable_v, "");
-static_assert(!std::is_trivially_copyable_v, "");
-static_assert(!std::is_trivially_copyable_v, "");
+static_assert( std::is_trivially_copyable_v, "");
+static_assert( std::is_trivially_copyable_v, "");
 #endif
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33021: [libcxx] [test] libc++ test changes for CWG 2094

[Billy Robert O'Neal III via Phabricator via cfe-commits]

BillyONeal added a comment.

In https://reviews.llvm.org/D33021#751145, @BillyONeal wrote:

> Hmm we actually took steps to do the opposite in std::copy -- to use 
> memcpy even in the presence of volatile -- as a result of this change.


;) http://imgur.com/a/PMKPS


https://reviews.llvm.org/D33021



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits