src: make minor improvements to EnabledDebugList

tniessen · web-flow · commit ddc5da935337 · 2022-08-25T00:28:46.000Z
Change the underlying type of DebugCategory to unsigned int so that it can be safely cast to an unsigned type without having to worry about negative values, removing the need for the DCHECK_GE statements. To fully benefit from type safety, remove DebugCategory::CATEGORY_COUNT and instead add a constexpr kDebugCategoryCount. Remove the second argument from EnabledDebugList::set_enabled() and EnabledDebugList::Parse() because it was always set to true. PR-URL: nodejs#44350 Reviewed-By: Chengzhong Wu <legendecas@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Darshan Sen <raisinten@gmail.com>
diff --git a/src/debug_utils.cc b/src/debug_utils.cc
@@ -63,10 +63,10 @@ void EnabledDebugList::Parse(std::shared_ptr<KVStore> env_vars,
                              v8::Isolate* isolate) {
   std::string cats;
   credentials::SafeGetenv("NODE_DEBUG_NATIVE", &cats, env_vars, isolate);
-  Parse(cats, true);
+  Parse(cats);
 }
 
-void EnabledDebugList::Parse(const std::string& cats, bool enabled) {
+void EnabledDebugList::Parse(const std::string& cats) {
   std::string debug_categories = cats;
   while (!debug_categories.empty()) {
     std::string::size_type comma_pos = debug_categories.find(',');
@@ -76,7 +76,7 @@ void EnabledDebugList::Parse(const std::string& cats, bool enabled) {
   {                                                                            \
     static const std::string available_category = ToLower(#name);              \
     if (available_category.find(wanted) != std::string::npos)                  \
-      set_enabled(DebugCategory::name, enabled);                               \
+      set_enabled(DebugCategory::name);                                        \
   }
 
     DEBUG_CATEGORY_NAMES(V)
diff --git a/src/debug_utils.h b/src/debug_utils.h
@@ -51,20 +51,21 @@ void NODE_EXTERN_PRIVATE FWrite(FILE* file, const std::string& str);
   V(WASI)                                                                      \
   V(MKSNAPSHOT)
 
-enum class DebugCategory {
+enum class DebugCategory : unsigned int {
 #define V(name) name,
   DEBUG_CATEGORY_NAMES(V)
 #undef V
-      CATEGORY_COUNT
 };
 
+#define V(name) +1
+constexpr unsigned int kDebugCategoryCount = DEBUG_CATEGORY_NAMES(V);
+#undef V
+
 class NODE_EXTERN_PRIVATE EnabledDebugList {
  public:
-  bool enabled(DebugCategory category) const {
-    DCHECK_GE(static_cast<int>(category), 0);
-    DCHECK_LT(static_cast<int>(category),
-              static_cast<int>(DebugCategory::CATEGORY_COUNT));
-    return enabled_[static_cast<int>(category)];
+  bool FORCE_INLINE enabled(DebugCategory category) const {
+    DCHECK_LT(static_cast<unsigned int>(category), kDebugCategoryCount);
+    return enabled_[static_cast<unsigned int>(category)];
   }
 
   // Uses NODE_DEBUG_NATIVE to initialize the categories. The env_vars variable
@@ -74,16 +75,14 @@ class NODE_EXTERN_PRIVATE EnabledDebugList {
              v8::Isolate* isolate = nullptr);
 
  private:
-  // Set all categories matching cats to the value of enabled.
-  void Parse(const std::string& cats, bool enabled);
-  void set_enabled(DebugCategory category, bool enabled) {
-    DCHECK_GE(static_cast<int>(category), 0);
-    DCHECK_LT(static_cast<int>(category),
-              static_cast<int>(DebugCategory::CATEGORY_COUNT));
+  // Enable all categories matching cats.
+  void Parse(const std::string& cats);
+  void set_enabled(DebugCategory category) {
+    DCHECK_LT(static_cast<unsigned int>(category), kDebugCategoryCount);
     enabled_[static_cast<int>(category)] = true;
   }
 
-  bool enabled_[static_cast<int>(DebugCategory::CATEGORY_COUNT)] = {false};
+  bool enabled_[kDebugCategoryCount] = {false};
 };
 
 template <typename... Args>
