[all-commits] [llvm/llvm-project] a1857e: [libcxx][test] Fix span tests.

Stephan T. Lavavej via All-commits all-commits at lists.llvm.org
Wed Jan 8 00:31:01 PST 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: a1857e2ce35e749e16d092305f53c0f2bf2e9c7b
      https://github.com/llvm/llvm-project/commit/a1857e2ce35e749e16d092305f53c0f2bf2e9c7b
  Author: Stephan T. Lavavej <stl at microsoft.com>
  Date:   2020-01-08 (Wed, 08 Jan 2020)

  Changed paths:
    M libcxx/test/std/containers/views/span.cons/container.pass.cpp
    M libcxx/test/std/containers/views/span.cons/span.pass.cpp
    M libcxx/test/std/containers/views/span.cons/stdarray.pass.cpp
    M libcxx/test/std/containers/views/span.objectrep/as_bytes.pass.cpp
    M libcxx/test/std/containers/views/span.objectrep/as_writable_bytes.pass.cpp
    M libcxx/test/std/containers/views/span.tuple/get.pass.cpp

  Log Message:
  -----------
  [libcxx][test] Fix span tests.

span.cons/container.pass.cpp
N4842 22.7.3.2 [span.cons]/13 constrains span's range constructor
for ranges::contiguous_range (among other criteria).

24.4.5 [range.refinements]/2 says that contiguous_range requires data(),
and (via contiguous_range, random_access_range, bidirectional_range,
forward_range, input_range, range) it also requires begin() and end()
(see 24.4.2 [range.range]/1).

Therefore, IsAContainer needs to provide begin() and end().

(Detected by MSVC's concept-constrained implementation.)

span.cons/stdarray.pass.cpp
This test uses std::array, so it must include <array>.
<span> isn't guaranteed to drag in <array>.

(Detected by MSVC's implementation which uses a forward declaration to
avoid dragging in <array>, for increased compiler throughput.)

span.objectrep/as_bytes.pass.cpp
span.objectrep/as_writable_bytes.pass.cpp
Testing `sp.extent == std::dynamic_extent` triggers MSVC warning
C4127 "conditional expression is constant". Using `if constexpr` is a
simple way to avoid this without disrupting anyone else (as span
requires C++20 mode).

span.tuple/get.pass.cpp
22.7.3.2 [span.cons]/4.3: "Preconditions: If extent is not equal to
dynamic_extent, then count is equal to extent."

These lines were triggering undefined behavior (detected by assertions
in MSVC's implementation).

I changed the count arguments in the first two chunks, followed by
changing the span extents, in order to preserve the test's coverage
and follow the existing pattern.

span.cons/span.pass.cpp
22.7.3.2 [span.cons]/18.1 constrains span's converting constructor with
"Extent == dynamic_extent || Extent == OtherExtent is true".

This means that converting from dynamic extent to static extent is
not allowed. (Other constructors tested elsewhere, like
span(It first, size_type count), can be used to write such code.)

As this is the test for the converting constructor, I have:

* Removed the "dynamic -> static" case from checkCV(), which is
comprehensive.

* Changed the initialization of std::span<T, 0> s1{}; in
testConstexprSpan() and testRuntimeSpan(), because s1 is used below.

* Removed ASSERT_NOEXCEPT(std::span<T, 0>{s0}); from those functions,
as they are otherwise comprehensive.

* Deleted testConversionSpan() entirely. Note that this could never
compile (it had a bool return type, but forgot to say `return`). And it
couldn't have provided useful coverage, as the /18.2 constraint
"OtherElementType(*)[] is convertible to ElementType(*)[]"
permits only cv-qualifications, which are already tested by checkCV().




More information about the All-commits mailing list