[core] Implement a universal printer

[dentiny]

Learned from my prev company how to use ordered visitor to implement a universal printer, which could handle ANY type.

Checkout unit tests:

ray/src/ray/util/strings/debug_printer_test.cc

Lines 31 to 136 in 414c895

	enum class MyEnum { kEnum };
	struct Point {
	template <typename Sink>
	friend void AbslStringify(Sink &sink, const Point &p) {
	absl::Format(&sink, "(%d, %d)", p.x, p.y);
	}
	int x;
	int y;
	};
	struct DebugStringStruct {
	std::string DebugString() const { return "hello"; }
	};
	} // namespace
	TEST(DebugStringTest, LiteralTest) {
	// Literal.
	EXPECT_EQ(DebugString(1), "1");
	// Debug string.
	EXPECT_EQ(DebugString(DebugStringStruct{}), "hello");
	// Container.
	const std::vector<int> vec{1, 2, 3};
	EXPECT_EQ(DebugString(vec), "[1, 2, 3]");
	// std::byte.
	const std::byte b{10};
	EXPECT_EQ(DebugString(b), "b[0x0a]");
	// Boolean.
	EXPECT_EQ(DebugString(true), "true");
	// nullptr
	EXPECT_EQ(DebugString(nullptr), "nullptr");
	// Tuple.
	const std::tuple<int, std::string, double> tpl{4, "hello", 4.5};
	EXPECT_EQ(DebugString(tpl), "[4, hello, 4.5]");
	// Map.
	std::map<std::string, std::string> m;
	m.emplace("a", "b");
	m.emplace("hello", "world");
	EXPECT_EQ(DebugString(m), "[{a, b}, {hello, world}]");
	// Pair.
	std::pair<int, double> p;
	p.first = 5;
	p.second = 10.6;
	EXPECT_EQ(DebugString(p), "{5, 10.6}");
	// Container inside of container.
	const std::vector<std::vector<int>> cont{
	{1, 2, 3},
	{4, 5, 6},
	};
	EXPECT_EQ(DebugString(cont), "[[1, 2, 3], [4, 5, 6]]");
	// Enum.
	EXPECT_EQ(DebugString(MyEnum::kEnum), "0");
	// Abseil stringify.
	EXPECT_EQ(DebugString(Point{.x = 10, .y = 20}), "(10, 20)");
	// std::optional
	const std::optional<int> has_value{10};
	const std::optional<int> no_value = std::nullopt;
	EXPECT_EQ(DebugString(has_value), "10");
	EXPECT_EQ(DebugString(no_value), "(nullopt)");
	// std::optional with std::vector and std::map inside.
	std::map<std::string, int> uno_m = {
	{"hello", 6},
	};
	std::vector<std::map<std::string, int>> map_vec = {uno_m};
	std::optional<std::vector<std::map<std::string, int>>> opt_vec_map = map_vec;
	EXPECT_EQ(DebugString(opt_vec_map), "[[{hello, 6}]]");
	// std::type_info
	EXPECT_EQ(DebugString(typeid(Point)), "ray::(anonymous namespace)::Point");
	// std::variant
	std::variant<std::monostate, std::string, double, std::vector<std::vector<int>>> v;
	v = std::monostate{};
	EXPECT_EQ(DebugString(v), "(monostate)");
	v = std::string{"hello world"};
	EXPECT_EQ(DebugString(v), "hello world");
	v = std::vector<std::vector<int>>{
	std::vector<int>{1},
	std::vector<int>{2},
	};
	EXPECT_EQ(DebugString(v), "[[1], [2]]");
	// Complex type.
	std::vector<int> vec1 = {1};
	std::vector<std::vector<int>> vec2 = {vec1, vec1};
	std::vector<std::vector<std::vector<int>>> vec3 = {vec2, vec2};
	std::vector<std::vector<std::vector<std::vector<int>>>> vec4 = {vec3, vec3};
	EXPECT_EQ(DebugString(vec4), "[[[[1], [1]], [[1], [1]]], [[[1], [1]], [[1], [1]]]]");
	// Complex with self-defined type.
	std::vector<Point> objs{Point{.x = 5, .y = 6}};
	EXPECT_EQ(DebugString(objs), "[(5, 6)]");
	}

Example usage 1:
We could replace all usage of RAY_CHECK to RAY_CHECK_EQ, etc

ray/src/ray/util/logging.h

Line 166 in 8b26f5c

RAY_CHECK(RAY_PREDICT_TRUE(_left_ op _right_)) << " " << _left_ << " vs " << _right_

In the past it's not feasible because current implementation assume all type ostream-printable.

Detailed example:

ray/src/ray/core_worker/fiber.h

Line 155 in 2bd1a56

RAY_CHECK(op_status == boost::fibers::channel_op_status::success);

ray/src/ray/core_worker/task_event_buffer.cc

Line 74 in 2bd1a56

RAY_CHECK(task_status_ == rpc::TaskStatus::SUBMITTED_TO_WORKER)

ray/src/ray/core_worker/task_event_buffer.cc

Line 130 in 2bd1a56

RAY_CHECK(task_status_ == rpc::TaskStatus::SUBMITTED_TO_WORKER)

The list goes on.

Example usage 2:
https://github.com/ray-project/ray/blob/master/src/ray/util/container_util.h

Current implementation hard codes all container type we've met, with the debug printer we no longer need it.

[edoakes]

This looks like more complexity & indirection than we need for string formatting, prefer to keep this simple

[dentiny]

This looks like more complexity & indirection than we need for string formatting, prefer to keep this simple

Could you please tell me another way to print all values?

[dentiny]

@edoakes As discussed offline, I added example usage to the PR description.

[dayshah]

will continue review later, in general though, have we considered a separate util repo from which we can try to upstream stuff to absl or other util repos

[dayshah]

what are the two extra void template params for

[dentiny]

It's the fallback template argument (basically for SFINAE, fallback template type should match L31-L34)

[dayshah]

would you write this differently with concepts, that should make this easier to read? (in perfect cpp20 world)

[dentiny]

Yeah in my prev company all in concepts

[dentiny]

And yes, if we do have C++20 I will update to concepts and modern type traits

[dayshah]

I think it'd be good to describe more specifically your definition of "array" with like something that's indexable. also a more specific is_array_v

[dentiny]

is_array_v is not correct, both contiguous container and non-continuous one like deque should apply