Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make TcUnionPieceView movable to satisfy std::ranges::viewable_range #9567

Conversation

mszabo-wikia
Copy link
Contributor

Building type-constraint.h with clang 18 + libc++ 18 currently fails, because TcUnionPieceView isn't movable:

In file included from /hhvm/hphp/system/systemlib.cpp:19:
In file included from /hhvm/hphp/runtime/base/array-init.h:21:
In file included from /hhvm/hphp/runtime/base/array-data-defs.h:23:
In file included from /hhvm/hphp/runtime/base/type-variant.h:20:
In file included from /hhvm/hphp/runtime/base/tv-conversions.h:23:
In file included from /hhvm/hphp/runtime/base/type-object.h:19:
In file included from /hhvm/hphp/runtime/base/object-data.h:27:
In file included from /hhvm/hphp/runtime/vm/class.h:32:
In file included from /hhvm/hphp/runtime/vm/preclass.h:28:
/hhvm/hphp/runtime/vm/type-constraint.h:996:15: error: static assertion failed
  996 | static_assert(std::ranges::viewable_range<HPHP::TcUnionPieceView>);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/hhvm/hphp/runtime/vm/type-constraint.h:996:15: note: because 'HPHP::TcUnionPieceView' does not satisfy 'viewable_range'
/usr/bin/../include/c++/v1/__ranges/concepts.h:131:7: note: because 'remove_cvref_t<TcUnionPieceView>' (aka 'HPHP::TcUnionPieceView') does not satisfy 'view'
  131 |     ((view<remove_cvref_t<_Tp>> && constructible_from<remove_cvref_t<_Tp>, _Tp>) ||
      |       ^
/usr/bin/../include/c++/v1/__ranges/concepts.h:94:30: note: because 'HPHP::TcUnionPieceView' does not satisfy 'movable'
   94 | concept view = range<_Tp> && movable<_Tp> && enable_view<_Tp>;
      |                              ^
/usr/bin/../include/c++/v1/__concepts/movable.h:29:66: note: because 'assignable_from<HPHP::TcUnionPieceView &, HPHP::TcUnionPieceView>' evaluated to false
   29 | concept movable = is_object_v<_Tp> && move_constructible<_Tp> && assignable_from<_Tp&, _Tp> && swappable<_Tp>;
      |                                                                  ^
/usr/bin/../include/c++/v1/__concepts/assignable.h:34:15: note: because '__lhs = std::forward<_Rhs>(__rhs)' would be invalid: object of type 'HPHP::TcUnionPieceView' cannot be assigned because its copy assignment operator is implicitly deleted
   34 |       { __lhs = std::forward<_Rhs>(__rhs) } -> same_as<_Lhs>;
      |               ^
/usr/bin/../include/c++/v1/__ranges/concepts.h:133:8: note: and 'is_lvalue_reference_v<HPHP::TcUnionPieceView>' evaluated to false
  133 |       (is_lvalue_reference_v<_Tp> ||
      |        ^
/usr/bin/../include/c++/v1/__ranges/concepts.h:134:9: note: and 'remove_reference_t<TcUnionPieceView>' (aka 'HPHP::TcUnionPieceView') does not satisfy 'movable'
  134 |        (movable<remove_reference_t<_Tp>> && !__is_std_initializer_list<remove_cvref_t<_Tp>>))));
      |         ^
/usr/bin/../include/c++/v1/__concepts/movable.h:29:66: note: because 'assignable_from<HPHP::TcUnionPieceView &, HPHP::TcUnionPieceView>' evaluated to false
   29 | concept movable = is_object_v<_Tp> && move_constructible<_Tp> && assignable_from<_Tp&, _Tp> && swappable<_Tp>;
      |                                                                  ^
/usr/bin/../include/c++/v1/__concepts/assignable.h:34:15: note: because '__lhs = std::forward<_Rhs>(__rhs)' would be invalid: object of type 'HPHP::TcUnionPieceView' cannot be assigned because its copy assignment operator is implicitly deleted
   34 |       { __lhs = std::forward<_Rhs>(__rhs) } -> same_as<_Lhs>;

The same error also occurs with GCC 14.

So, make the members non-const to satisfy std::movable.

Split from #9564.

