jgorbe created this revision.
jgorbe added reviewers: labath, rupprecht, jingham.
jgorbe added a project: LLDB.
Herald added a subscriber: JDevlieghere.
Herald added a project: All.
jgorbe requested review of this revision.



- Merge pairs like `eFormatCategoryItemSummary` and 
`eFormatCategoryItemRegexSummary` into a single value. See explanation below.

- Rename `eFormatCategoryItemValue` to `eFormatCategoryItemFormat`. This makes 
the enum match the names used elsewhere for formatter kinds (format, summary, 
filter, synth).

- Delete unused values `eFormatCategoryItemValidator` and 
`eFormatCategoryItemRegexValidator`.

This enum is only used to reuse some code in CommandObjectType.cpp.  For
example, instead of having separate implementations for `type summary
delete`, `type format delete`, and so on, there's a single generic
implementation that takes an enum value, and then the specific commands
derive from it and set the right flags for the specific kind of
formatter.

Even though the enum distinguishes between regular and regex matches for
every kind of formatter, this distinction is never used: enum values are
always specified in pairs like
`eFormatCategoryItemSummary | eFormatCategoryItemRegexSummary`.

This causes some ugly code duplication in TypeCategory.cpp. In order to
handle every flag combination some code appears 8 times:

{format, summary, synth, filter} x {exact, regex}


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134244

Files:
  lldb/include/lldb/lldb-private-enumerations.h
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/DataFormatters/DataVisualization.cpp
  lldb/source/DataFormatters/TypeCategory.cpp

Index: lldb/source/DataFormatters/TypeCategory.cpp
===================================================================
--- lldb/source/DataFormatters/TypeCategory.cpp
+++ lldb/source/DataFormatters/TypeCategory.cpp
@@ -142,53 +142,49 @@
 }
 
 void TypeCategoryImpl::Clear(FormatCategoryItems items) {
-  if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue)
+  if (items & eFormatCategoryItemFormat) {
     GetTypeFormatsContainer()->Clear();
-  if ((items & eFormatCategoryItemRegexValue) == eFormatCategoryItemRegexValue)
     GetRegexTypeFormatsContainer()->Clear();
+  }
 
-  if ((items & eFormatCategoryItemSummary) == eFormatCategoryItemSummary)
+  if (items & eFormatCategoryItemSummary) {
     GetTypeSummariesContainer()->Clear();
-  if ((items & eFormatCategoryItemRegexSummary) ==
-      eFormatCategoryItemRegexSummary)
     GetRegexTypeSummariesContainer()->Clear();
+  }
 
-  if ((items & eFormatCategoryItemFilter) == eFormatCategoryItemFilter)
+  if (items & eFormatCategoryItemFilter) {
     GetTypeFiltersContainer()->Clear();
-  if ((items & eFormatCategoryItemRegexFilter) ==
-      eFormatCategoryItemRegexFilter)
     GetRegexTypeFiltersContainer()->Clear();
+  }
 
-  if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth)
+  if (items & eFormatCategoryItemSynth) {
     GetTypeSyntheticsContainer()->Clear();
-  if ((items & eFormatCategoryItemRegexSynth) == eFormatCategoryItemRegexSynth)
     GetRegexTypeSyntheticsContainer()->Clear();
+  }
 }
 
 bool TypeCategoryImpl::Delete(ConstString name, FormatCategoryItems items) {
   bool success = false;
 
-  if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue)
+  if (items & eFormatCategoryItemFormat) {
     success = GetTypeFormatsContainer()->Delete(name) || success;
-  if ((items & eFormatCategoryItemRegexValue) == eFormatCategoryItemRegexValue)
     success = GetRegexTypeFormatsContainer()->Delete(name) || success;
+  }
 
-  if ((items & eFormatCategoryItemSummary) == eFormatCategoryItemSummary)
+  if (items & eFormatCategoryItemSummary) {
     success = GetTypeSummariesContainer()->Delete(name) || success;
-  if ((items & eFormatCategoryItemRegexSummary) ==
-      eFormatCategoryItemRegexSummary)
     success = GetRegexTypeSummariesContainer()->Delete(name) || success;
+  }
 
-  if ((items & eFormatCategoryItemFilter) == eFormatCategoryItemFilter)
+  if (items & eFormatCategoryItemFilter) {
     success = GetTypeFiltersContainer()->Delete(name) || success;
-  if ((items & eFormatCategoryItemRegexFilter) ==
-      eFormatCategoryItemRegexFilter)
     success = GetRegexTypeFiltersContainer()->Delete(name) || success;
+  }
 
