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

[Michael Schellenberger Costa via Phabricator via cfe-commits]

miscco added a comment.

Ah sorry about that, I overlooked that we had special cased tests without count.

Thanks Billy


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.

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

[Louis Dionne via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7dcbe90cc2d: [libc++] Fix overly complicated test of 
std::spans extent (authored by ldionne).
Herald added a project: libc++.
Herald added a subscriber: libcxx-commits.

Changed prior to commit:
  https://reviews.llvm.org/D73138?vs=239423=245236#toc

Repository:
  rG LLVM Github Monorepo

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

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,13 +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);
-if constexpr (Count != std::dynamic_extent) {
-static_assert(S1::extent == Count);
-} else if constexpr (Span::extent != std::dynamic_extent) {
-static_assert(S1::extent == Span::extent - Offset);
-} else {
-static_assert(S1::extent == std::dynamic_extent);
-}
+static_assert(S1::extent == Count);
 static_assert(S2::extent == std::dynamic_extent, "");
 return
 s1.data() == s2.data()
@@ -82,13 +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);
-if constexpr (Count != std::dynamic_extent) {
-static_assert(S1::extent == Count);
-} else if constexpr (Span::extent != std::dynamic_extent) {
-static_assert(S1::extent == Span::extent - Offset);
-} else {
-static_assert(S1::extent == std::dynamic_extent);
-}
+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,13 +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);
-if constexpr (Count != std::dynamic_extent) {
-static_assert(S1::extent == Count);
-} else if constexpr (Span::extent != std::dynamic_extent) {
-static_assert(S1::extent == Span::extent - Offset);
-} else {
-static_assert(S1::extent == std::dynamic_extent);
-}
+static_assert(S1::extent == Count);
 static_assert(S2::extent == std::dynamic_extent, "");
 return
 s1.data() == s2.data()
@@ -82,13 +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);
-if constexpr (Count != std::dynamic_extent) {
-static_assert(S1::extent == Count);
-} else if constexpr (Span::extent != std::dynamic_extent) {
-static_assert(S1::extent == Span::extent - Offset);
-} else {
-static_assert(S1::extent == std::dynamic_extent);
-}
+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] D73138: [libcxx] [test] Correct asserted type in subspan test; subspan with count should never produce dynamic_extent

[Louis Dionne via Phabricator via cfe-commits]

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

In D73138#1881198 , @BillyONeal wrote:

> @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).


Hmm, yes, you're right! I'll take your patch on top. Thanks!


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] D73138: [libcxx] [test] Correct asserted type in subspan test; subspan with count should never produce dynamic_extent

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

I think this was addressed by https://reviews.llvm.org/D71997.


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 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