[libcxx-commits] [PATCH] D143914: [libc++] Clean up pair's constructors and assignment operators

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added subscribers: huixie90, tcanens, libc++ vendors.
ldionne added a comment.

Pinging vendors since this is technically a source break. I'll also take this for a run internally to see how much breakage this causes, if any.



================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/assign.pair_like_rv.pass.cpp:80
+    static_assert(!std::is_assignable_v<std::pair<long, std::string>&, std::tuple<long, void*>>); // second not convertible
+    static_assert( std::is_assignable_v<std::pair<long, std::string>&, std::tuple<long, std::string>>); // works (test the test)
+  }
----------------
@huixie90 @tcanens 

While implementing this, I found out that stuff like this would compile just fine:

```
std::pair<long, std::string> p;
p = std::tuple<long, float>{};
```

This is because `std::string` can be assigned-to from `float`. While this has nothing to do with `pair` specifically, this seems completely bonkers to me -- this is almost Javascript bad. I'm curious to know whether that has been discussed and acknowledged?


================
Comment at: libcxx/test/std/utilities/utility/pairs/pairs.pair/assign.pair_like_rv_const.pass.cpp:57
+
+    // Make sure it doesn't work for `ranges::subrange`, since it is called out explicitly.
+    {
----------------
@tcanens @huixie90 

This behavior seems to be inconsistent with the non-const-qualified `operator=` from `pair-like`. For `operator=(PairLike)` (not `const`), it seems like we go through `subrange`'s `operator pair-like()`, which makes this work. But with `operator=(pair-like) const`, it looks like this conversion doesn't trigger and we end up not being able to perform the assignment. Does that match the LWG design intent?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143914