[Lldb-commits] [lldb] dce6887 - [NFCI] Clean up enum FormatCategoryItem.

Jorge Gorbe Moya via lldb-commits lldb-commits at lists.llvm.org
Tue Sep 20 10:43:46 PDT 2022


Author: Jorge Gorbe Moya
Date: 2022-09-20T10:41:06-07:00
New Revision: dce68873360097be77ff8b3ce68b6eedc612fbef

URL: https://github.com/llvm/llvm-project/commit/dce68873360097be77ff8b3ce68b6eedc612fbef
DIFF: https://github.com/llvm/llvm-project/commit/dce68873360097be77ff8b3ce68b6eedc612fbef.diff

LOG: [NFCI] Clean up enum FormatCategoryItem.

- 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}

Differential Revision: https://reviews.llvm.org/D134244

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/lldb-private-enumerations.h b/lldb/include/lldb/lldb-private-enumerations.h
index 4a26127bebd6a..179b8b944cc68 100644
--- a/lldb/include/lldb/lldb-private-enumerations.h
+++ b/lldb/include/lldb/lldb-private-enumerations.h
@@ -128,16 +128,10 @@ enum InstructionType {
 
 /// 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
+  eFormatCategoryItemSummary = 1,
+  eFormatCategoryItemFilter = 1 << 1,
+  eFormatCategoryItemSynth = 1 << 2,
+  eFormatCategoryItemFormat = 1 << 3,
 };
 
 /// Expression execution policies

diff  --git a/lldb/source/Commands/CommandObjectType.cpp b/lldb/source/Commands/CommandObjectType.cpp
index 11acbb5c627f6..a285835aa277e 100644
--- a/lldb/source/Commands/CommandObjectType.cpp
+++ b/lldb/source/Commands/CommandObjectType.cpp
@@ -783,27 +783,27 @@ class CommandObjectTypeFormatterDelete : public CommandObjectParsed {
 
     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 @@ class CommandObjectTypeFormatDelete : public CommandObjectTypeFormatterDelete {
 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 CommandObjectTypeFormatDelete : public CommandObjectTypeFormatterDelete {
 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 @@ class CommandObjectTypeSummaryDelete : public CommandObjectTypeFormatterDelete {
 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 CommandObjectTypeSummaryDelete : public CommandObjectTypeFormatterDelete {
 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 @@ class CommandObjectTypeFilterDelete : public CommandObjectTypeFormatterDelete {
 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 @@ class CommandObjectTypeSynthDelete : public CommandObjectTypeFormatterDelete {
 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 CommandObjectTypeSynthDelete : public CommandObjectTypeFormatterDelete {
 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 @@ class CommandObjectTypeSynthClear : public CommandObjectTypeFormatterClear {
 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 @@ bool CommandObjectTypeSynthAdd::AddSynth(ConstString type_name,
       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 @@ class CommandObjectTypeFilterAdd : public CommandObjectParsed {
         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 "

diff  --git a/lldb/source/DataFormatters/DataVisualization.cpp b/lldb/source/DataFormatters/DataVisualization.cpp
index ded8bbd90391a..53832492aa253 100644
--- a/lldb/source/DataFormatters/DataVisualization.cpp
+++ b/lldb/source/DataFormatters/DataVisualization.cpp
@@ -102,8 +102,7 @@ void DataVisualization::Categories::Clear() {
 }
 
 void DataVisualization::Categories::Clear(ConstString category) {
-  GetFormatManager().GetCategory(category)->Clear(
-      eFormatCategoryItemSummary | eFormatCategoryItemRegexSummary);
+  GetFormatManager().GetCategory(category)->Clear(eFormatCategoryItemSummary);
 }
 
 void DataVisualization::Categories::Enable(ConstString category,

diff  --git a/lldb/source/DataFormatters/TypeCategory.cpp b/lldb/source/DataFormatters/TypeCategory.cpp
index f1c6210edd1a7..f9f8cbb364a0f 100644
--- a/lldb/source/DataFormatters/TypeCategory.cpp
+++ b/lldb/source/DataFormatters/TypeCategory.cpp
@@ -142,53 +142,49 @@ bool TypeCategoryImpl::Get(lldb::LanguageType lang,
 }
 
 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 @@ bool TypeCategoryImpl::Delete(ConstString name, FormatCategoryItems items) {
 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 @@ bool TypeCategoryImpl::AnyMatches(ConstString type_name,
   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 @@ bool TypeCategoryImpl::AnyMatches(ConstString type_name,
       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 @@ bool TypeCategoryImpl::AnyMatches(ConstString type_name,
       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 @@ bool TypeCategoryImpl::AnyMatches(ConstString type_name,
       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;
 }


        


More information about the lldb-commits mailing list