[PATCH] D49338: Implement <span> - P0122R7

[Stephan T. Lavavej via Phabricator via cfe-commits]

STL_MSFT added inline comments.


================
Comment at: test/std/containers/views/span.comparison/op.eq.pass.cpp:23
+#include <cassert>
+#include <string>
+
----------------
The comparison tests appear to be unnecessarily including `<string>`.


================
Comment at: test/std/containers/views/span.cons/assign.pass.cpp:25
+{
+    static_assert(noexcept(std::declval<T&>() = rhs), "");
+    lhs = rhs;
----------------
Technically need `<utility>` for declval.


================
Comment at: test/std/containers/views/span.cons/assign.pass.cpp:73
+
+//  No for loops in constexpr land :-(
+        static_assert(doAssign(spans[0], spans[0]), "");
----------------
span is C++20, so you can use C++14 extended constexpr's for-loops. Just write a helper function.


================
Comment at: test/std/containers/views/span.cons/container.fail.cpp:44
+
+    constexpr T const *getV() const {return &v_;} // for checking
+    T v_;
----------------
This file is inconsistent about left vs right const (see line immediately above).


================
Comment at: test/std/containers/views/span.cons/deduct.pass.cpp:45
+    using S = decltype(s);
+    static_assert(std::is_same_v<S, std::span<int, 3>>, "");
+    assert((std::equal(std::begin(arr), std::end(arr), s.begin(), s.end())));
----------------
Technically need `<type_traits>` for is_same_v.


================
Comment at: test/std/containers/views/span.cons/default.pass.cpp:61
+    std::span<const T, 0> s2;
+    return
+    assert(s1.data() == nullptr && s1.size() == 0);
----------------
This return appears to be spurious.


================
Comment at: test/std/containers/views/span.cons/span.pass.cpp:96
+    std::span<T>    s2(s1); // static -> dynamic
+    return
+    assert(s1.data() == nullptr && s1.size() == 0);
----------------
Spurious return occurs in multiple files.


================
Comment at: test/std/containers/views/span.iterators/begin.pass.cpp:31
+    {
+        ret &= ( b ==  s.end());
+        ret &= (cb == s.cend());
----------------
Using bitwise &= on a bool isn't very cromulent.


================
Comment at: test/std/containers/views/span.iterators/rbegin.pass.cpp:115
+    std::string s;
+    testRuntimeSpan(std::span<std::string>(&s, (std::ptrdiff_t) 0));
+    testRuntimeSpan(std::span<std::string>(&s, 1));
----------------
static_cast would be nicer than C cast.


================
Comment at: test/std/containers/views/span.objectrep/as_bytes.pass.cpp:31
+{
+    static_assert(noexcept(std::as_bytes(sp)));
+
----------------
This is a C++17 terse static assert unlike your others. Should you consistently use this form, or the other one?


================
Comment at: test/std/containers/views/span.objectrep/as_writeable_bytes.pass.cpp:42
+        
+    assert((void *) spBytes.data() == (void *) sp.data());
+    assert(spBytes.size() == sp.size_bytes());
----------------
static_cast?


================
Comment at: test/std/containers/views/span.sub/last.pass.cpp:44
+     && s1.size() == s2.size()
+     && std::equal(s1.begin(), s1.end(), sp.end() - Count);
+}
----------------
Need `<algorithm>`.


================
Comment at: test/std/containers/views/span.sub/subspan.pass.cpp:45
+     && s1.size() == s2.size()
+     && std::equal(s1.begin(), s1.end(), sp.begin() + Offset);
+}
----------------
Also need `<algorithm>`, presumably occurs in more files.


================
Comment at: test/std/containers/views/types.pass.cpp:41
+{
+    typedef std::iterator_traits<Iter> ItT;
+
----------------
Need `<iterator>`.


https://reviews.llvm.org/D49338