[dump-args] Move DumpArgs to use AbslStringify (#36897)

This provides better integration with our ecosystem, and allows more types to be automatically printed (yay tuples!).

Closes #36897

COPYBARA_INTEGRATE_REVIEW=https://github.com/grpc/grpc/pull/36897 from ctiller:stringify da98d247e2
PiperOrigin-RevId: 644120732

CMakeLists.txt

@@ -13057,6 +13057,7 @@ target_link_libraries(dump_args_test
   gtest
   absl::any_invocable
   absl::check
+  absl::str_format
 )
 
 

build_autogenerated.yaml

@@ -9556,6 +9556,7 @@ targets:
   - gtest
   - absl/functional:any_invocable
   - absl/log:check
+  - absl/strings:str_format
   uses_polling: false
 - name: duplicate_header_bad_client_test
   gtest: true

include/grpc/event_engine/event_engine.h

@@ -503,6 +503,20 @@ bool operator!=(const EventEngine::ConnectionHandle& lhs,
 std::ostream& operator<<(std::ostream& out,
                          const EventEngine::ConnectionHandle& handle);
 
+namespace detail {
+std::string FormatHandleString(uint64_t key1, uint64_t key2);
+}
+
+template <typename Sink>
+void AbslStringify(Sink& out, const EventEngine::ConnectionHandle& handle) {
+  out.Append(detail::FormatHandleString(handle.keys[0], handle.keys[1]));
+}
+
+template <typename Sink>
+void AbslStringify(Sink& out, const EventEngine::TaskHandle& handle) {
+  out.Append(detail::FormatHandleString(handle.keys[0], handle.keys[1]));
+}
+
 }  // namespace experimental
 }  // namespace grpc_event_engine
 

src/core/BUILD

