[libcxx-commits] [PATCH] D109475: [libc++] Simplify span specializations

[Joe Loser via Phabricator via libcxx-commits]

jloser added inline comments.


================
Comment at: libcxx/test/std/containers/views/span.sub/subspan.fail.cpp:50
   {
-    [[maybe_unused]] auto s1 = sp.subspan<3, std::size_t(-2)>(); // expected-error-re at span:* {{static_assert failed{{( due to requirement '.*')?}} "Offset + count out of range in span::subspan()"}}, expected-error-re at span:* {{array is too large{{(.* elements)}}}}
+    [[maybe_unused]] auto s1 = sp.subspan<3, std::size_t(-2)>(); // expected-error-re at span:* {{static_assert failed{{( due to requirement '.*')?}} "Offset + count out of range in span::subspan()"}}
   }
----------------
curdeius wrote:
> Where did the `array is too large` message came from before? Why doesn't it show up with this change?
In the previous code, an error diagnostic of 

> Line 232: array is too large (18446744073709551614 elements)

would have been given in checking [[ https://github.com/llvm/llvm-project/blob/d61ec93ff2752201c8f1c281728aa9803ac9be1c/libcxx/include/span#L232 | this constructor ]].

In the new code, I'm not entirely sure why we don't get that additional constructor error in the list of errors diagnostics. Note that the `static_assert` still fires for the "Offset + count out of range in span::subspan()". I poked around a bit, but couldn't get it to trigger the additional constructor error about the array size. Do you or others have an idea why?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109475