pw_span: Introduce pw::span
- pw::span is identical to Pigweed's polyfilled std::span, but in a
different namespace. Implicit converions between std::span and
pw::span are supported inherently.
pw::span and std::span are maintained as separate classes, with shared
code in a .inc file. This is done instead of aliasing since C++17 does
not support class template argument decuction with aliases. Also, it
is simpler to have separate implementations to avoid circular
dependencies while migrating code to pw::span.
- Add DEFINES to the pw_add_test() CMake function to support defining
preprocessor macros for the pw_span test.
- Move '.inc' and '.inl' extensions from CPP_HEADER_EXTS to
CPP_SOURCE_EXTS, since these should be treated like sources not
headers (no include guards).
Bug: b/235237667
Change-Id: I4d76fb373cdaa4d3a5d72a0537bea492b5c06814
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/98160
Commit-Queue: Wyatt Hepler <hepler@google.com>
Pigweed-Auto-Submit: Wyatt Hepler <hepler@google.com>
Reviewed-by: Ted Pudlik <tpudlik@google.com>
diff --git a/pw_build/pigweed.cmake b/pw_build/pigweed.cmake
index 212d0eb..3ef02c4 100644
--- a/pw_build/pigweed.cmake
+++ b/pw_build/pigweed.cmake
@@ -540,11 +540,12 @@
# NAME: name to use for the target
# SOURCES: source files for this test
# DEPS: libraries on which this test depends
+# DEFINES: defines to set for the source files in this test
# GROUPS: groups to which to add this test; if none are specified, the test is
# added to the 'default' and 'all' groups
#
function(pw_add_test NAME)
- pw_parse_arguments_strict(pw_add_test 1 "" "" "SOURCES;DEPS;GROUPS")
+ pw_parse_arguments_strict(pw_add_test 1 "" "" "SOURCES;DEPS;DEFINES;GROUPS")
add_executable("${NAME}" EXCLUDE_FROM_ALL ${arg_SOURCES})
target_link_libraries("${NAME}"
@@ -553,6 +554,11 @@
${pw_unit_test_MAIN}
${arg_DEPS}
)
+ target_compile_definitions("${NAME}"
+ PRIVATE
+ ${arg_DEFINES}
+ )
+
# Tests require at least one source file.
if(NOT arg_SOURCES)
target_sources("${NAME}" PRIVATE $<TARGET_PROPERTY:pw_build.empty,SOURCES>)
diff --git a/pw_presubmit/py/pw_presubmit/format_code.py b/pw_presubmit/py/pw_presubmit/format_code.py
index 623b544..e4b617f 100755
--- a/pw_presubmit/py/pw_presubmit/format_code.py
+++ b/pw_presubmit/py/pw_presubmit/format_code.py
@@ -283,9 +283,9 @@
fix: Callable[[Iterable], Dict[Path, str]]
-CPP_HEADER_EXTS = frozenset(
- ('.h', '.hpp', '.hxx', '.h++', '.hh', '.H', '.inc', '.inl'))
-CPP_SOURCE_EXTS = frozenset(('.c', '.cpp', '.cxx', '.c++', '.cc', '.C'))
+CPP_HEADER_EXTS = frozenset(('.h', '.hpp', '.hxx', '.h++', '.hh', '.H'))
+CPP_SOURCE_EXTS = frozenset(
+ ('.c', '.cpp', '.cxx', '.c++', '.cc', '.C', '.inc', '.inl'))
CPP_EXTS = CPP_HEADER_EXTS.union(CPP_SOURCE_EXTS)
C_FORMAT: CodeFormat = CodeFormat('C and C++', CPP_EXTS,
diff --git a/pw_span/BUILD.bazel b/pw_span/BUILD.bazel
index 296a995..ca6e901 100644
--- a/pw_span/BUILD.bazel
+++ b/pw_span/BUILD.bazel
@@ -22,9 +22,22 @@
licenses(["notice"])
+# TODO(b/235237667): Rename this target to pw_span:pw_span.
+pw_cc_library(
+ name = "pigweed_span",
+ srcs = ["public/pw_span/internal/span_common.inc"],
+ hdrs = ["public/pw_span/span.h"],
+ includes = ["public"],
+ deps = ["//pw_polyfill"],
+)
+
+# TODO(b/235237667): Rename this target to pw_span:polyfill.
pw_cc_library(
name = "pw_span",
- srcs = ["public/pw_span/internal/span_common.inc"],
+ srcs = [
+ "public/pw_span/internal/span.h",
+ "public/pw_span/internal/span_common.inc",
+ ],
hdrs = ["public_overrides/span"],
includes = [
"public",
@@ -37,10 +50,27 @@
)
pw_cc_test(
- name = "span_test",
+ name = "polyfill_test",
srcs = ["span_test.cc"],
+ defines = [
+ "PW_SPAN_TEST_INCLUDE=<span>",
+ "PW_SPAN_TEST_NAMESPACE=std",
+ ],
deps = [
":pw_span",
"//pw_unit_test",
],
)
+
+pw_cc_test(
+ name = "pw_span_test",
+ srcs = ["span_test.cc"],
+ defines = [
+ 'PW_SPAN_TEST_INCLUDE=\\"pw_span/span.h\\"',
+ "PW_SPAN_TEST_NAMESPACE=pw",
+ ],
+ deps = [
+ ":pigweed_span",
+ "//pw_unit_test",
+ ],
+)
diff --git a/pw_span/BUILD.gn b/pw_span/BUILD.gn
index 86c769a..e73e05f 100644
--- a/pw_span/BUILD.gn
+++ b/pw_span/BUILD.gn
@@ -29,22 +29,11 @@
visibility = [ ":*" ]
}
-# This source set provides the <span> header, which is accessed only through
-# pw_polyfill.
-pw_source_set("polyfill") {
- remove_public_deps = [ "*" ]
- public_configs = [ ":overrides_config" ]
- public_deps = [ ":pw_span" ]
- public = [ "public_overrides/span" ]
- visibility = [ "$dir_pw_polyfill:*" ]
-}
-
-# This source set provides the internal span.h header included by <span>. This
-# source set is only used by pw_polyfill, so its visibility is restricted.
pw_source_set("pw_span") {
- remove_public_deps = [ "*" ]
+ public = [ "public/pw_span/span.h" ]
public_configs = [ ":public_config" ]
- public_deps = [ "$dir_pw_polyfill" ]
+ public_deps = [ dir_pw_polyfill ]
+ sources = [ "public/pw_span/internal/span_common.inc" ]
# Polyfill <cstddef> (std::byte) and <iterator> (std::size(), std::data) if
# C++17 is not supported.
@@ -54,18 +43,64 @@
"$dir_pw_polyfill:iterator",
]
}
- sources = [ "public/pw_span/internal/span_common.inc" ]
- visibility = [ ":*" ]
+}
+
+# This source set provides the <span> header, which is accessed only through
+# pw_polyfill. This source set provides the internal span.h header included by
+# <span>. This source set is only used by pw_polyfill, so its visibility is
+# restricted.
+pw_source_set("polyfill") {
+ remove_public_deps = [ "*" ]
+ public_configs = [
+ ":public_config",
+ ":overrides_config",
+ ]
+ public_deps = [ dir_pw_polyfill ]
+
+ # Polyfill <cstddef> (std::byte) and <iterator> (std::size(), std::data) if
+ # C++17 is not supported.
+ if (pw_toolchain_CXX_STANDARD < pw_toolchain_STANDARD.CXX17) {
+ public_deps += [
+ "$dir_pw_polyfill:cstddef",
+ "$dir_pw_polyfill:iterator",
+ ]
+ }
+ public = [ "public_overrides/span" ]
+ sources = [
+ "public/pw_span/internal/span.h",
+ "public/pw_span/internal/span_common.inc",
+ ]
+ visibility = [
+ ":*",
+ "$dir_pw_polyfill:*",
+ ]
}
pw_test_group("tests") {
- tests = [ ":test" ]
+ tests = [
+ ":polyfill_test",
+ ":pw_span_test",
+ ]
}
-pw_test("test") {
+pw_test("polyfill_test") {
+ deps = [ ":polyfill" ]
+ remove_configs = [ "$dir_pw_build:extra_strict_warnings" ]
+ sources = [ "span_test.cc" ]
+ defines = [
+ "PW_SPAN_TEST_INCLUDE=<span>",
+ "PW_SPAN_TEST_NAMESPACE=std",
+ ]
+}
+
+pw_test("pw_span_test") {
deps = [ ":pw_span" ]
remove_configs = [ "$dir_pw_build:extra_strict_warnings" ]
sources = [ "span_test.cc" ]
+ defines = [
+ "PW_SPAN_TEST_INCLUDE=\"pw_span/span.h\"",
+ "PW_SPAN_TEST_NAMESPACE=pw",
+ ]
}
pw_doc_group("docs") {
diff --git a/pw_span/CMakeLists.txt b/pw_span/CMakeLists.txt
index a53fd0d..e6f5b5f 100644
--- a/pw_span/CMakeLists.txt
+++ b/pw_span/CMakeLists.txt
@@ -14,12 +14,39 @@
include($ENV{PW_ROOT}/pw_build/pigweed.cmake)
-pw_auto_add_simple_module(pw_span
+pw_add_module_library(pw_span
+ HEADERS
+
+ PUBLIC_INCLUDES
+ public
+ public_overrides
PUBLIC_DEPS
pw_polyfill
pw_polyfill.standard_library
)
-target_include_directories(pw_span INTERFACE public_overrides)
+
+pw_add_test(pw_span.polyfill_test
+ SOURCES
+ span_test.cc
+ DEFINES
+ "PW_SPAN_TEST_INCLUDE=<span>"
+ "PW_SPAN_TEST_NAMESPACE=std"
+ GROUPS
+ modules
+ pw_span
+)
+
+
+pw_add_test(pw_span.pw_span_test
+ SOURCES
+ span_test.cc
+ DEFINES
+ "PW_SPAN_TEST_INCLUDE=\"pw_span/span.h\""
+ "PW_SPAN_TEST_NAMESPACE=pw"
+ GROUPS
+ modules
+ pw_span
+)
if(Zephyr_FOUND AND CONFIG_PIGWEED_SPAN)
zephyr_link_libraries(pw_span)
diff --git a/pw_span/docs.rst b/pw_span/docs.rst
index 63d44ba..65a0466 100644
--- a/pw_span/docs.rst
+++ b/pw_span/docs.rst
@@ -1,27 +1,26 @@
.. _module-pw_span:
--------
+=======
pw_span
--------
-The ``pw_span`` module provides an implementation of C++20's
-`std::span <https://en.cppreference.com/w/cpp/container/span>`_, which is a
-non-owning view of an array of values. The intent is for this implementation of
-``std::span`` is to exactly match the C++20 standard.
+=======
+The ``pw_span`` module provides :cpp:class:`pw::span`, an implementation of
+C++20's `std::span <https://en.cppreference.com/w/cpp/container/span>`_.
+``std::span`` is a non-owning view of an array of values. The intent is for
+:cpp:class:`pw::span` is to match the C++20 standard as closely as possible.
-The only header provided by the ``pw_span`` module is ``<span>``. It is included
-as if it were coming from the C++ Standard Library. If the C++ library provides
-``<span>``, the library's version of ``std::span`` is used in place of
-``pw_span``'s.
+.. note::
-``pw_span`` requires two include paths -- ``public/`` and ``public_overrides/``.
-The internal implementation header is in ``public/``, and the ``<span>`` header
-that mimics the C++ Standard Library is in ``public_overrides/``.
+ ``pw_span:polyfill`` provides ``<span>`` and a ``std::span`` class that is
+ identical to :cpp:class:`pw::span`. The ``std::span`` polyfill is DEPRECATED;
+ do NOT use it for new code. Use :cpp:class:`pw::span` instead, or
+ ``std::span`` if you are building with C++20.
-Using std::span
-===============
-``std::span`` is a convenient abstraction that wraps a pointer and a size.
-``std::span`` is especially useful in APIs. Spans support implicit conversions
-from C arrays, ``std::array``, or any STL-style container, such as
+--------------
+Using pw::span
+--------------
+:cpp:class:`pw::span` is a convenient abstraction that wraps a pointer and a
+size. :cpp:class:`pw::span` is especially useful in APIs. Spans support implicit
+conversions from C arrays, ``std::array``, or any STL-style container, such as
``std::string_view``.
Functions operating on an array of bytes typically accept pointer and size
@@ -37,7 +36,7 @@
ProcessBuffer(data_pointer, data_size);
}
-Pointer and size arguments can be replaced with a ``std::span``:
+Pointer and size arguments can be replaced with a :cpp:class:`pw::span`:
.. code-block:: cpp
@@ -53,9 +52,10 @@
}
.. tip::
- Use ``std::span<std::byte>`` or ``std::span<const std::byte>`` to represent
- spans of binary data. Use ``std::as_bytes`` or ``std::as_writeable_bytes``
- to convert any span to a byte span.
+
+ Use ``pw::span<std::byte>`` or ``std::span<const std::byte>`` to represent
+ spans of binary data. Use ``pw::as_bytes`` or ``pw::as_writeable_bytes`` to
+ convert any span to a byte span.
.. code-block:: cpp
@@ -66,11 +66,17 @@
ProcessData(std::as_bytes(std::span(data)));
}
-Compatibility
-=============
-Works with C++14, but some features require C++17.
+ ``pw_bytes/span.h`` provides ``ByteSpan`` and ``ConstByteSpan`` aliases for
+ these types.
+-------------
+Compatibility
+-------------
+Works with C++14, but some features require C++17. In C++20, use ``std::span``
+instead.
+
+------
Zephyr
-======
+------
To enable ``pw_span`` for Zephyr add ``CONFIG_PIGWEED_SPAN=y`` to the project's
configuration.
diff --git a/pw_span/public/pw_span/internal/span.h b/pw_span/public/pw_span/internal/span.h
new file mode 100644
index 0000000..3bd8090
--- /dev/null
+++ b/pw_span/public/pw_span/internal/span.h
@@ -0,0 +1,51 @@
+// Copyright 2020 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+// std::span is a stand-in for C++20's std::span. Do NOT include this header
+// directly; instead, include it as <span>.
+//
+// A span is a non-owning array view class. It refers to an external array by
+// storing a pointer and length. Unlike std::array, the size does not have to be
+// a template parameter, so this class can be used to without stating its size.
+//
+// This file a modified version of base::span from Chromium:
+// https://chromium.googlesource.com/chromium/src/+/d93ae920e4309682deb9352a4637cfc2941c1d1f/base/containers/span.h
+//
+// In order to minimize changes from the original, this file does NOT fully
+// adhere to Pigweed's style guide.
+//
+// A few changes were made to the Chromium version of span. These include:
+// - Use std::data and std::size instead of base::* versions.
+// - Rename base namespace to std.
+// - Rename internal namespace to pw_span_internal.
+// - Remove uses of checked_iterators.h and CHECK.
+// - Replace make_span functions with C++17 class template deduction guides.
+// - Use std::byte instead of uint8_t for compatibility with std::span.
+//
+#pragma once
+
+#include "pw_polyfill/standard_library/namespace.h"
+
+#ifndef __cpp_lib_span
+#define __cpp_lib_span 202002L
+
+#define _PW_SPAN_COMMON_NAMEPACE_BEGIN _PW_POLYFILL_BEGIN_NAMESPACE_STD
+#define _PW_SPAN_COMMON_NAMEPACE_END _PW_POLYFILL_END_NAMESPACE_STD
+
+#include "pw_span/internal/span_common.inc"
+
+#undef _PW_SPAN_COMMON_NAMEPACE_BEGIN
+#undef _PW_SPAN_COMMON_NAMEPACE_END
+
+#endif // __cpp_lib_span
diff --git a/pw_span/public/pw_span/internal/span_common.inc b/pw_span/public/pw_span/internal/span_common.inc
index 2799bfd..a347ffc 100644
--- a/pw_span/public/pw_span/internal/span_common.inc
+++ b/pw_span/public/pw_span/internal/span_common.inc
@@ -12,8 +12,8 @@
// License for the specific language governing permissions and limitations under
// the License.
-// std::span is a stand-in for C++20's std::span. Do NOT include this header
-// directly; instead, include it as <span>.
+// This span implementation is a stand-in for C++20's std::span. Do NOT include
+// this header directly; instead, include it as "pw_span/span.h".
//
// A span is a non-owning array view class. It refers to an external array by
// storing a pointer and length. Unlike std::array, the size does not have to be
@@ -32,11 +32,9 @@
// - Remove uses of checked_iterators.h and CHECK.
// - Replace make_span functions with C++17 class template deduction guides.
// - Use std::byte instead of uint8_t for compatibility with std::span.
-//
-#pragma once
-#ifndef __cpp_lib_span
-#define __cpp_lib_span 202002L
+// This file does not have #pragma once! It is intended to be included in
+// multiple header files!
#include <algorithm>
#include <array>
@@ -47,12 +45,16 @@
#include <utility>
#include "pw_polyfill/language_feature_macros.h"
-#include "pw_polyfill/standard_library/namespace.h"
// Pigweed: Disable the asserts from Chromium for now.
#define _PW_SPAN_ASSERT(arg)
-_PW_POLYFILL_BEGIN_NAMESPACE_STD
+// The file that includes this .inc file must define the following macros:
+//
+// * _PW_SPAN_COMMON_NAMEPACE_BEGIN
+// * _PW_SPAN_COMMON_NAMEPACE_END
+
+_PW_SPAN_COMMON_NAMEPACE_BEGIN
// [views.constants]
constexpr size_t dynamic_extent = std::numeric_limits<size_t>::max();
@@ -72,7 +74,7 @@
struct ExtentImpl<std::array<T, N>> : std::integral_constant<size_t, N> {};
template <typename T, size_t N>
-struct ExtentImpl<std::span<T, N>> : std::integral_constant<size_t, N> {};
+struct ExtentImpl<span<T, N>> : std::integral_constant<size_t, N> {};
template <typename T>
using Extent = ExtentImpl<std::remove_cv_t<std::remove_reference_t<T>>>;
@@ -473,7 +475,6 @@
#endif // __cpp_deduction_guides
-_PW_POLYFILL_END_NAMESPACE_STD
+_PW_SPAN_COMMON_NAMEPACE_END
#undef _PW_SPAN_ASSERT
-#endif // __cpp_lib_span
diff --git a/pw_span/public/pw_span/span.h b/pw_span/public/pw_span/span.h
new file mode 100644
index 0000000..a5fb5a2
--- /dev/null
+++ b/pw_span/public/pw_span/span.h
@@ -0,0 +1,27 @@
+// Copyright 2022 The Pigweed Authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License"); you may not
+// use this file except in compliance with the License. You may obtain a copy of
+// the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+// License for the specific language governing permissions and limitations under
+// the License.
+
+// pw::span is an implementation of std::span for C++14 or newer. The
+// implementation is shared with the std::span polyfill class.
+#pragma once
+
+// TODO(b/235237667): Make pw::span an alias of std::span when it is available.
+
+#define _PW_SPAN_COMMON_NAMEPACE_BEGIN namespace pw {
+#define _PW_SPAN_COMMON_NAMEPACE_END } // namespace pw
+
+#include "pw_span/internal/span_common.inc"
+
+#undef _PW_SPAN_COMMON_NAMEPACE_BEGIN
+#undef _PW_SPAN_COMMON_NAMEPACE_END
diff --git a/pw_span/public_overrides/span b/pw_span/public_overrides/span
index c22a188..1ac4288 100644
--- a/pw_span/public_overrides/span
+++ b/pw_span/public_overrides/span
@@ -17,4 +17,4 @@
#include_next <span>
#endif // __has_include_next(<span>)
-#include "pw_span/internal/span_common.inc"
+#include "pw_span/internal/span.h"
diff --git a/pw_span/span_test.cc b/pw_span/span_test.cc
index 7277368..5b9ea68 100644
--- a/pw_span/span_test.cc
+++ b/pw_span/span_test.cc
@@ -21,10 +21,18 @@
// In order to minimize changes from the original, this file does NOT fully
// adhere to Pigweed's style guide.
+#ifndef PW_SPAN_TEST_INCLUDE
+#error "The PW_SPAN_TEST_INCLUDE macro must be defined to compile this test."
+#endif // PW_SPAN_TEST_INCLUDE
+
+#ifndef PW_SPAN_TEST_NAMESPACE
+#error "The PW_SPAN_TEST_NAMESPACE macro must be defined to compile this test."
+#endif // PW_SPAN_TEST_NAMESPACE
+
#include <algorithm>
#include <cstdint>
#include <memory>
-#include <span>
+#include PW_SPAN_TEST_INCLUDE
#include <string>
#include <type_traits>
#include <vector>
@@ -39,7 +47,7 @@
using ::testing::Pointwise;
#endif // 0
-namespace std {
+namespace PW_SPAN_TEST_NAMESPACE {
namespace {
@@ -1645,4 +1653,4 @@
"Error: const iterator should not be convertible to iterator");
}
-} // namespace std
+} // namespace PW_SPAN_TEST_NAMESPACE