@@ -60,6 +60,7 @@ grpc_cc_library(
         "absl/functional:any_invocable",
         "absl/log:check",
         "absl/strings",
+        "absl/strings:str_format",
     ],
     language = "c++",
     deps = [

src/core/lib/event_engine/event_engine.cc

@@ -23,6 +23,13 @@ const EventEngine::TaskHandle EventEngine::TaskHandle::kInvalid = {-1, -1};
 const EventEngine::ConnectionHandle EventEngine::ConnectionHandle::kInvalid = {
     -1, -1};
 
+namespace detail {
+std::string FormatHandleString(uint64_t key1, uint64_t key2) {
+  return absl::StrCat("{", absl::Hex(key1, absl::kZeroPad16), ",",
+                      absl::Hex(key2, absl::kZeroPad16), "}");
+}
+}  // namespace detail
+
 namespace {
 template <typename T>
 bool eq(const T& lhs, const T& rhs) {
@@ -30,8 +37,7 @@ bool eq(const T& lhs, const T& rhs) {
 }
 template <typename T>
 std::ostream& printout(std::ostream& out, const T& handle) {
-  out << absl::StrCat("{", absl::Hex(handle.keys[0], absl::kZeroPad16), ",",
-                      absl::Hex(handle.keys[1], absl::kZeroPad16), "}");
+  out << detail::FormatHandleString(handle.keys[0], handle.keys[1]);
   return out;
 }
 }  // namespace

src/core/lib/gprpp/dump_args.cc

@@ -21,7 +21,7 @@
 namespace grpc_core {
 namespace dump_args_detail {
 
-std::ostream& operator<<(std::ostream& out, const DumpArgs& args) {
+void DumpArgs::Stringify(CustomSink& sink) const {
   // Parse the argument string into a vector of keys.
   // #__VA_ARGS__ produces a stringified version of the arguments passed to the
   // macro. It's comma separated, and we can use that to split the string into
@@ -29,8 +29,8 @@ std::ostream& operator<<(std::ostream& out, const DumpArgs& args) {
   // we need to skip commas that are inside parenthesis.
   std::vector<absl::string_view> keys;
   int depth = 0;
-  const char* start = args.arg_string_;
-  for (const char* p = args.arg_string_; *p; ++p) {
+  const char* start = arg_string_;
+  for (const char* p = arg_string_; *p; ++p) {
     if (*p == '(') {
       ++depth;
     } else if (*p == ')') {
@@ -41,13 +41,13 @@ std::ostream& operator<<(std::ostream& out, const DumpArgs& args) {
     }
   }
   keys.push_back(start);
-  CHECK_EQ(keys.size(), args.arg_dumpers_.size());
+  CHECK_EQ(keys.size(), arg_dumpers_.size());
   for (size_t i = 0; i < keys.size(); i++) {
-    if (i != 0) out << ", ";
-    out << absl::StripAsciiWhitespace(keys[i]) << " = ";
-    args.arg_dumpers_[i](out);
+    if (i != 0) sink.Append(", ");
+    sink.Append(absl::StripAsciiWhitespace(keys[i]));
+    sink.Append(" = ");
+    arg_dumpers_[i](sink);
   }
-  return out;
 }
 
 }  // namespace dump_args_detail

src/core/lib/gprpp/dump_args.h

@@ -19,6 +19,9 @@
 #include <vector>
 
 #include "absl/functional/any_invocable.h"
+#include "absl/strings/str_cat.h"
+#include "absl/strings/str_format.h"
+#include "absl/strings/string_view.h"
 
 namespace grpc_core {
 namespace dump_args_detail {
@@ -39,17 +42,62 @@ class DumpArgs {
     do_these_things({AddDumper(&args)...});
   }
 
-  friend std::ostream& operator<<(std::ostream& out, const DumpArgs& args);
+  template <typename Sink>
+  friend void AbslStringify(Sink& sink, const DumpArgs& dumper) {
+    CustomSinkImpl<Sink> custom_sink(sink);
+    dumper.Stringify(custom_sink);
+  }
+
+  friend std::ostream& operator<<(std::ostream& out, const DumpArgs& dumper) {
+    return out << absl::StrCat(dumper);
+  }
 
  private:
+  class CustomSink {
+   public:
+    virtual void Append(absl::string_view x) = 0;
+
+   protected:
+    ~CustomSink() = default;
+  };
+
+  template <typename Sink>
+  class CustomSinkImpl final : public CustomSink {
+   public:
+    explicit CustomSinkImpl(Sink& sink) : sink_(sink) {}
+    void Append(absl::string_view x) override { sink_.Append(x); }
+
+   private:
+    Sink& sink_;
+  };
+
   template <typename T>
   int AddDumper(T* p) {
-    arg_dumpers_.push_back([p](std::ostream& os) { os << *p; });
+    arg_dumpers_.push_back(
+        [p](CustomSink& os) { os.Append(absl::StrCat(*p)); });
     return 0;
   }
 
+  int AddDumper(void** p) {
+    arg_dumpers_.push_back(
+        [p](CustomSink& os) { os.Append(absl::StrFormat("%p", *p)); });
+    return 0;
+  }
+
+  template <typename T>
+  int AddDumper(T** p) {
+    return AddDumper(reinterpret_cast<void**>(p));
+  }
+
+  template <typename T>
+  int AddDumper(T* const* p) {
+    return AddDumper(const_cast<T**>(p));
+  }
+
+  void Stringify(CustomSink& sink) const;
+
   const char* arg_string_;
-  std::vector<absl::AnyInvocable<void(std::ostream&) const>> arg_dumpers_;
+  std::vector<absl::AnyInvocable<void(CustomSink&) const>> arg_dumpers_;
 };
 
 }  // namespace dump_args_detail

src/core/lib/gprpp/time.h

@@ -169,6 +169,11 @@ class Timestamp {
 
   std::string ToString() const;
 
+  template <typename Sink>
+  friend void AbslStringify(Sink& sink, const Timestamp& t) {
+    sink.Append(t.ToString());
+  }
+
  private:
   explicit constexpr Timestamp(int64_t millis) : millis_(millis) {}
 
@@ -293,6 +298,11 @@ class Duration {
   // https://developers.google.com/protocol-buffers/docs/proto3#json
   std::string ToJsonString() const;
 
+  template <typename Sink>
+  friend void AbslStringify(Sink& sink, const Duration& t) {
+    sink.Append(t.ToString());
+  }
+
  private:
   explicit constexpr Duration(int64_t millis) : millis_(millis) {}
 
@@ -377,9 +387,6 @@ inline Timestamp& Timestamp::operator+=(Duration duration) {
 
 void TestOnlySetProcessEpoch(gpr_timespec epoch);
 
-std::ostream& operator<<(std::ostream& out, Timestamp timestamp);
-std::ostream& operator<<(std::ostream& out, Duration duration);
-
 }  // namespace grpc_core
 
 #endif  // GRPC_SRC_CORE_LIB_GPRPP_TIME_H

test/core/event_engine/handle_tests.cc

@@ -13,6 +13,7 @@
 // limitations under the License.
 #include <memory>
 
+#include "absl/strings/str_cat.h"
 #include "gtest/gtest.h"
 
 #include <grpc/event_engine/event_engine.h>
@@ -46,6 +47,11 @@ TYPED_TEST(TaskHandleTest, Validity) {
   ASSERT_EQ(TypeParam::kInvalid, TypeParam::kInvalid);
 }
 
+TYPED_TEST(TaskHandleTest, AbslStringify) {
+  TypeParam t{42, 43};
+  ASSERT_EQ(absl::StrCat(t), "{000000000000002a,000000000000002b}");
+}
+
 int main(int argc, char** argv) {
   testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();

test/core/gprpp/dump_args_test.cc

@@ -18,24 +18,17 @@
 
 #include "gtest/gtest.h"
 
-template <typename T>
-std::string Stringify(const T& t) {
-  std::ostringstream oss;
-  oss << t;
-  return oss.str();
-}
-
 int add(int a, int b) { return a + b; }
 
 TEST(DumpArgsTest, Basic) {
   int a = 1;
   int b = 2;
   int c = 3;
-  EXPECT_EQ("a = 1, b = 2, c = 3", Stringify(GRPC_DUMP_ARGS(a, b, c)));
+  EXPECT_EQ("a = 1, b = 2, c = 3", absl::StrCat(GRPC_DUMP_ARGS(a, b, c)));
 }
 
 TEST(DumpArgsTest, FunctionCall) {
-  EXPECT_EQ("add(1, 2) = 3", Stringify(GRPC_DUMP_ARGS(add(1, 2))));
+  EXPECT_EQ("add(1, 2) = 3", absl::StrCat(GRPC_DUMP_ARGS(add(1, 2))));
 }
 
 int main(int argc, char** argv) {