[PATCH] D49338: Implement <span> - P0122R7

[Louis Dionne via Phabricator via cfe-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM, with a suggestion for adding one test.



================
Comment at: include/span:189
+struct __is_span_compatible_container<_Tp, _ElementType,
+        void_t<
+        // is not a specialization of span
----------------
ldionne wrote:
> You seem to be missing the following condition from the paper: `is_array_v<Container> is false`. Are you omitting it because the array overloads would take precedence over this constructor if an array were passed? If so, I think there's a bug since you could be passing an array of incorrect size and this constructor would kick in. The overload for `ElementType(*)[N]` would be discarded because of a mismatched `N`, but this one wouldn't (I think).
> 
> If I'm right, this would turn what should be a compilation failure into a runtime error with the `_LIBCPP_ASSERT(_Extent == _VSTD::size(__c))`.
Consider adding a test for this.


https://reviews.llvm.org/D49338