From ec0310d3658e5db5a5a7b10d094bfaf0b7a03562 Mon Sep 17 00:00:00 2001
From: Yedidya Feldblum <yfeldblum@meta.com>
Date: Tue, 3 Dec 2024 13:23:24 -0800
Subject: [PATCH] tweaks to helper fn countPlaceholders

Summary:
* Properly namespace it.
* Revise it with C++ >= 14 constexpr defn.

Reviewed By: Gownta

Differential Revision: D66590536

fbshipit-source-id: a537c89e64fa436a5f7ddb2ae50535a992551a38
---
 fb303/ThreadCachedServiceData.h     | 47 +++++++++++++++++------------
 fb303/detail/QuantileStatWrappers.h | 12 ++++----
 2 files changed, 33 insertions(+), 26 deletions(-)

diff --git a/fb303/ThreadCachedServiceData.h b/fb303/ThreadCachedServiceData.h
index 6179b88bc..d87913f1e 100644
--- a/fb303/ThreadCachedServiceData.h
+++ b/fb303/ThreadCachedServiceData.h
@@ -1544,31 +1544,38 @@ inline fb303::ThreadCachedServiceData& tcData() {
 #define DEFINE_histogram(varname, ...) \
   ::facebook::fb303::HistogramWrapper STATS_##varname(#varname, ##__VA_ARGS__)
 
+namespace facebook::fb303::detail {
+
 // We use this function to extract the number of placeholders from our keyformat
 // at compile-time.
 // This also ensures that our keyformat is a constexpr.
-constexpr int countPlaceholders(folly::StringPiece keyformat) {
-  return keyformat.size() < 2
-      ? 0
-      : ((*keyformat.begin() == '{' && *(keyformat.begin() + 1) == '}')
-             ? (1 +
-                countPlaceholders(
-                    folly::range(keyformat.begin() + 2, keyformat.end())))
-             : countPlaceholders(
-                   folly::range(keyformat.begin() + 1, keyformat.end())));
+constexpr size_t count_placeholders(std::string_view keyformat) {
+  size_t n = 0;
+  for (size_t i = 0; i < keyformat.size(); ++i) {
+    if (keyformat[i] == '{') {
+      assert(i + 1 < keyformat.size());
+      assert(keyformat[i + 1] == '}');
+      ++n;
+    }
+  }
+  return n;
 }
 
-#define DEFINE_dynamic_timeseries(varname, keyformat, ...)                  \
-  static_assert(                                                            \
-      countPlaceholders(keyformat) > 0,                                     \
-      "Must have at least one placeholder.");                               \
-  ::facebook::fb303::DynamicTimeseriesWrapper<countPlaceholders(keyformat)> \
+} // namespace facebook::fb303::detail
+
+#define DEFINE_dynamic_timeseries(varname, keyformat, ...)          \
+  static_assert(                                                    \
+      ::facebook::fb303::detail::count_placeholders(keyformat) > 0, \
+      "Must have at least one placeholder.");                       \
+  ::facebook::fb303::DynamicTimeseriesWrapper<                      \
+      ::facebook::fb303::detail::count_placeholders(keyformat)>     \
       STATS_##varname(keyformat, ##__VA_ARGS__)
 
-#define DEFINE_dynamic_histogram(                                          \
-    varname, keyformat, bucketWidth, min, max, ...)                        \
-  static_assert(                                                           \
-      countPlaceholders(keyformat) > 0,                                    \
-      "Must have at least one placeholder.");                              \
-  ::facebook::fb303::DynamicHistogramWrapper<countPlaceholders(keyformat)> \
+#define DEFINE_dynamic_histogram(                                   \
+    varname, keyformat, bucketWidth, min, max, ...)                 \
+  static_assert(                                                    \
+      ::facebook::fb303::detail::count_placeholders(keyformat) > 0, \
+      "Must have at least one placeholder.");                       \
+  ::facebook::fb303::DynamicHistogramWrapper<                       \
+      ::facebook::fb303::detail::count_placeholders(keyformat)>     \
       STATS_##varname(keyformat, bucketWidth, min, max, __VA_ARGS__)
diff --git a/fb303/detail/QuantileStatWrappers.h b/fb303/detail/QuantileStatWrappers.h
index cd4d73a63..f6b35dbf0 100644
--- a/fb303/detail/QuantileStatWrappers.h
+++ b/fb303/detail/QuantileStatWrappers.h
@@ -127,10 +127,10 @@ class DynamicQuantileStatWrapper {
   extern ::facebook::fb303::detail::DynamicQuantileStatWrapper<keyNumArgs> \
       STATS_##varname
 
-#define DEFINE_dynamic_quantile_stat(varname, keyformat, ...)            \
-  static_assert(                                                         \
-      countPlaceholders(keyformat) > 0,                                  \
-      "Must have at least one placeholder.");                            \
-  facebook::fb303::detail::DynamicQuantileStatWrapper<countPlaceholders( \
-      keyformat)>                                                        \
+#define DEFINE_dynamic_quantile_stat(varname, keyformat, ...)       \
+  static_assert(                                                    \
+      ::facebook::fb303::detail::count_placeholders(keyformat) > 0, \
+      "Must have at least one placeholder.");                       \
+  facebook::fb303::detail::DynamicQuantileStatWrapper<              \
+      ::facebook::fb303::detail::count_placeholders(keyformat)>     \
       STATS_##varname(keyformat, ##__VA_ARGS__)