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 <[email protected]> Pigweed-Auto-Submit: Wyatt Hepler <[email protected]> Reviewed-by: Ted Pudlik <[email protected]>
google · Jun 16, 2022 · 1b338eb · 1b338eb
1 parent 92be6f4
commit 1b338eb
Show file tree

Hide file tree

Showing 11 changed files with 259 additions and 68 deletions.
diff --git a/pw_build/pigweed.cmake b/pw_build/pigweed.cmake
@@ -540,11 +540,12 @@ set(pw_unit_test_GOOGLETEST_BACKEND pw_unit_test.light CACHE STRING
 #   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 @@ function(pw_add_test NAME)
       ${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
@@ -283,9 +283,9 @@ class CodeFormat(NamedTuple):
     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
@@ -22,9 +22,22 @@ package(default_visibility = ["//visibility:public"])
 
 licenses(["notice"])
 
+# TODO(b/235237667): Rename this target to pw_span:pw_span.
 pw_cc_library(
-    name = "pw_span",
+    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.h",
+        "public/pw_span/internal/span_common.inc",
+    ],
     hdrs = ["public_overrides/span"],
     includes = [
         "public",
@@ -37,10 +50,27 @@ pw_cc_library(
 )
 
 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
@@ -29,22 +29,33 @@ config("overrides_config") {
   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:*" ]
+pw_source_set("pw_span") {
+  public = [ "public/pw_span/span.h" ]
+  public_configs = [ ":public_config" ]
+  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.
+  if (pw_toolchain_CXX_STANDARD < pw_toolchain_STANDARD.CXX17) {
+    public_deps += [
+      "$dir_pw_polyfill:cstddef",
+      "$dir_pw_polyfill:iterator",
+    ]
+  }
 }
 
-# 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") {
+# 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" ]
-  public_deps = [ "$dir_pw_polyfill" ]
+  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.
@@ -54,18 +65,42 @@ pw_source_set("pw_span") {
       "$dir_pw_polyfill:iterator",
     ]
   }
-  sources = [ "public/pw_span/internal/span_common.inc" ]
-  visibility = [ ":*" ]
+  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("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("test") {
+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
@@ -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
@@ -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 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.
-
-``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/``.
-
-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
+=======
+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.
+
+.. note::
+
+ ``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 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 @@ arguments:
     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 @@ Pointer and size arguments can be replaced with a ``std::span``:
   }
 
 .. 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 @@ Pointer and size arguments can be replaced with a ``std::span``:
       ProcessData(std::as_bytes(std::span(data)));
     }
 
+  ``pw_bytes/span.h`` provides ``ByteSpan`` and ``ConstByteSpan`` aliases for
+  these types.
+
+-------------
 Compatibility
-=============
-Works with C++14, but some features require C++17.
+-------------
+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
@@ -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