[PATCH] D60069: Declare std::tuple_element as struct instead of class

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie created this revision.
jdoerrie added a reviewer: mclow.lists.
Herald added subscribers: libcxx-commits, ldionne.

Similarly to https://reviews.llvm.org/rL350972 this revision changes 
std::tuple_element from class to struct. Fixes PR#41331.


Repository:
  rCXX libc++

https://reviews.llvm.org/D60069

Files:
  include/__tuple
  include/array
  include/span
  include/tuple
  include/utility
  test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp
  test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.fail.cpp
  test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.pass.cpp
  
test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp

Index: test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
@@ -116,7 +116,7 @@
 int get(Test const&) { static_assert(N == 0, ""); return -1; }
 
 template <>
-class std::tuple_element<0, Test> {
+struct std::tuple_element<0, Test> {
 public:
   typedef int type;
 };
Index: test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.pass.cpp
@@ -11,7 +11,7 @@
 // template  class tuple;
 
 // template 
-// class tuple_element >
+// struct tuple_element >
 // {
 // public:
 // typedef Ti type;
Index: test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.fail.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.fail.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.fail.cpp
@@ -11,7 +11,7 @@
 // template  class tuple;
 
 // template 
-// class tuple_element >
+// struct tuple_element >
 // {
 // public:
 // typedef Ti type;
Index: test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp
@@ -11,7 +11,7 @@
 // template  class tuple;
 
 // template 
-// class tuple_element >
+// struct tuple_element >
 // {
 // public:
 // typedef Ti type;
Index: include/utility
===
--- include/utility
+++ include/utility
@@ -103,7 +103,7 @@
 inline constexpr piecewise_construct_t piecewise_construct = piecewise_construct_t();
 
 template  struct tuple_size;
-template  class tuple_element;
+template  struct tuple_element;
 
 template  struct tuple_size >;
 template  struct tuple_element<0, pair >;
@@ -687,20 +687,20 @@
 : public integral_constant {};
 
 template 
-class _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, pair<_T1, _T2> >
+struct _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, pair<_T1, _T2> >
 {
 static_assert(_Ip < 2, "Index out of bounds in std::tuple_element>");
 };
 
 template 
-class _LIBCPP_TEMPLATE_VIS tuple_element<0, pair<_T1, _T2> >
+struct _LIBCPP_TEMPLATE_VIS tuple_element<0, pair<_T1, _T2> >
 {
 public:
 typedef _T1 type;
 };
 
 template 
-class _LIBCPP_TEMPLATE_VIS tuple_element<1, pair<_T1, _T2> >
+struct _LIBCPP_TEMPLATE_VIS tuple_element<1, pair<_T1, _T2> >
 {
 public:
 typedef _T2 type;
Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -87,8 +87,8 @@
 template  struct tuple_size>;
 template 
  inline constexpr size_t tuple_size_v = tuple_size::value; // C++17
-template  class tuple_element; // undefined
-template  class tuple_element>;
+template  struct tuple_element; // undefined
+template  struct tuple_element>;
 template 
   using tuple_element_t = typename tuple_element ::type; // C++14
 
Index: include/span
===
--- include/span
+++ include/span
@@ -531,7 +531,7 @@
 
 
 template 
-class _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, span<_Tp, _Size>>
+struct _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, span<_Tp, _Size>>
 {
 static_assert( dynamic_extent != _Size, "std::tuple_element<> not supported for std::span");
 static_assert(_Ip < _Size, "Index out of bounds in std::tuple_element<> (std::span)");
Index: include/array
===
--- include/array
+++ include/array
@@ -91,7 +91,7 @@
   void swap(array& x, array& y) noexcept(noexcept(x.swap(y))); // C++17
 
 template  struct tuple_size;
-template  class tuple_element;
+template  struct 

[PATCH] D60069: Declare std::tuple_element as struct instead of class

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

In D60069#1449932 , @mclow.lists wrote:

> In D60069#1449928 , @mclow.lists 
> wrote:
>
> > Did you check the places that inherit from  `tuple_element`? The 
> > public/private bits change between class and struct.
>
>
> Never mind. I was thinking of something else; I don't think that anything 
> inherits from `tuple_element`


Right, there shouldn't be any inheritance. Some of the `public:` access 
specifications are now redundant, though. Instead of

  template 
  struct tuple_element >
  {
  public:
  typedef Ti type;
  };

we could now simply say

  template 
  struct tuple_element >
  {
  typedef Ti type;
  };


Repository:
  rCXX libc++

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

https://reviews.llvm.org/D60069



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

[PATCH] D60069: Declare std::tuple_element as struct instead of class

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie updated this revision to Diff 193106.
jdoerrie added a comment.

Remove redundant `public:` access controls


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

https://reviews.llvm.org/D60069

Files:
  include/__tuple
  include/array
  include/span
  include/tuple
  include/utility
  test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp
  test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.fail.cpp
  test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.pass.cpp
  
test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp

Index: test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_size_structured_bindings.pass.cpp
@@ -116,8 +116,7 @@
 int get(Test const&) { static_assert(N == 0, ""); return -1; }
 
 template <>
-class std::tuple_element<0, Test> {
-public:
+struct std::tuple_element<0, Test> {
   typedef int type;
 };
 
Index: test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.pass.cpp
@@ -11,9 +11,8 @@
 // template  class tuple;
 
 // template 
-// class tuple_element >
+// struct tuple_element >
 // {
-// public:
 // typedef Ti type;
 // };
 
Index: test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.fail.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.fail.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple_element.fail.cpp
@@ -11,9 +11,8 @@
 // template  class tuple;
 
 // template 
-// class tuple_element >
+// struct tuple_element >
 // {
-// public:
 // typedef Ti type;
 // };
 
Index: test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp
===
--- test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp
+++ test/std/utilities/tuple/tuple.tuple/tuple.helper/tuple.include.array.pass.cpp
@@ -11,9 +11,8 @@
 // template  class tuple;
 
 // template 
-// class tuple_element >
+// struct tuple_element >
 // {
-// public:
 // typedef Ti type;
 // };
 //
Index: include/utility
===
--- include/utility
+++ include/utility
@@ -103,7 +103,7 @@
 inline constexpr piecewise_construct_t piecewise_construct = piecewise_construct_t();
 
 template  struct tuple_size;
-template  class tuple_element;
+template  struct tuple_element;
 
 template  struct tuple_size >;
 template  struct tuple_element<0, pair >;
@@ -687,22 +687,20 @@
 : public integral_constant {};
 
 template 
-class _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, pair<_T1, _T2> >
+struct _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, pair<_T1, _T2> >
 {
 static_assert(_Ip < 2, "Index out of bounds in std::tuple_element>");
 };
 
 template 
-class _LIBCPP_TEMPLATE_VIS tuple_element<0, pair<_T1, _T2> >
+struct _LIBCPP_TEMPLATE_VIS tuple_element<0, pair<_T1, _T2> >
 {
-public:
 typedef _T1 type;
 };
 
 template 
-class _LIBCPP_TEMPLATE_VIS tuple_element<1, pair<_T1, _T2> >
+struct _LIBCPP_TEMPLATE_VIS tuple_element<1, pair<_T1, _T2> >
 {
-public:
 typedef _T2 type;
 };
 
Index: include/tuple
===
--- include/tuple
+++ include/tuple
@@ -87,8 +87,8 @@
 template  struct tuple_size>;
 template 
  inline constexpr size_t tuple_size_v = tuple_size::value; // C++17
-template  class tuple_element; // undefined
-template  class tuple_element>;
+template  struct tuple_element; // undefined
+template  struct tuple_element>;
 template 
   using tuple_element_t = typename tuple_element ::type; // C++14
 
Index: include/span
===
--- include/span
+++ include/span
@@ -531,11 +531,10 @@
 
 
 template 
-class _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, span<_Tp, _Size>>
+struct _LIBCPP_TEMPLATE_VIS tuple_element<_Ip, span<_Tp, _Size>>
 {
 static_assert( dynamic_extent != _Size, "std::tuple_element<> not supported for std::span");
 static_assert(_Ip < _Size, "Index out of bounds in std::tuple_element<> (std::span)");
-public:
 typedef _Tp type;
 };
 
Index: include/array
===
--- include/array
+++ include/array
@@ -91,7 +91,7 @@
   void swap(array& x, array& y) noexcept(noexcept(x.swap(y))); // C++17
 
 template  struct tuple_size;
-template  class tuple_element;
+template  struct tuple_element;
 te

[PATCH] D60069: Declare std::tuple_element as struct instead of class

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

In D60069#1449979 , @mclow.lists wrote:

> I'm less enthusiastic about this change than the one for PR39871, because 
> there we were being inconsistent with ourselves.
>  However, my lack of enthusiasm is no reason not to land this.


Thank you for accepting. I understand your point, but you could argue that this 
change fixes a inconsistency between `tuple_size` and `tuple_element`. As I 
don't think I have commit access, could one of you commit this change for me?


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

https://reviews.llvm.org/D60069



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

[PATCH] D32788: Fix std::inplace_merge to be stable for all inputs

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

Thanks for applying a fix based on my patch! I could have sworn the added tests 
failed locally before applying the algorithm patch, though... oh well.


https://reviews.llvm.org/D32788



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

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie created this revision.
jdoerrie added reviewers: mclow.lists, ldionne.
Herald added a reviewer: EricWF.
Herald added subscribers: libcxx-commits, dexonsmith, christof.

[libc++] Disallow dynamic -> static span conversions

This change disallows dynamic to static span conversions. This complies
with the standard, which lists the following constaint in
views.span#span.cons-20.1 [1]:

  Extent == dynamic_extent || Extent == OtherExtent

Thus this should fail if Extent != dynamic_extent && Extent !=
OtherExtent, which is the case when trying to construct a static span
from a dynamic span.

[1] https://eel.is/c++draft/views.span#span.cons-20.1


Repository:
  rCXX libc++

https://reviews.llvm.org/D69520

Files:
  libcxx/include/span
  libcxx/test/std/containers/views/span.cons/span.fail.cpp
  libcxx/test/std/containers/views/span.cons/span.pass.cpp

Index: libcxx/test/std/containers/views/span.cons/span.pass.cpp
===
--- libcxx/test/std/containers/views/span.cons/span.pass.cpp
+++ libcxx/test/std/containers/views/span.cons/span.pass.cpp
@@ -62,48 +62,41 @@
 std::span s4{ vsp0};// a span pointing at volatile int.
 assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
 }
-
-//  dynamic -> static
-{
-std::span s1{  sp};  // a span pointing at int.
-std::span<  volatile int, 0> s2{  sp};  // a span<  volatile int> pointing at int.
-std::span s3{  sp};  // a span pointing at int.
-std::span s4{ vsp};  // a span pointing at volatile int.
-assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
-}
 }
 
 
 template 
 constexpr bool testConstexprSpan()
 {
-std::spans0{};
-std::span s1(s0); // dynamic -> static
-std::spans2(s1); // static -> dynamic
-ASSERT_NOEXCEPT(std::span   {s0});
-ASSERT_NOEXCEPT(std::span{s1});
-ASSERT_NOEXCEPT(std::span   {s1});
-ASSERT_NOEXCEPT(std::span{s0});
-
-return
-s1.data() == nullptr && s1.size() == 0
-&&  s2.data() == nullptr && s2.size() == 0;
+  std::span s0d{};
+  std::span s0s{};
+  std::span s1(s0d);// dynamic -> dynamic
+  std::span s2(s0s); // static -> static
+  std::span s3(s2); // static -> dynamic
+  ASSERT_NOEXCEPT(std::span{s0d});
+  ASSERT_NOEXCEPT(std::span{s0s});
+  ASSERT_NOEXCEPT(std::span{s0s});
+
+  return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
+ s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;
 }
 
 
 template 
 void testRuntimeSpan()
 {
-std::spans0{};
-std::span s1(s0); // dynamic -> static
-std::spans2(s1); // static -> dynamic
-ASSERT_NOEXCEPT(std::span   {s0});
-ASSERT_NOEXCEPT(std::span{s1});
-ASSERT_NOEXCEPT(std::span   {s1});
-ASSERT_NOEXCEPT(std::span{s0});
-
-assert(s1.data() == nullptr && s1.size() == 0);
-assert(s2.data() == nullptr && s2.size() == 0);
+  std::span s0d{};
+  std::span s0s{};
+  std::span s1(s0d);// dynamic -> dynamic
+  std::span s2(s0s); // static -> static
+  std::span s3(s2); // static -> dynamic
+  ASSERT_NOEXCEPT(std::span{s0d});
+  ASSERT_NOEXCEPT(std::span{s0s});
+  ASSERT_NOEXCEPT(std::span{s0s});
+
+  assert(s1.data() == nullptr && s1.size() == 0);
+  assert(s2.data() == nullptr && s2.size() == 0);
+  assert(s3.data() == nullptr && s3.size() == 0);
 }
 
 
@@ -113,10 +106,12 @@
 static_assert(std::is_convertible_v, "Bad input types to 'testConversionSpan");
 std::spans0d{};
 std::spans0s{};
-std::span s1(s0d); // dynamic -> static
-std::spans2(s0s); // static -> dynamic
-s1.data() == nullptr && s1.size() == 0
-&&  s2.data() == nullptr && s2.size() == 0;
+std::span s1(s0d);// dynamic -> dynamic
+std::span s2(s0s); // static -> static
+std::span s3(s0d);// static -> dynamic
+s1.data() == nullptr&& s1.size() == 0 &&
+s2.data() == nullptr&& s2.size() == 0 &&
+s3.data() == nullptr&& s3.size() == 0;
 }
 
 struct A{};
Index: libcxx/test/std/containers/views/span.cons/span.fail.cpp
===
--- libcxx/test/std/containers/views/span.cons/span.fail.cpp
+++ libcxx/test/std/containers/views/span.cons/span.fail.cpp
@@ -74,19 +74,6 @@
 std::span<  volatile int> s6{ csp0}; // expected-error {{no matching constructor for initialization of 'std::span'}}
 std::span<  volatile int> s7{cvsp0}; // expected-error {{no matching constructor for initialization of 'std::span'}}
 }
