watchman: implement w_string::build in terms of w_string::format

Summary: This improves handling of a number of types (eg: bool and pointer types) and allows removing a chunk of ugly boiler plate that was borrowed and adapted from folly in the past. Reviewed By: chadaustin Differential Revision: D17783425 fbshipit-source-id: 7316c89f9d3527722f5ab1cccb3da54031cff3ad
lptr · Oct 31, 2019 · 980a222 · 980a222
1 parent 997b4f0
commit 980a222
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 285 deletions.
diff --git a/cmds/subscribe.cpp b/cmds/subscribe.cpp
@@ -55,7 +55,7 @@ static std::tuple<sub_action, w_string> get_subscription_action(
   watchman::log(
       watchman::DBG,
       "sub=",
-      sub,
+      fmt::ptr(sub),
       " ",
       sub->name,
       ", last=",
@@ -577,18 +577,15 @@ static void cmd_subscribe(
 
   // Connect the root to our subscription
   {
-    auto client_id = folly::to<std::string>(uintptr_t(client));
-    auto client_stream = folly::to<std::string>(uintptr_t(client->stm.get()));
-    auto info_json = json_object(
-        {{"name", w_string_to_json(sub->name)},
-         {"query", sub->query->query_spec},
-         {"client",
-          w_string_to_json(w_string(client_id.data(), client_id.size()))},
-         {"stm",
-          w_string_to_json(
-              w_string(client_stream.data(), client_stream.size()))},
-         {"is_owner", json_boolean(client->stm->peerIsOwner())},
-         {"pid", json_integer(client->stm->getPeerProcessID())}});
+    auto client_id = w_string::build(fmt::ptr(client));
+    auto client_stream = w_string::build(fmt::ptr(client->stm.get()));
+    auto info_json =
+        json_object({{"name", w_string_to_json(sub->name)},
+                     {"query", sub->query->query_spec},
+                     {"client", w_string_to_json(client_id)},
+                     {"stm", w_string_to_json(client_stream)},
+                     {"is_owner", json_boolean(client->stm->peerIsOwner())},
+                     {"pid", json_integer(client->stm->getPeerProcessID())}});
 
     std::weak_ptr<watchman_client> clientRef(client->shared_from_this());
     client->unilateralSub.insert(std::make_pair(

diff --git a/root/stat.cpp b/root/stat.cpp
@@ -102,19 +102,19 @@ void InMemoryView::statPath(
           "getFileInformation(",
           path,
           ") file=",
-          file,
+          fmt::ptr(file),
           " dir=",
-          dir_ent,
+          fmt::ptr(dir_ent),
           "\n");
     } catch (const std::system_error& exc) {
       errcode = exc.code();
       log(DBG,
           "getFileInformation(",
           path,
           ") file=",
-          file,
+          fmt::ptr(file),
           " dir=",
-          dir_ent,
+          fmt::ptr(dir_ent),
           " failed: ",
           exc.what(),
           "\n");

diff --git a/tests/string_test.cpp b/tests/string_test.cpp
@@ -42,8 +42,8 @@ TEST(String, integrals) {
 
   EXPECT_EQ(w_string::build(int8_t(-127)), w_string("-127"));
 
-  EXPECT_EQ(w_string::build(bool(true)), w_string("1"));
-  EXPECT_EQ(w_string::build(bool(false)), w_string("0"));
+  EXPECT_EQ(w_string::build(bool(true)), w_string("true"));
+  EXPECT_EQ(w_string::build(bool(false)), w_string("false"));
 }
 
 TEST(String, strings) {
@@ -94,7 +94,7 @@ TEST(String, pointers) {
   bool foo = true;
   char lowerBuf[20];
 
-  auto str = w_string::build(&foo);
+  auto str = w_string::build(fmt::ptr(&foo));
   snprintf(
       lowerBuf, sizeof(lowerBuf), "0x%" PRIx64, (uint64_t)(uintptr_t)(&foo));
   EXPECT_EQ(str.size(), strlen_uint32(lowerBuf))
@@ -117,17 +117,12 @@ TEST(String, pointers) {
 
 TEST(String, double) {
   auto str = w_string::build(5.5);
-  char buf[16];
-  snprintf(buf, sizeof(buf), "%f", 5.5);
-  EXPECT_EQ(str.size(), 8);
-  EXPECT_TRUE(!strcmp(str.c_str(), buf))
-      << "str.c_str=" << str.c_str() << " buf=" << buf;
-  EXPECT_EQ(str, w_string("5.500000"));
+  EXPECT_EQ(str, w_string("5.5"));
 }
 
 TEST(String, concat) {
   auto str = w_string::build("one", 2, "three", 1.2, false, w_string(nullptr));
-  EXPECT_EQ(str, w_string("one2three1.2000000"));
+  EXPECT_EQ(str, w_string("one2three1.2false"));
 }
 
 TEST(String, lowercase_suffix) {