-  if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth)
+  if (items & eFormatCategoryItemSynth) {
     success = GetTypeSyntheticsContainer()->Delete(name) || success;
-  if ((items & eFormatCategoryItemRegexSynth) == eFormatCategoryItemRegexSynth)
     success = GetRegexTypeSyntheticsContainer()->Delete(name) || success;
+  }
 
   return success;
 }
@@ -196,27 +192,25 @@
 uint32_t TypeCategoryImpl::GetCount(FormatCategoryItems items) {
   uint32_t count = 0;
 
-  if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue)
+  if (items & eFormatCategoryItemFormat) {
     count += GetTypeFormatsContainer()->GetCount();
-  if ((items & eFormatCategoryItemRegexValue) == eFormatCategoryItemRegexValue)
     count += GetRegexTypeFormatsContainer()->GetCount();
+  }
 
-  if ((items & eFormatCategoryItemSummary) == eFormatCategoryItemSummary)
+  if (items & eFormatCategoryItemSummary) {
     count += GetTypeSummariesContainer()->GetCount();
-  if ((items & eFormatCategoryItemRegexSummary) ==
-      eFormatCategoryItemRegexSummary)
     count += GetRegexTypeSummariesContainer()->GetCount();
+  }
 
-  if ((items & eFormatCategoryItemFilter) == eFormatCategoryItemFilter)
+  if (items & eFormatCategoryItemFilter) {
     count += GetTypeFiltersContainer()->GetCount();
-  if ((items & eFormatCategoryItemRegexFilter) ==
-      eFormatCategoryItemRegexFilter)
     count += GetRegexTypeFiltersContainer()->GetCount();
+  }
 
-  if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth)
+  if (items & eFormatCategoryItemSynth) {
     count += GetTypeSyntheticsContainer()->GetCount();
-  if ((items & eFormatCategoryItemRegexSynth) == eFormatCategoryItemRegexSynth)
     count += GetRegexTypeSyntheticsContainer()->GetCount();
+  }
 
   return count;
 }
@@ -233,28 +227,20 @@
   TypeFilterImpl::SharedPointer filter_sp;
   ScriptedSyntheticChildren::SharedPointer synth_sp;
 
-  if ((items & eFormatCategoryItemValue) == eFormatCategoryItemValue) {
-    if (GetTypeFormatsContainer()->Get(type_name, format_sp)) {
-      if (matching_category)
-        *matching_category = m_name.GetCString();
-      if (matching_type)
-        *matching_type = eFormatCategoryItemValue;
-      return true;
-    }
-  }
-  if ((items & eFormatCategoryItemRegexValue) ==
-      eFormatCategoryItemRegexValue) {
-    if (GetRegexTypeFormatsContainer()->Get(type_name, format_sp)) {
+  if (items & eFormatCategoryItemFormat) {
+    if (GetTypeFormatsContainer()->Get(type_name, format_sp) ||
+        GetRegexTypeFormatsContainer()->Get(type_name, format_sp)) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
-        *matching_type = eFormatCategoryItemRegexValue;
+        *matching_type = eFormatCategoryItemFormat;
       return true;
     }
   }
 
-  if ((items & eFormatCategoryItemSummary) == eFormatCategoryItemSummary) {
-    if (GetTypeSummariesContainer()->Get(type_name, summary_sp)) {
+  if (items & eFormatCategoryItemSummary) {
+    if (GetTypeSummariesContainer()->Get(type_name, summary_sp) ||
+        GetRegexTypeSummariesContainer()->Get(type_name, summary_sp)) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
@@ -262,19 +248,10 @@
       return true;
     }
   }
-  if ((items & eFormatCategoryItemRegexSummary) ==
-      eFormatCategoryItemRegexSummary) {
-    if (GetRegexTypeSummariesContainer()->Get(type_name, summary_sp)) {
-      if (matching_category)
-        *matching_category = m_name.GetCString();
-      if (matching_type)
-        *matching_type = eFormatCategoryItemRegexSummary;
-      return true;
-    }
-  }
 
-  if ((items & eFormatCategoryItemFilter) == eFormatCategoryItemFilter) {
-    if (GetTypeFiltersContainer()->Get(type_name, filter_sp)) {
+  if (items & eFormatCategoryItemFilter) {
+    if (GetTypeFiltersContainer()->Get(type_name, filter_sp) ||
+        GetRegexTypeFiltersContainer()->Get(type_name, filter_sp)) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
@@ -282,19 +259,10 @@
       return true;
     }
   }
-  if ((items & eFormatCategoryItemRegexFilter) ==
-      eFormatCategoryItemRegexFilter) {
-    if (GetRegexTypeFiltersContainer()->Get(type_name, filter_sp)) {
-      if (matching_category)
-        *matching_category = m_name.GetCString();
-      if (matching_type)
-        *matching_type = eFormatCategoryItemRegexFilter;
-      return true;
-    }
-  }
 
-  if ((items & eFormatCategoryItemSynth) == eFormatCategoryItemSynth) {
-    if (GetTypeSyntheticsContainer()->Get(type_name, synth_sp)) {
+  if (items & eFormatCategoryItemSynth) {
+    if (GetTypeSyntheticsContainer()->Get(type_name, synth_sp) ||
+        GetRegexTypeSyntheticsContainer()->Get(type_name, synth_sp)) {
       if (matching_category)
         *matching_category = m_name.GetCString();
       if (matching_type)
@@ -302,16 +270,6 @@
       return true;
     }
   }
-  if ((items & eFormatCategoryItemRegexSynth) ==
-      eFormatCategoryItemRegexSynth) {
-    if (GetRegexTypeSyntheticsContainer()->Get(type_name, synth_sp)) {
-      if (matching_category)
-        *matching_category = m_name.GetCString();
-      if (matching_type)
-        *matching_type = eFormatCategoryItemRegexSynth;
-      return true;
-    }
-  }
 
   return false;
 }
Index: lldb/source/DataFormatters/DataVisualization.cpp
===================================================================
--- lldb/source/DataFormatters/DataVisualization.cpp
+++ lldb/source/DataFormatters/DataVisualization.cpp
@@ -102,8 +102,7 @@
 }
 
 void DataVisualization::Categories::Clear(ConstString category) {
-  GetFormatManager().GetCategory(category)->Clear(
-      eFormatCategoryItemSummary | eFormatCategoryItemRegexSummary);
+  GetFormatManager().GetCategory(category)->Clear(eFormatCategoryItemSummary);
 }
 
 void DataVisualization::Categories::Enable(ConstString category,
Index: lldb/source/Commands/CommandObjectType.cpp
===================================================================
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -783,27 +783,27 @@
 
     DataVisualization::Categories::ForEach(
         [this, &request](const lldb::TypeCategoryImplSP &category_sp) {
-          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemValue))
+          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemFormat)) {
             category_sp->GetTypeFormatsContainer()->AutoComplete(request);
-          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemRegexValue))
             category_sp->GetRegexTypeFormatsContainer()->AutoComplete(request);
+          }
 
