[libcxx-commits] [libcxx] a6c43e8 - [libc++][format] Fixes usage of contiguous ranges.

[Mark de Wever via libcxx-commits]

Author: Mark de Wever
Date: 2023-01-24T18:08:53+01:00
New Revision: a6c43e8ca8f622ee2da4bad5458ebf5cecf6dd90

URL: https://github.com/llvm/llvm-project/commit/a6c43e8ca8f622ee2da4bad5458ebf5cecf6dd90
DIFF: https://github.com/llvm/llvm-project/commit/a6c43e8ca8f622ee2da4bad5458ebf5cecf6dd90.diff

LOG: [libc++][format] Fixes usage of contiguous ranges.

The contiguous range made incorrect assumptions for certain input
ranges.

Fixes llvm.org/PR60164

Reviewed By: #libc, ldionne

Differential Revision: https://reviews.llvm.org/D142302

Added: 
    

Modified: 
    libcxx/include/__format/range_formatter.h
    libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h

Removed: 
    


################################################################################
diff  --git a/libcxx/include/__format/range_formatter.h b/libcxx/include/__format/range_formatter.h
index 6188842a6eb48..9ea61a70350a7 100644
--- a/libcxx/include/__format/range_formatter.h
+++ b/libcxx/include/__format/range_formatter.h
@@ -29,6 +29,8 @@
 #include <__format/parser_std_format_spec.h>
 #include <__iterator/back_insert_iterator.h>
 #include <__ranges/concepts.h>
+#include <__ranges/data.h>
+#include <__ranges/size.h>
 #include <__type_traits/remove_cvref.h>
 #include <string_view>
 
@@ -165,11 +167,16 @@ struct _LIBCPP_TEMPLATE_VIS _LIBCPP_AVAILABILITY_FORMAT range_formatter {
     // When the range is contiguous use a basic_string_view instead to avoid a
     // copy of the underlying data. The basic_string_view formatter
     // specialization is the "basic" string formatter in libc++.
-    if constexpr (ranges::contiguous_range<_Rp>) {
+    if constexpr (ranges::contiguous_range<_Rp> && std::ranges::sized_range<_Rp>) {
       std::formatter<basic_string_view<_CharT>, _CharT> __formatter;
       if (__debug_format)
         __formatter.set_debug_format();
-      return __formatter.format(basic_string_view<_CharT>{__range.data(), __range.size()}, __ctx);
+      return __formatter.format(
+          basic_string_view<_CharT>{
+              ranges::data(__range),
+              ranges::size(__range),
+          },
+          __ctx);
     } else {
       std::formatter<basic_string<_CharT>, _CharT> __formatter;
       if (__debug_format)

diff  --git a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h
index 890f597bdeaed..a465916afa10e 100644
--- a/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h
+++ b/libcxx/test/std/utilities/format/format.range/format.range.formatter/format.functions.tests.h
@@ -12,6 +12,7 @@
 #include <array>
 #include <charconv>
 #include <concepts>
+#include <deque>
 #include <format>
 #include <list>
 #include <ranges>
@@ -1261,6 +1262,66 @@ void test_with_ranges(TestFunction check, ExceptionTest check_exception) {
       check, check_exception, std::counted_iterator{contiguous_iterator<int*>(input.data()), input.size()});
 }
 
+//
+// Adaptor
+//
+
+template <class CharT>
+class non_contiguous {
+  // A deque iterator is random access, but not contiguous.
+  using adaptee = std::deque<CharT>;
+
+public:
+  using iterator = typename adaptee::iterator;
+  using pointer  = typename adaptee::pointer;
+
+  iterator begin() { return data_.begin(); }
+  iterator end() { return data_.end(); }
+
+  explicit non_contiguous(adaptee&& data) : data_(std::move(data)) {}
+
+private:
+  adaptee data_;
+};
+
+template <class CharT>
+class contiguous {
+  // A vector iterator is contiguous.
+  using adaptee = std::vector<CharT>;
+
+public:
+  using iterator = typename adaptee::iterator;
+  using pointer  = typename adaptee::pointer;
+
+  iterator begin() { return data_.begin(); }
+  iterator end() { return data_.end(); }
+
+  explicit contiguous(adaptee&& data) : data_(std::move(data)) {}
+
+private:
+  adaptee data_;
+};
+
+// This tests two 
diff erent implementations in libc++. A basic_string_view
+// formatter if the range is contiguous, a basic_string otherwise.
+template <class CharT, class TestFunction, class ExceptionTest>
+void test_adaptor(TestFunction check, ExceptionTest check_exception) {
+  static_assert(std::format_kind<non_contiguous<CharT>> == std::range_format::sequence);
+  static_assert(std::ranges::sized_range<non_contiguous<CharT>>);
+  static_assert(!std::ranges::contiguous_range<non_contiguous<CharT>>);
+  test_char_string<CharT>(
+      check,
+      check_exception,
+      non_contiguous<CharT>{std::deque{CharT('H'), CharT('e'), CharT('l'), CharT('l'), CharT('o')}});
+
+  static_assert(std::format_kind<contiguous<CharT>> == std::range_format::sequence);
+  static_assert(std::ranges::sized_range<contiguous<CharT>>);
+  static_assert(std::ranges::contiguous_range<contiguous<CharT>>);
+  test_char_string<CharT>(check,
+                          check_exception,
+                          contiguous<CharT>{std::vector{CharT('H'), CharT('e'), CharT('l'), CharT('l'), CharT('o')}});
+}
+
 //
 // Driver
 //
@@ -1285,6 +1346,8 @@ void format_tests(TestFunction check, ExceptionTest check_exception) {
   test_tuple_int_int_int<CharT>(check, check_exception);
 
   test_with_ranges<CharT>(check, check_exception);
+
+  test_adaptor<CharT>(check, check_exception);
 }
 
 #endif // TEST_STD_UTILITIES_FORMAT_FORMAT_RANGE_FORMAT_RANGE_FORMATTER_FORMAT_FUNCTIONS_TESTS_H