[libcxx-commits] [PATCH] D126663: [libc++][test] Refactor SmallBasicString uses in range.lazy.split tests

Joe Loser via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Sat Jun 11 12:39:30 PDT 2022

jloser marked an inline comment as done.
jloser added inline comments.

Comment at: libcxx/test/std/ranges/range.adaptors/range.lazy.split/general.pass.cpp:34
+template <class Char>
+class BasicSmallString {
+  std::basic_string<Char> buffer_{};
var-const wrote:
> What prevents replacing this class with `std::string` as well?
I think it's the different types of things we're working with the `Char` and `Str` types in `is_equal` function template. Specifically, the family of types it takes on is "stringlike" but we take advantage of that extra layer of conversion when constructing the `BasicSmallString` (note the three different constructors all of which we use - specifically when going from a `string_view`-like type to a `string` for example).

I played with it for a while and didn't find a way to remove the dependency. I do recommend you or @philnik play with it as a follow up for a bit to see if you come up with something clever as you're both more knowledgable in the ranges area. I really would like to get rid of `is_equal` entirely and just use `std::ranges::equal`, but the default projection won't work for similar reasons as to why we have `BasicSmallString` to begin with (and the view type's iterator/sentinel won't satisfy `input_range` needed for `std::ranges::equal`).

Comment at: libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.outer/deref.pass.cpp:34
-    assert(SmallString(*i) == "abc"_str);
-    assert(SmallString(*(++i)) == "def"_str);
-    assert(SmallString(*(++i)) == "ghi"_str);
+    assert(std::ranges::equal(*i, "abc"s));
+    assert(std::ranges::equal(*(++i), "def"s));
var-const wrote:
> Why is calling `equal` necessary instead of `==`?
There's no valid `operator==` now. The error is:

libcxx/test/std/ranges/range.adaptors/range.lazy.split/range.lazy.split.outer/deref.pass.cpp:35:15: error: invalid operands to binary expression ('std::ranges::lazy_split_view<ForwardDiffView, ForwardDiffView>::__outer_iterator<false>::value_type' and 'basic_string<char>')
    assert(*i == "abc"s);

In the previous code, it worked because of

constexpr bool operator==(const BasicSmallString& lhs, std::string_view rhs)

in `small_string.h` if I understand correctly. Now that we removed `SmallString`, we don't have that comparison operator. Similar story in the other call sites that were made to use `std::ranges::equal`.

Does that make sense?

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list