-          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemSummary))
+          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemSummary)) {
             category_sp->GetTypeSummariesContainer()->AutoComplete(request);
-          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemRegexSummary))
             category_sp->GetRegexTypeSummariesContainer()->AutoComplete(
                 request);
+          }
 
-          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemFilter))
+          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemFilter)) {
             category_sp->GetTypeFiltersContainer()->AutoComplete(request);
-          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemRegexFilter))
             category_sp->GetRegexTypeFiltersContainer()->AutoComplete(request);
+          }
 
-          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemSynth))
+          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemSynth)) {
             category_sp->GetTypeSyntheticsContainer()->AutoComplete(request);
-          if (CHECK_FORMATTER_KIND_MASK(eFormatCategoryItemRegexSynth))
             category_sp->GetRegexTypeSyntheticsContainer()->AutoComplete(
                 request);
+          }
           return true;
         });
   }
@@ -958,9 +958,7 @@
 public:
   CommandObjectTypeFormatDelete(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterDelete(
-            interpreter,
-            eFormatCategoryItemValue | eFormatCategoryItemRegexValue,
-            "type format delete",
+            interpreter, eFormatCategoryItemFormat, "type format delete",
             "Delete an existing formatting style for a type.") {}
 
   ~CommandObjectTypeFormatDelete() override = default;
@@ -971,10 +969,9 @@
 class CommandObjectTypeFormatClear : public CommandObjectTypeFormatterClear {
 public:
   CommandObjectTypeFormatClear(CommandInterpreter &interpreter)
-      : CommandObjectTypeFormatterClear(
-            interpreter,
-            eFormatCategoryItemValue | eFormatCategoryItemRegexValue,
-            "type format clear", "Delete all existing format styles.") {}
+      : CommandObjectTypeFormatterClear(interpreter, eFormatCategoryItemFormat,
+                                        "type format clear",
+                                        "Delete all existing format styles.") {}
 };
 
 #define LLDB_OPTIONS_type_formatter_list
@@ -1629,9 +1626,8 @@
 public:
   CommandObjectTypeSummaryDelete(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterDelete(
-            interpreter,
-            eFormatCategoryItemSummary | eFormatCategoryItemRegexSummary,
-            "type summary delete", "Delete an existing summary for a type.") {}
+            interpreter, eFormatCategoryItemSummary, "type summary delete",
+            "Delete an existing summary for a type.") {}
 
   ~CommandObjectTypeSummaryDelete() override = default;
 
@@ -1646,10 +1642,9 @@
 class CommandObjectTypeSummaryClear : public CommandObjectTypeFormatterClear {
 public:
   CommandObjectTypeSummaryClear(CommandInterpreter &interpreter)
-      : CommandObjectTypeFormatterClear(
-            interpreter,
-            eFormatCategoryItemSummary | eFormatCategoryItemRegexSummary,
-            "type summary clear", "Delete all existing summaries.") {}
+      : CommandObjectTypeFormatterClear(interpreter, eFormatCategoryItemSummary,
+                                        "type summary clear",
+                                        "Delete all existing summaries.") {}
 
 protected:
   void FormatterSpecificDeletion() override {
@@ -2181,9 +2176,8 @@
 public:
   CommandObjectTypeFilterDelete(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterDelete(
-            interpreter,
-            eFormatCategoryItemFilter | eFormatCategoryItemRegexFilter,
-            "type filter delete", "Delete an existing filter for a type.") {}
+            interpreter, eFormatCategoryItemFilter, "type filter delete",
+            "Delete an existing filter for a type.") {}
 
   ~CommandObjectTypeFilterDelete() override = default;
 };
@@ -2196,9 +2190,7 @@
 public:
   CommandObjectTypeSynthDelete(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterDelete(
-            interpreter,
-            eFormatCategoryItemSynth | eFormatCategoryItemRegexSynth,
-            "type synthetic delete",
+            interpreter, eFormatCategoryItemSynth, "type synthetic delete",
             "Delete an existing synthetic provider for a type.") {}
 
   ~CommandObjectTypeSynthDelete() override = default;
@@ -2211,10 +2203,9 @@
 class CommandObjectTypeFilterClear : public CommandObjectTypeFormatterClear {
 public:
   CommandObjectTypeFilterClear(CommandInterpreter &interpreter)
-      : CommandObjectTypeFormatterClear(
-            interpreter,
-            eFormatCategoryItemFilter | eFormatCategoryItemRegexFilter,
-            "type filter clear", "Delete all existing filter.") {}
+      : CommandObjectTypeFormatterClear(interpreter, eFormatCategoryItemFilter,
+                                        "type filter clear",
+                                        "Delete all existing filter.") {}
 };
 
 #if LLDB_ENABLE_PYTHON
@@ -2224,9 +2215,7 @@
 public:
   CommandObjectTypeSynthClear(CommandInterpreter &interpreter)
       : CommandObjectTypeFormatterClear(
-            interpreter,
-            eFormatCategoryItemSynth | eFormatCategoryItemRegexSynth,
-            "type synthetic clear",
+            interpreter, eFormatCategoryItemSynth, "type synthetic clear",
             "Delete all existing synthetic providers.") {}
 };
 
@@ -2346,9 +2335,7 @@
       type = eRegexSynth;
   }
 
-  if (category->AnyMatches(
-          type_name, eFormatCategoryItemFilter | eFormatCategoryItemRegexFilter,
-          false)) {
+  if (category->AnyMatches(type_name, eFormatCategoryItemFilter, false)) {
     if (error)
       error->SetErrorStringWithFormat("cannot add synthetic for type %s when "
                                       "filter is defined in same category!",
@@ -2471,9 +2458,7 @@
         type = eRegexFilter;
     }
 
-    if (category->AnyMatches(
-            type_name, eFormatCategoryItemSynth | eFormatCategoryItemRegexSynth,
-            false)) {
+    if (category->AnyMatches(type_name, eFormatCategoryItemSynth, false)) {
       if (error)
         error->SetErrorStringWithFormat("cannot add filter for type %s when "
                                         "synthetic is defined in same "
Index: lldb/include/lldb/lldb-private-enumerations.h
===================================================================
--- lldb/include/lldb/lldb-private-enumerations.h
+++ lldb/include/lldb/lldb-private-enumerations.h
@@ -129,15 +129,9 @@
 /// Format category entry types
 enum FormatCategoryItem {
   eFormatCategoryItemSummary = 0x0001,
-  eFormatCategoryItemRegexSummary = 0x0002,
   eFormatCategoryItemFilter = 0x0004,
-  eFormatCategoryItemRegexFilter = 0x0008,
   eFormatCategoryItemSynth = 0x0010,
-  eFormatCategoryItemRegexSynth = 0x0020,
-  eFormatCategoryItemValue = 0x0040,
-  eFormatCategoryItemRegexValue = 0x0080,
-  eFormatCategoryItemValidator = 0x0100,
-  eFormatCategoryItemRegexValidator = 0x0200
+  eFormatCategoryItemFormat = 0x0040,
 };
 
 /// Expression execution policies
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to