[PATCH] D14839: [libcxx] LWG2485: get() should be overloaded for const tuple&&

[Eric Fiselier via cfe-commits]

EricWF added a comment.

Added initial review comments.

I need to look a little more into how we form the rvalue reference to the returned element in the case where the element is an lvalue reference. I think it's currently correct but I just want to double check. I think the new language is a bit vague when it says `get returns a reference to the element`.


================
Comment at: test/std/containers/sequences/array/array.tuple/get_const_rv.pass.cpp:32
@@ +31,3 @@
+        const C c = {std::unique_ptr<double>(new double(3.5))};
+        const T&& t = std::get<0>(std::move(c));
+        assert(*t == 3.5);
----------------
1. Please test the constexprness of this function in C++14 and beyond.
2. Please `static_assert` the return type using `decltype`. The assignment to the expected type might hide conversions (though I doubt it in this case).

================
Comment at: test/std/utilities/tuple/tuple.tuple/tuple.elem/get_const_rv.fail.cpp:30
@@ +29,3 @@
+    {
+        cref(std::get<0>(tup4()));
+    }
----------------
Add `// expected-error {{call to deleted function 'cref'}}` to the end of this line to make the test use clang verify.

================
Comment at: test/std/utilities/tuple/tuple.tuple/tuple.elem/get_const_rv.pass.cpp:26
@@ +25,3 @@
+struct Empty {};
+
+int main()
----------------
Same note as on the array test.

================
Comment at: test/std/utilities/utility/pairs/pair.astuple/get_const_rv.pass.cpp:24
@@ +23,3 @@
+{
+#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+    {
----------------
I prefer  using `// UNSUPPORTED: c++98, c++03` instead of conditional compilation. That way LIT can report that the test was not run.

================
Comment at: test/std/utilities/utility/pairs/pair.astuple/get_const_rv.pass.cpp:25
@@ +24,3 @@
+#ifndef _LIBCPP_HAS_NO_RVALUE_REFERENCES
+    {
+        typedef std::pair<std::unique_ptr<int>, short> P;
----------------
Same note as the array and tuple tests.

================
Comment at: test/std/utilities/utility/pairs/pair.astuple/pairs.by.type.pass.cpp:43
@@ -42,1 +42,3 @@
 
+    {
+    typedef std::unique_ptr<int> upint;
----------------
This test seems wrong to me. The name of the file suggests we are testing the "by-type" overloads but your tests use indexes with `get`. It seems like the test above yours does the same thing as well.

Also please add constexpr tests and tests for the second type as well. 


http://reviews.llvm.org/D14839