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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