-
-//  Try to remove const and/or volatile (static -> static)
-{
-std::span<   int, 0> s1{ csp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-std::span<   int, 0> s2{ vsp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-std::span<   int, 0> s3{cvsp}; // expected-error {{no matching constr

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie updated this revision to Diff 226713.
jdoerrie added a comment.

Formatting


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

https://reviews.llvm.org/D69520

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


Index: libcxx/test/std/containers/views/span.cons/span.pass.cpp
===
--- libcxx/test/std/containers/views/span.cons/span.pass.cpp
+++ libcxx/test/std/containers/views/span.cons/span.pass.cpp
@@ -105,13 +105,12 @@
 {
 static_assert(std::is_convertible_v, "Bad input types 
to 'testConversionSpan");
 std::spans0d{};
-std::spans0s{};
+std::span s0s{};
 std::span s1(s0d);// dynamic -> dynamic
 std::span s2(s0s); // static -> static
 std::span s3(s0d);// static -> dynamic
-s1.data() == nullptr&& s1.size() == 0 &&
-s2.data() == nullptr&& s2.size() == 0 &&
-s3.data() == nullptr&& s3.size() == 0;
+return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
+   s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;
 }
 
 struct A{};


Index: libcxx/test/std/containers/views/span.cons/span.pass.cpp
===
--- libcxx/test/std/containers/views/span.cons/span.pass.cpp
+++ libcxx/test/std/containers/views/span.cons/span.pass.cpp
@@ -105,13 +105,12 @@
 {
 static_assert(std::is_convertible_v, "Bad input types to 'testConversionSpan");
 std::spans0d{};
-std::spans0s{};
+std::span s0s{};
 std::span s1(s0d);// dynamic -> dynamic
 std::span s2(s0s); // static -> static
 std::span s3(s0d);// static -> dynamic
-s1.data() == nullptr&& s1.size() == 0 &&
-s2.data() == nullptr&& s2.size() == 0 &&
-s3.data() == nullptr&& s3.size() == 0;
+return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
+   s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;
 }
 
 struct A{};
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie updated this revision to Diff 226714.
jdoerrie added a comment.

Full patch


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

https://reviews.llvm.org/D69520

Files:
  libcxx/include/span
  libcxx/test/std/containers/views/span.cons/span.fail.cpp
  libcxx/test/std/containers/views/span.cons/span.pass.cpp

Index: libcxx/test/std/containers/views/span.cons/span.pass.cpp
===
--- libcxx/test/std/containers/views/span.cons/span.pass.cpp
+++ libcxx/test/std/containers/views/span.cons/span.pass.cpp
@@ -62,48 +62,41 @@
 std::span s4{ vsp0};// a span pointing at volatile int.
 assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
 }
-
-//  dynamic -> static
-{
-std::span s1{  sp};  // a span pointing at int.
-std::span<  volatile int, 0> s2{  sp};  // a span<  volatile int> pointing at int.
-std::span s3{  sp};  // a span pointing at int.
-std::span s4{ vsp};  // a span pointing at volatile int.
-assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
-}
 }
 
 
 template 
 constexpr bool testConstexprSpan()
 {
-std::spans0{};
-std::span s1(s0); // dynamic -> static
-std::spans2(s1); // static -> dynamic
-ASSERT_NOEXCEPT(std::span   {s0});
-ASSERT_NOEXCEPT(std::span{s1});
-ASSERT_NOEXCEPT(std::span   {s1});
-ASSERT_NOEXCEPT(std::span{s0});
-
-return
-s1.data() == nullptr && s1.size() == 0
-&&  s2.data() == nullptr && s2.size() == 0;
+  std::span s0d{};
+  std::span s0s{};
+  std::span s1(s0d);// dynamic -> dynamic
+  std::span s2(s0s); // static -> static
+  std::span s3(s2); // static -> dynamic
+  ASSERT_NOEXCEPT(std::span{s0d});
+  ASSERT_NOEXCEPT(std::span{s0s});
+  ASSERT_NOEXCEPT(std::span{s0s});
+
+  return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
+ s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;
 }
 
 
 template 
 void testRuntimeSpan()
 {
-std::spans0{};
-std::span s1(s0); // dynamic -> static
-std::spans2(s1); // static -> dynamic
-ASSERT_NOEXCEPT(std::span   {s0});
-ASSERT_NOEXCEPT(std::span{s1});
-ASSERT_NOEXCEPT(std::span   {s1});
-ASSERT_NOEXCEPT(std::span{s0});
-
-assert(s1.data() == nullptr && s1.size() == 0);
-assert(s2.data() == nullptr && s2.size() == 0);
+  std::span s0d{};
+  std::span s0s{};
+  std::span s1(s0d);// dynamic -> dynamic
+  std::span s2(s0s); // static -> static
+  std::span s3(s2); // static -> dynamic
+  ASSERT_NOEXCEPT(std::span{s0d});
+  ASSERT_NOEXCEPT(std::span{s0s});
+  ASSERT_NOEXCEPT(std::span{s0s});
+
+  assert(s1.data() == nullptr && s1.size() == 0);
+  assert(s2.data() == nullptr && s2.size() == 0);
+  assert(s3.data() == nullptr && s3.size() == 0);
 }
 
 
@@ -112,11 +105,12 @@
 {
 static_assert(std::is_convertible_v, "Bad input types to 'testConversionSpan");
 std::spans0d{};
-std::spans0s{};
-std::span s1(s0d); // dynamic -> static
-std::spans2(s0s); // static -> dynamic
-s1.data() == nullptr && s1.size() == 0
-&&  s2.data() == nullptr && s2.size() == 0;
+std::span s0s{};
+std::span s1(s0d);// dynamic -> dynamic
+std::span s2(s0s); // static -> static
+std::span s3(s0d);// static -> dynamic
+return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
+   s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;
 }
 
 struct A{};
Index: libcxx/test/std/containers/views/span.cons/span.fail.cpp
===
--- libcxx/test/std/containers/views/span.cons/span.fail.cpp
+++ libcxx/test/std/containers/views/span.cons/span.fail.cpp
@@ -74,19 +74,6 @@
 std::span<  volatile int> s6{ csp0}; // expected-error {{no matching constructor for initialization of 'std::span'}}
 std::span<  volatile int> s7{cvsp0}; // expected-error {{no matching constructor for initialization of 'std::span'}}
 }
-
-//  Try to remove const and/or volatile (static -> static)
-{
-std::span<   int, 0> s1{ csp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-std::span<   int, 0> s2{ vsp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-std::span<   int, 0> s3{cvsp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-
-std::span s4{ vsp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-std::span s5{cvsp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-
-std::span<  volatile int, 0> s6{ csp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-std::span<  volatile int, 0> s7{cvsp}; // expected-error {{no matching constructor for initialization of 'std:

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

Friendly Ping :)


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

https://reviews.llvm.org/D69520



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

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

Thanks Jorg! Given that the status is still "Needs Review", I assume I also 
still need an LGTM from Marshall, Louis or Eric, right?


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

https://reviews.llvm.org/D69520



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

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie updated this revision to Diff 227605.
jdoerrie marked 4 inline comments as done.
jdoerrie added a comment.

Addressed Marshall's comments


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

https://reviews.llvm.org/D69520

Files:
  libcxx/include/span
  libcxx/test/std/containers/views/span.cons/span.fail.cpp
  libcxx/test/std/containers/views/span.cons/span.pass.cpp

Index: libcxx/test/std/containers/views/span.cons/span.pass.cpp
===
--- libcxx/test/std/containers/views/span.cons/span.pass.cpp
+++ libcxx/test/std/containers/views/span.cons/span.pass.cpp
@@ -62,48 +62,42 @@
 std::span s4{ vsp0};// a span pointing at volatile int.
 assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
 }
-
-//  dynamic -> static
-{
-std::span s1{  sp};  // a span pointing at int.
-std::span<  volatile int, 0> s2{  sp};  // a span<  volatile int> pointing at int.
-std::span s3{  sp};  // a span pointing at int.
-std::span s4{ vsp};  // a span pointing at volatile int.
-assert(s1.size() + s2.size() + s3.size() + s4.size() == 0);
-}
 }
 
 
 template 
 constexpr bool testConstexprSpan()
 {
-std::spans0{};
-std::span s1(s0); // dynamic -> static
-std::spans2(s1); // static -> dynamic
-ASSERT_NOEXCEPT(std::span   {s0});
-ASSERT_NOEXCEPT(std::span{s1});
-ASSERT_NOEXCEPT(std::span   {s1});
-ASSERT_NOEXCEPT(std::span{s0});
-
-return
-s1.data() == nullptr && s1.size() == 0
-&&  s2.data() == nullptr && s2.size() == 0;
+  std::span s0d{};
+  std::span s0s{};
+  std::span s1(s0d);// dynamic -> dynamic
+  std::span s2(s0s); // static -> static
+  std::span s3(s2); // static -> dynamic
+  ASSERT_NOEXCEPT(std::span{s0d});
+  ASSERT_NOEXCEPT(std::span{s0s});
+  ASSERT_NOEXCEPT(std::span{s0s});
+
+  return s1.data() == nullptr && s1.size() == 0
+  && s2.data() == nullptr && s2.size() == 0
+  && s3.data() == nullptr && s3.size() == 0;
 }
 
 
 template 
 void testRuntimeSpan()
 {
-std::spans0{};
-std::span s1(s0); // dynamic -> static
-std::spans2(s1); // static -> dynamic
-ASSERT_NOEXCEPT(std::span   {s0});
-ASSERT_NOEXCEPT(std::span{s1});
-ASSERT_NOEXCEPT(std::span   {s1});
-ASSERT_NOEXCEPT(std::span{s0});
-
-assert(s1.data() == nullptr && s1.size() == 0);
-assert(s2.data() == nullptr && s2.size() == 0);
+  std::span s0d{};
+  std::span s0s{};
+  std::span s1(s0d);// dynamic -> dynamic
+  std::span s2(s0s); // static -> static
+  std::span s3(s2); // static -> dynamic
+  ASSERT_NOEXCEPT(std::span{s0d});
+  ASSERT_NOEXCEPT(std::span{s0s});
+  ASSERT_NOEXCEPT(std::span{s0s});
+
+  assert(s1.data() == nullptr && s1.size() == 0);
+  assert(s2.data() == nullptr && s2.size() == 0);
+  assert(s3.data() == nullptr && s3.size() == 0);
 }
 
 
@@ -112,11 +106,13 @@
 {
 static_assert(std::is_convertible_v, "Bad input types to 'testConversionSpan");
 std::spans0d{};
-std::spans0s{};
-std::span s1(s0d); // dynamic -> static
-std::spans2(s0s); // static -> dynamic
-s1.data() == nullptr && s1.size() == 0
-&&  s2.data() == nullptr && s2.size() == 0;
+std::span s0s{};
+std::span s1(s0d);// dynamic -> dynamic
+std::span s2(s0s); // static -> static
+std::span s3(s0d);// static -> dynamic
+return s1.data() == nullptr && s1.size() == 0
+&& s2.data() == nullptr && s2.size() == 0
+&& s3.data() == nullptr && s3.size() == 0;
 }
 
 struct A{};
Index: libcxx/test/std/containers/views/span.cons/span.fail.cpp
===
--- libcxx/test/std/containers/views/span.cons/span.fail.cpp
+++ libcxx/test/std/containers/views/span.cons/span.fail.cpp
@@ -74,18 +74,34 @@
 std::span<  volatile int> s6{ csp0}; // expected-error {{no matching constructor for initialization of 'std::span'}}
 std::span<  volatile int> s7{cvsp0}; // expected-error {{no matching constructor for initialization of 'std::span'}}
 }
+}
 
-//  Try to remove const and/or volatile (static -> static)
+void checkExtent ()
+{
+std::spanspd;
+std::span sp0;
+std::span sp1{nullptr, 1};
+std::span sp2{nullptr, 2};
+
+//  Try to construct static span with extent 0 from span with different extent
 {
-std::span<   int, 0> s1{ csp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-std::span<   int, 0> s2{ vsp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
-std::span<   int, 0> s3{cvsp}; // expected-error {{no matching constructor for initialization of 'std::span'}}
+std::span s1{spd}; // expected-error {{no matching constructor for initialization of 'std::span'}}
+std::span s2{sp1}; // expected-error {{no matching constructor 

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added inline comments.



Comment at: libcxx/test/std/containers/views/span.cons/span.fail.cpp:78
-
-//  Try to remove const and/or volatile (static -> static)
-{

mclow.lists wrote:
> Ok. The comment here is wrong; this is testing dynamic -> static.
> 
> However, why are you removing these (failing) tests?
> 
Considering that `Extent == dynamic_extent || Extent == OtherExtent` should be 
checked first I felt these particular tests distracted from the actual root 
cause of the error. I added now a new section that performs explicit Extent 
checks.



Comment at: libcxx/test/std/containers/views/span.cons/span.pass.cpp:80
+
+  return s1.data() == nullptr && s1.size() == 0 && s2.data() == nullptr &&
+ s2.size() == 0 && s3.data() == nullptr && s3.size() == 0;

mclow.lists wrote:
> Please line these up like the other ones were. Makes it easy to see 
> copy-pasta errors:
> ```
> return s1.data() == nullptr && s1.size() == 0 
> && s2.data() == nullptr && s2.size() == 0
> && s3.data() == nullptr && s3.size() == 0;
> ```
FWIW my formatting was performed by `clang-format`. Do you think it's worth 
wrapping this block in

```
// clang-format off
...
// clang-format on
```

so that this doesn't regress in the future?


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

https://reviews.llvm.org/D69520



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

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

Friendly Ping


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

https://reviews.llvm.org/D69520



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

[PATCH] D69520: [libc++] Disallow dynamic -> static span conversions

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

Oh interesting! Yes, it looks like you are right.


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

https://reviews.llvm.org/D69520



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

[PATCH] D32788: Fix std::inplace_merge to be stable for all inputs

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie created this revision.

This change fixes std::inplace_merge to be stable for all inputs and adds 
corresponding tests.

Fixes Bug 31166: https://bugs.llvm.org//show_bug.cgi?id=31166


https://reviews.llvm.org/D32788

Files:
  include/algorithm
  test/std/algorithms/alg.sorting/alg.merge/inplace_merge_comp.pass.cpp


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
@@ -32,6 +32,14 @@
 {return *x < *y;}
 };
 
+struct first_only
+{
+bool operator()(const std::pair& x, const std::pair& y)
+{
+return x.first < y.first;
+}
+};
+
 struct S {
S() : i_(0) {}
S(int i) : i_(i) {}
@@ -85,6 +93,23 @@
 delete [] ia;
 }
 
+void
+test_two(unsigned N, unsigned M)
+{
+typedef std::pair P;
+std::vector v(N);
+unsigned mod = std::max(M, N - M);
+for (unsigned i = 0; i < N; ++i)
+{
+v[i] = P(i % mod, i >= M);
+}
+
+std::stable_sort(v.begin(), v.begin() + M, first_only());
+std::stable_sort(v.begin() + M, v.end(), first_only());
+std::inplace_merge(v.begin(), v.begin() + M, v.end(), first_only());
+assert(std::is_sorted(v.begin(), v.end()));
+}
+
 template 
 void
 test(unsigned N)
@@ -94,6 +119,12 @@
 test_one(N, N/2);
 test_one(N, 3*N/4);
 test_one(N, N);
+
+test_two(N, 0);
+test_two(N, N/4);
+test_two(N, N/2);
+test_two(N, 3*N/4);
+test_two(N, N);
 }
 
 template 
@@ -110,6 +141,18 @@
 test_one(3, 1);
 test_one(3, 2);
 test_one(3, 3);
+
+test_two(0, 0);
+test_two(1, 0);
+test_two(1, 1);
+test_two(2, 0);
+test_two(2, 1);
+test_two(2, 2);
+test_two(3, 0);
+test_two(3, 1);
+test_two(3, 2);
+test_two(3, 3);
+
 test(4);
 test(20);
 test(100);
Index: include/algorithm
===
--- include/algorithm
+++ include/algorithm
@@ -745,7 +745,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-bool operator()(const _T1& __x, const _T2& __y) {return !__p_(__x, __y);}
+bool operator()(const _T1& __x, const _T2& __y) {return __p_(__y, __x);}
 };
 
 #ifdef _LIBCPP_DEBUG


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
@@ -32,6 +32,14 @@
 {return *x < *y;}
 };
 
+struct first_only
+{
+bool operator()(const std::pair& x, const std::pair& y)
+{
+return x.first < y.first;
+}
+};
+
 struct S {
 	S() : i_(0) {}
 	S(int i) : i_(i) {}
@@ -85,6 +93,23 @@
 delete [] ia;
 }
 
+void
+test_two(unsigned N, unsigned M)
+{
+typedef std::pair P;
+std::vector v(N);
+unsigned mod = std::max(M, N - M);
+for (unsigned i = 0; i < N; ++i)
+{
+v[i] = P(i % mod, i >= M);
+}
+
+std::stable_sort(v.begin(), v.begin() + M, first_only());
+std::stable_sort(v.begin() + M, v.end(), first_only());
+std::inplace_merge(v.begin(), v.begin() + M, v.end(), first_only());
+assert(std::is_sorted(v.begin(), v.end()));
+}
+
 template 
 void
 test(unsigned N)
@@ -94,6 +119,12 @@
 test_one(N, N/2);
 test_one(N, 3*N/4);
 test_one(N, N);
+
+test_two(N, 0);
+test_two(N, N/4);
+test_two(N, N/2);
+test_two(N, 3*N/4);
+test_two(N, N);
 }
 
 template 
@@ -110,6 +141,18 @@
 test_one(3, 1);
 test_one(3, 2);
 test_one(3, 3);
+
+test_two(0, 0);
+test_two(1, 0);
+test_two(1, 1);
+test_two(2, 0);
+test_two(2, 1);
+test_two(2, 2);
+test_two(3, 0);
+test_two(3, 1);
+test_two(3, 2);
+test_two(3, 3);
+
 test(4);
 test(20);
 test(100);
Index: include/algorithm
===
--- include/algorithm
+++ include/algorithm
@@ -745,7 +745,7 @@
 
 template 
 _LIBCPP_INLINE_VISIBILITY
-bool operator()(const _T1& __x, const _T2& __y) {return !__p_(__x, __y);}
+bool operator()(const _T1& __x, const _T2& __y) {return __p_(__y, __x);}
 };
 
 #ifdef _LIBCPP_DEBUG
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D32788: Fix std::inplace_merge to be stable for all inputs

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added inline comments.



Comment at: include/algorithm:4471
  _RBi(__middle), _RBi(__first),
  _RBi(__last), __negate<_Compare>(__comp));
 }

As marshall pointed out in https://bugs.llvm.org//show_bug.cgi?id=31166 this is 
where the problem lies. While reversing the input ranges is a very nifty trick 
to be able to use the moved out space at the end of `[first, last)`, simply 
negating the comparison functor is not enough to preserve stability. 

Assuming a default `__comp` of `std::less`, in line 4431 a negated `__comp` 
will evaluate to true for equivalent elements, resulting in a move from 
`*__first2` and thus breaking stability. The fix for this to to still have 
`__comp` evaluate to false for equivalent elements, what the change in 748 
accomplishes.

Given that now `__negate` technically does not negate anymore, maybe it should 
be renamed to a more suiting `__reverse` or `__swap` or similar. Doing a quick 
grep over the code base line 4471 seems to be the only place where this is 
used, so a rename should not have further consequences. Furthermore, the unary 
`bool operator()` could be deleted.


https://reviews.llvm.org/D32788



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

[PATCH] D32788: Fix std::inplace_merge to be stable for all inputs

[Jan Wilken Dörrie via Phabricator via cfe-commits]

jdoerrie added a comment.

Ping, please take a look at this.


https://reviews.llvm.org/D32788



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