src: make minor improvements to EnabledDebugList

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: #44350 Reviewed-By: Chengzhong Wu <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Darshan Sen <[email protected]>
nodejs · Sep 5, 2022 · 3eb7918 · 3eb7918
1 parent 2563401
commit 3eb7918
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 17 deletions.
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>