Fix build with -Wctad-maybe-unsupported

The immediate use of CTAD was actually unnecessary due to CBS implicitly
converting to Span. Still, let's make Span CTAD-capable and thus remove
some unnecessary MakeSpans and MakeConstSpans in libpki. I've kept them
in libssl which, for now, still allows C++14.

Change-Id: Iec92f520f645f86f098afb860a2129fb30c61da9
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/68847
Commit-Queue: Bob Beck <[email protected]>
Auto-Submit: David Benjamin <[email protected]>
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>

CMakeLists.txt

@@ -152,6 +152,14 @@ if(CMAKE_COMPILER_IS_GNUCXX OR CLANG)
     set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wframe-larger-than=25344")
   endif()
 
+  # -Wctad-maybe-unsupported was added in Clang 10, which is AppleClang 12.0.0.
+  if((CMAKE_C_COMPILER_ID STREQUAL "Clang" AND
+      CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL "10.0.0") OR
+     (CMAKE_C_COMPILER_ID STREQUAL "AppleClang" AND
+      CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL "12.0.0"))
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wctad-maybe-unsupported")
+  endif()
+
   if(CLANG OR CMAKE_C_COMPILER_VERSION VERSION_GREATER_EQUAL "7.0.0")
     set(C_CXX_FLAGS "${C_CXX_FLAGS} -Wimplicit-fallthrough")
   endif()

include/openssl/span.h

@@ -49,6 +49,16 @@ class SpanBase {
 
   friend bool operator!=(Span<T> lhs, Span<T> rhs) { return !(lhs == rhs); }
 };
+
+// Heuristically test whether C is a container type that can be converted into
+// a Span<T> by checking for data() and size() member functions.
+//
+// TODO(davidben): Require C++17 support for std::is_convertible_v, etc.
+template <typename C, typename T>
+using EnableIfContainer = std::enable_if_t<
+    std::is_convertible<decltype(std::declval<C>().data()), T *>::value &&
+    std::is_integral<decltype(std::declval<C>().size())>::value>;
+
 }  // namespace internal
 
 // A Span<T> is a non-owning reference to a contiguous array of objects of type
@@ -84,16 +94,6 @@ class SpanBase {
 // a reference or pointer to a container or array.
 template <typename T>
 class Span : private internal::SpanBase<const T> {
- private:
-  // Heuristically test whether C is a container type that can be converted into
-  // a Span by checking for data() and size() member functions.
-  //
-  // TODO(davidben): Require C++17 support for std::is_convertible_v, etc.
-  template <typename C>
-  using EnableIfContainer = std::enable_if_t<
-      std::is_convertible<decltype(std::declval<C>().data()), T *>::value &&
-      std::is_integral<decltype(std::declval<C>().size())>::value>;
-
  public:
   static const size_t npos = static_cast<size_t>(-1);
 
@@ -114,12 +114,12 @@ class Span : private internal::SpanBase<const T> {
   template <size_t N>
   constexpr Span(T (&array)[N]) : Span(array, N) {}
 
-  template <typename C, typename = EnableIfContainer<C>,
+  template <typename C, typename = internal::EnableIfContainer<C, T>,
             typename = std::enable_if_t<std::is_const<T>::value, C>>
   constexpr Span(const C &container)
       : data_(container.data()), size_(container.size()) {}
 
-  template <typename C, typename = EnableIfContainer<C>,
+  template <typename C, typename = internal::EnableIfContainer<C, T>,
             typename = std::enable_if_t<!std::is_const<T>::value, C>>
   constexpr explicit Span(C &container)
       : data_(container.data()), size_(container.size()) {}
@@ -188,6 +188,20 @@ class Span : private internal::SpanBase<const T> {
 template <typename T>
 const size_t Span<T>::npos;
 
+#if __cplusplus >= 201703L
+template <typename T>
+Span(T *, size_t) -> Span<T>;
+template <typename T, size_t size>
+Span(T (&array)[size]) -> Span<T>;
+template <
+    typename C,
+    typename T = std::remove_pointer_t<decltype(std::declval<C>().data())>,
+    typename = internal::EnableIfContainer<C, T>>
+Span(C &) -> Span<T>;
+#endif
+
+// C++17 callers can instead rely on CTAD and the deduction guides defined
+// above.
 template <typename T>
 constexpr Span<T> MakeSpan(T *ptr, size_t size) {
   return Span<T>(ptr, size);

pki/cert_error_params.cc

@@ -43,11 +43,8 @@ class CertErrorParams2Der : public CertErrorParams {
   static void AppendDer(const char *name, const std::string &der,
                         std::string *out) {
     *out += name;
-    // TODO(crbug.com/boringssl/661): Introduce a convenience function to go
-    // from a Span<const char> to a Span<const uint8_t>.
-    *out +=
-        ": " + bssl::string_util::HexEncode(MakeConstSpan(
-                   reinterpret_cast<const uint8_t *>(der.data()), der.size()));
+    *out += ": ";
+    *out += bssl::string_util::HexEncode(StringAsBytes(der));
   }
 
   const char *name1_;

pki/input.cc

@@ -10,9 +10,7 @@ namespace bssl::der {
 
 std::string Input::AsString() const { return std::string(AsStringView()); }
 
-bool operator==(Input lhs, Input rhs) {
-  return MakeConstSpan(lhs) == MakeConstSpan(rhs);
-}
+bool operator==(Input lhs, Input rhs) { return Span(lhs) == Span(rhs); }
 
 bool operator!=(Input lhs, Input rhs) { return !(lhs == rhs); }
 

pki/input.h

@@ -42,16 +42,14 @@ class OPENSSL_EXPORT Input {
 
   // Creates an Input from the given |data| and |len|.
   constexpr explicit Input(const uint8_t *data, size_t len)
-      : data_(MakeConstSpan(data, len)) {}
+      : data_(Span(data, len)) {}
 
   // Deprecated: Use StringAsBytes.
   //
   // Creates an Input from a std::string_view. The constructed Input is only
   // valid as long as |data| points to live memory. If constructed from, say, a
   // |std::string|, mutating the vector will invalidate the Input.
-  explicit Input(std::string_view str)
-      : data_(MakeConstSpan(reinterpret_cast<const uint8_t *>(str.data()),
-                            str.size())) {}
+  explicit Input(std::string_view str) : data_(StringAsBytes(str)) {}
 
   // The following APIs have the same semantics as in |bssl::Span|.
   constexpr Span<const uint8_t>::iterator begin() const {

pki/parse_name.cc

@@ -129,8 +129,7 @@ bool X509NameAttribute::AsRFC2253String(std::string *out) const {
         value_string += c;
       } else if (c < 32 || c > 126) {
         nonprintable = true;
-        value_string +=
-            "\\" + bssl::string_util::HexEncode(MakeConstSpan(&c, 1));
+        value_string += "\\" + bssl::string_util::HexEncode(Span(&c, 1));
       } else {
         value_string += c;
       }

pki/verify.cc

@@ -103,8 +103,7 @@ std::unique_ptr<VerifyTrustStore> VerifyTrustStore::FromDER(
       return {};
     }
 
-    auto parsed_cert = InternalParseCertificate(
-        Span(CBS_data(&cert), CBS_len(&cert)), out_diagnostic);
+    auto parsed_cert = InternalParseCertificate(cert, out_diagnostic);
     if (!parsed_cert.has_value()) {
       return {};
     }