Building type-constraint.h with clang 18 + libc++ 18 currently fails,
because TcUnionPieceView isn't movable:

```
In file included from /hhvm/hphp/system/systemlib.cpp:19:
In file included from /hhvm/hphp/runtime/base/array-init.h:21:
In file included from /hhvm/hphp/runtime/base/array-data-defs.h:23:
In file included from /hhvm/hphp/runtime/base/type-variant.h:20:
In file included from /hhvm/hphp/runtime/base/tv-conversions.h:23:
In file included from /hhvm/hphp/runtime/base/type-object.h:19:
In file included from /hhvm/hphp/runtime/base/object-data.h:27:
In file included from /hhvm/hphp/runtime/vm/class.h:32:
In file included from /hhvm/hphp/runtime/vm/preclass.h:28:
/hhvm/hphp/runtime/vm/type-constraint.h:996:15: error: static assertion failed
  996 | static_assert(std::ranges::viewable_range<HPHP::TcUnionPieceView>);
      |               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/hhvm/hphp/runtime/vm/type-constraint.h:996:15: note: because 'HPHP::TcUnionPieceView' does not satisfy 'viewable_range'
/usr/bin/../include/c++/v1/__ranges/concepts.h:131:7: note: because 'remove_cvref_t<TcUnionPieceView>' (aka 'HPHP::TcUnionPieceView') does not satisfy 'view'
  131 |     ((view<remove_cvref_t<_Tp>> && constructible_from<remove_cvref_t<_Tp>, _Tp>) ||
      |       ^
/usr/bin/../include/c++/v1/__ranges/concepts.h:94:30: note: because 'HPHP::TcUnionPieceView' does not satisfy 'movable'
   94 | concept view = range<_Tp> && movable<_Tp> && enable_view<_Tp>;
      |                              ^
/usr/bin/../include/c++/v1/__concepts/movable.h:29:66: note: because 'assignable_from<HPHP::TcUnionPieceView &, HPHP::TcUnionPieceView>' evaluated to false
   29 | concept movable = is_object_v<_Tp> && move_constructible<_Tp> && assignable_from<_Tp&, _Tp> && swappable<_Tp>;
      |                                                                  ^
/usr/bin/../include/c++/v1/__concepts/assignable.h:34:15: note: because '__lhs = std::forward<_Rhs>(__rhs)' would be invalid: object of type 'HPHP::TcUnionPieceView' cannot be assigned because its copy assignment operator is implicitly deleted
   34 |       { __lhs = std::forward<_Rhs>(__rhs) } -> same_as<_Lhs>;
      |               ^
/usr/bin/../include/c++/v1/__ranges/concepts.h:133:8: note: and 'is_lvalue_reference_v<HPHP::TcUnionPieceView>' evaluated to false
  133 |       (is_lvalue_reference_v<_Tp> ||
      |        ^
/usr/bin/../include/c++/v1/__ranges/concepts.h:134:9: note: and 'remove_reference_t<TcUnionPieceView>' (aka 'HPHP::TcUnionPieceView') does not satisfy 'movable'
  134 |        (movable<remove_reference_t<_Tp>> && !__is_std_initializer_list<remove_cvref_t<_Tp>>))));
      |         ^
/usr/bin/../include/c++/v1/__concepts/movable.h:29:66: note: because 'assignable_from<HPHP::TcUnionPieceView &, HPHP::TcUnionPieceView>' evaluated to false
   29 | concept movable = is_object_v<_Tp> && move_constructible<_Tp> && assignable_from<_Tp&, _Tp> && swappable<_Tp>;
      |                                                                  ^
/usr/bin/../include/c++/v1/__concepts/assignable.h:34:15: note: because '__lhs = std::forward<_Rhs>(__rhs)' would be invalid: object of type 'HPHP::TcUnionPieceView' cannot be assigned because its copy assignment operator is implicitly deleted
   34 |       { __lhs = std::forward<_Rhs>(__rhs) } -> same_as<_Lhs>;
```

The same error also occurs with GCC 14.

So, make the members non-const to satisfy std::movable.
@facebook-github-bot
Copy link
Contributor

@facebook-github-bot has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. (Because this pull request was imported automatically, there will not be any future comments.)

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in e80a85e.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants