Deprecate public range print helpers

[bhalevy]

Seastar exposes two print helpers in

seastar/include/seastar/core/sstring.hh

Lines 749 to 782 in 58a4e90

	namespace std {
	template <typename T>
	inline
	std::ostream& operator<<(std::ostream& os, const std::vector<T>& v) {
	bool first = true;
	os << "{";
	for (auto&& elem : v) {
	if (!first) {
	os << ", ";
	} else {
	first = false;
	}
	os << elem;
	}
	os << "}";
	return os;
	}
	template <typename Key, typename T, typename Hash, typename KeyEqual, typename Allocator>
	std::ostream& operator<<(std::ostream& os, const std::unordered_map<Key, T, Hash, KeyEqual, Allocator>& v) {
	bool first = true;
	os << "{";
	for (auto&& elem : v) {
	if (!first) {
	os << ", ";
	} else {
	first = false;
	}
	os << "{" << elem.first << " -> " << elem.second << "}";
	}
	os << "}";
	return os;
	}
	}

and a bunch of to_sstring helpers in

seastar/include/seastar/core/sstring.hh

Lines 720 to 747 in 58a4e90

	string_type to_sstring(T value) {
	auto size = fmt::formatted_size("{}", value);
	auto formatted = uninitialized_string<string_type>(size);
	fmt::format_to(formatted.data(), "{}", value);
	return formatted;
	}
	template <typename string_type>
	string_type to_sstring(const char* value) {
	return string_type(value);
	}
	template <typename string_type>
	string_type to_sstring(sstring value) {
	return value;
	}
	template <typename string_type>
	string_type to_sstring(const temporary_buffer<char>& buf) {
	return string_type(buf.get(), buf.size());
	}
	}
	template <typename string_type = sstring, typename T>
	string_type to_sstring(T value) {
	return internal::to_sstring<string_type>(value);
	}
	}

This is out of scope for the seastar library.

In particular, the implementation for operator<<(std::ostream&, const std::unordered_map) is unusual and conflicts with similar helpers in scylla utils/to_string.hh

cc @avikivity @nyh @tchaikov

[tchaikov]

first of all, i am for this change. Seastar is not a formatting library, neither should it help user to print standard containers or even range-like containers or string-like objects -- this is covered by fmtlib already.

currently, we have 6 ways to print a range

1. fmt::join() to print a range with customized delimiter
2. fmt::format() to print a range. this is also supported by fmt/range.h
3. operator<<(ostream&, ...) overloads provided by scylladb/utils/to_string.hh
4. to_string() overloads provided by scylladb/utils/to_string.hh
5. operator<<(ostream&, ...) overloads provided by seastar/core/sstring.hh
6. to_string() overloads provided by seastar/core/sstring.hh

i think we should consolidate them, and only use fmtlib for printing out various objects and containers of them. but we should at least ready Scylla for this change. to make this happen, there is a strong requisite: to make fmt::is_formatable<Printable>::value true, where Printable is any type which want to print. fmt/ranges.h uses this predicate to tell if an element can be printed using its builtin range-like formatter. currently, despite that seastar::sstring can be printed using fmt::format() either because of FMT_DEPRECATED_OSTREAM or because of the fmt::formatter<seastar::sstring> partial specialization, we cannot print std::pair<int, sstring> or std::unordered_map<int, sstring> using Seastar's builtin support for range-like container. because !detail::has_fallback_formatter<T, Char>::value> is false, so is fmt::is_formatable<sstring>::value. see https://github.com/fmtlib/fmt/blob/master/include/fmt/core.h#L1505-L1509. the reason why has_fallback_formatter<> is true is that we have FMT_DEPRECATED_OSTREAM defined in scylla.

we need to use a bottom-up approach to achieve this goal:

1. replace all os << foo << " and " << bar with fmt::print(os, "{} and {}", foo, bar)
2. use fmt::join() for printing containers. use fmt::join() for printing ranges scylladb#13163 is a step in this direction.
3. use specialization of fmt::formatter<Printable> in the place of ostream<<(ostream&, const Printable&)
4. if we have to keep ostream<<(..), we can implement it using the former.
5. repeat step 1,2,3 and 4 until we don't depend on FMT_DEPRECATED_OSTREAM anymore
6. drop FMT_DEPRECATED_OSTREAM from configure.py
7. use the builtin range-like formatting in the place of fmt::join() where the delimiter and the open/close mark used by fmtlib's builtin implementation fulfills our needs
8. remove the helpers in seastar/core/sstring.hh and scylladb/utils/to_string.hh

as scylla is a large scale project, we might want to address this in multiple iterations.

[bhalevy]

@tchaikov the above sounds like a solid plan

[avikivity]

Agree with the plan. Note that FMT_DEPRECATED_OSTREAM isn't in seastar, IIRC.

[tchaikov]

No, it's not in Seastar. But it prevents Scylla from using fmtlib's builtin range and tuple formatters with our types like sstring. So we need to fix Scylla first and then drop these helpers.

[tchaikov]

i am copying this issue and the plan above to scylla. so when we have PRs address it, we can associate these PRs to the scylla issue, as Avi pointed out, FMT_DEPRECATED_OSTREAM is not in Seastar. a lot of work is on the Seastar applications' side, once they drop the dependencies on these helpers, we can safely deprecate or better off completely remove them.

[nyh]

This issue is a duplicate of the seven-year-old #98.

The original reason why these various pretty-printers were added to Seastar was that Seastar included a logging library, and you often wanted to log things like exceptions, vectors, and so on. Of course as #98 explains this was never a very good reason to add all of them, and certainly not in random header files like we did. @tchaikov is right that probably as we switch more and more to using fmt instead of C++'s operator<<, there are better ways to do some of these things.

Personally, to reduce friction and backward-incompatibility, and make Seastar's logging easier to use, I would add a header file that makes it easy to log objects of various common types (and also serve as an example how different projects, like Scylla, should make their own objects printable) - and not require most uses of Seastar loggers to use various fmt functions manually.

[bhalevy]

@nyh I can understand seastar proving printers for data structures it defines, but why would it provide pretty printers for stl types?

[gleb-cloudius]

@nyh I can understand seastar proving printers for data structures it defines, but why would it provide pretty printers for stl types?

Because if nobody else provides them then printing containers becomes a PITA.

[bhalevy]

@nyh I can understand seastar proving printers for data structures it defines, but why would it provide pretty printers for stl types?

Because if nobody else provides them then printing containers becomes a PITA.

Modern fmt library version provide helpers for containers / ranges.
See #1544 (comment)

[gleb-cloudius]

@nyh I can understand seastar proving printers for data structures it defines, but why would it provide pretty printers for stl types?

Because if nobody else provides them then printing containers becomes a PITA.

Modern fmt library version provide helpers for containers / ranges. See #1544 (comment)

I am explaining why do we have them now. Obviously if we move to another library that provides them there is no need to have them in Seastar any more.