Move FormattersMatchCandidate flags to a struct.

[jgorbe]

Summary

Move FormattersMatchCandidate flags to a struct.

This removes some error-prone repetition in
FormatManager::GetPossibleMatches, where the same three boolean flags
are passed in a row multiple times as arguments to recursive calls to
GetPossibleMatches.

Instead of:

// same flags, but with did_strip_typedef set to true.
GetPossibleMatches(..., did_strip_ptr, did_strip_ref, true);

we can now say

GetPossibleMatches(..., current_flags.WithStrippedTypedef());

which hopefully makes the intent clearer, and more readable in case we
add another flag.

[DavidSpickett]

LGTM, a nice improvement.

[labath]

lldb/include/lldb/DataFormatters/FormatClasses.h

50

We don't generally use the m_ prefix on structs. This struct is a bit of an edge case, as it has non-trivial member functions -- so one could consider making it a class (members private and public accessors) -- but if you don't want to do that (which is fine by me), then I think it'd be better to remove the prefixes.

lldb/include/lldb/DataFormatters/FormatClasses.h

public:
 
   typedef HardcodedFormatterFinders<TypeFormatImpl> HardcodedFormatFinder;
   typedef HardcodedFormatterFinders<TypeSummaryImpl> HardcodedSummaryFinder;
   typedef HardcodedFormatterFinders<SyntheticChildren> HardcodedSyntheticFinder;
 };
 
 class FormattersMatchCandidate {
 public:
-  FormattersMatchCandidate(ConstString name, bool strip_ptr,
-                           bool strip_ref, bool strip_tydef)
-      : m_type_name(name), m_stripped_pointer(strip_ptr),
-        m_stripped_reference(strip_ref), m_stripped_typedef(strip_tydef) {}
+  // Contains flags to indicate how this candidate was generated (e.g. if
+  // typedefs were stripped, or pointers were skipped). These are later compared
+  // to flags in formatters to confirm a string match.
+  struct Flags {
+    bool stripped_pointer = false;
+    bool stripped_reference = false;
+    bool stripped_typedef = false;
+
+    // Returns a copy of this with the "stripped pointer" flag set.
+    Flags WithStrippedPointer() {
+      Flags result(*this);
+      result.stripped_pointer = true;
+      return result;
+    }
+
+    // Returns a copy of this with the "stripped reference" flag set.
+    Flags WithStrippedReference() {
+      Flags result(*this);
+      result.stripped_reference = true;
+      return result;
+    }
+
+    // Returns a copy of this with the "stripped typedef" flag set.
+    Flags WithStrippedTypedef() {
+      Flags result(*this);
+      result.stripped_typedef = true;
+      return result;
+    }
+  };
+
+  FormattersMatchCandidate(ConstString name, Flags flags)
+      : m_type_name(name), m_flags(flags) {}
 
   ~FormattersMatchCandidate() = default;
 
   ConstString GetTypeName() const { return m_type_name; }
 
-  bool DidStripPointer() const { return m_stripped_pointer; }
+  bool DidStripPointer() const { return m_flags.stripped_pointer; }
 
-  bool DidStripReference() const { return m_stripped_reference; }
+  bool DidStripReference() const { return m_flags.stripped_reference; }
 
-  bool DidStripTypedef() const { return m_stripped_typedef; }
+  bool DidStripTypedef() const { return m_flags.stripped_typedef; }
 
   template <class Formatter>
   bool IsMatch(const std::shared_ptr<Formatter> &formatter_sp) const {
     if (!formatter_sp)
       return false;
     if (formatter_sp->Cascades() == false && DidStripTypedef())
       return false;
     if (formatter_sp->SkipsPointers() && DidStripPointer())
       return false;
     if (formatter_sp->SkipsReferences() && DidStripReference())
       return false;
     return true;
   }
 
 private:
   ConstString m_type_name;
-  bool m_stripped_pointer;
-  bool m_stripped_reference;
-  bool m_stripped_typedef;
+  Flags m_flags;
 };
 
 typedef std::vector<FormattersMatchCandidate> FormattersMatchVector;
 typedef std::vector<lldb::LanguageType> CandidateLanguagesVector;
 
 class FormattersMatchData {
 public:
   FormattersMatchData(ValueObject &, lldb::DynamicValueType);

lldb/include/lldb/DataFormatters/FormatManager.h

public:
 
   void Changed() override;
 
   uint32_t GetCurrentRevision() override { return m_last_revision; }
 
   static FormattersMatchVector
   GetPossibleMatches(ValueObject &valobj, lldb::DynamicValueType use_dynamic) {
     FormattersMatchVector matches;
-    GetPossibleMatches(valobj, valobj.GetCompilerType(),
-                       use_dynamic, matches, false, false, false, true);
+    GetPossibleMatches(valobj, valobj.GetCompilerType(), use_dynamic, matches,
+                       FormattersMatchCandidate::Flags(), true);
     return matches;
   }
 
   static ConstString GetTypeForCache(ValueObject &, lldb::DynamicValueType);
 
   LanguageCategory *GetCategoryForLanguage(lldb::LanguageType lang_type);
 
   static std::vector<lldb::LanguageType>
   GetCandidateLanguages(lldb::LanguageType lang_type);
 
 private:
   static void GetPossibleMatches(ValueObject &valobj,
                                  CompilerType compiler_type,
                                  lldb::DynamicValueType use_dynamic,
                                  FormattersMatchVector &entries,
-                                 bool did_strip_ptr, bool did_strip_ref,
-                                 bool did_strip_typedef,
+                                 FormattersMatchCandidate::Flags current_flags,
                                  bool root_level = false);
 
   std::atomic<uint32_t> m_last_revision;
   FormatCache m_format_cache;
   std::recursive_mutex m_language_categories_mutex;
   LanguageCategories m_language_categories_map;
   NamedSummariesMap m_named_summaries_map;
   TypeCategoryMap m_categories_map;

lldb/source/DataFormatters/FormatManager.cpp

for (auto &iter : m_language_categories_map) {
     if (iter.second)
       iter.second->Disable();
   }
 }
 
 void FormatManager::GetPossibleMatches(
     ValueObject &valobj, CompilerType compiler_type,
     lldb::DynamicValueType use_dynamic, FormattersMatchVector &entries,
-    bool did_strip_ptr, bool did_strip_ref, bool did_strip_typedef,
-    bool root_level) {
+    FormattersMatchCandidate::Flags current_flags, bool root_level) {
   compiler_type = compiler_type.GetTypeForFormatters();
   ConstString type_name(compiler_type.GetTypeName());
   if (valobj.GetBitfieldBitSize() > 0) {
     StreamString sstring;
     sstring.Printf("%s:%d", type_name.AsCString(), valobj.GetBitfieldBitSize());
     ConstString bitfieldname(sstring.GetString());
-    entries.push_back(
-        {bitfieldname, did_strip_ptr, did_strip_ref, did_strip_typedef});
+    entries.push_back({bitfieldname, current_flags});
   }
 
   if (!compiler_type.IsMeaninglessWithoutDynamicResolution()) {
-    entries.push_back(
-        {type_name, did_strip_ptr, did_strip_ref, did_strip_typedef});
+    entries.push_back({type_name, current_flags});
 
     ConstString display_type_name(compiler_type.GetTypeName());
     if (display_type_name != type_name)
-      entries.push_back({display_type_name, did_strip_ptr,
-                         did_strip_ref, did_strip_typedef});
+      entries.push_back({display_type_name, current_flags});
   }
 
   for (bool is_rvalue_ref = true, j = true;
        j && compiler_type.IsReferenceType(nullptr, &is_rvalue_ref); j = false) {
     CompilerType non_ref_type = compiler_type.GetNonReferenceType();
-    GetPossibleMatches(
-        valobj, non_ref_type,
-        use_dynamic, entries, did_strip_ptr, true, did_strip_typedef);
+    GetPossibleMatches(valobj, non_ref_type, use_dynamic, entries,
+                       current_flags.WithStrippedReference());
     if (non_ref_type.IsTypedefType()) {
       CompilerType deffed_referenced_type = non_ref_type.GetTypedefedType();
       deffed_referenced_type =
           is_rvalue_ref ? deffed_referenced_type.GetRValueReferenceType()
                         : deffed_referenced_type.GetLValueReferenceType();
+      // this is not exactly the usual meaning of stripping typedefs
       GetPossibleMatches(
           valobj, deffed_referenced_type,
-          use_dynamic, entries, did_strip_ptr, did_strip_ref,
-          true); // this is not exactly the usual meaning of stripping typedefs
+          use_dynamic, entries, current_flags.WithStrippedTypedef());
     }
   }
 
   if (compiler_type.IsPointerType()) {
     CompilerType non_ptr_type = compiler_type.GetPointeeType();
-    GetPossibleMatches(
-        valobj, non_ptr_type,
-        use_dynamic, entries, true, did_strip_ref, did_strip_typedef);
+    GetPossibleMatches(valobj, non_ptr_type, use_dynamic, entries,
+                       current_flags.WithStrippedPointer());
     if (non_ptr_type.IsTypedefType()) {
       CompilerType deffed_pointed_type =
           non_ptr_type.GetTypedefedType().GetPointerType();
-      const bool stripped_typedef = true;
-      GetPossibleMatches(
-          valobj, deffed_pointed_type,
-          use_dynamic, entries, did_strip_ptr, did_strip_ref,
-          stripped_typedef); // this is not exactly the usual meaning of
-                             // stripping typedefs
+      // this is not exactly the usual meaning of stripping typedefs
+      GetPossibleMatches(valobj, deffed_pointed_type, use_dynamic, entries,
+                         current_flags.WithStrippedTypedef());
     }
   }
 
   // For arrays with typedef-ed elements, we add a candidate with the typedef
   // stripped.
   uint64_t array_size;
   if (compiler_type.IsArrayType(nullptr, &array_size, nullptr)) {
     ExecutionContext exe_ctx(valobj.GetExecutionContextRef());
     CompilerType element_type = compiler_type.GetArrayElementType(
         exe_ctx.GetBestExecutionContextScope());
     if (element_type.IsTypedefType()) {
       // Get the stripped element type and compute the stripped array type
       // from it.
       CompilerType deffed_array_type =
           element_type.GetTypedefedType().GetArrayType(array_size);
-      const bool stripped_typedef = true;
+      // this is not exactly the usual meaning of stripping typedefs
       GetPossibleMatches(
           valobj, deffed_array_type,
-          use_dynamic, entries, did_strip_ptr, did_strip_ref,
-          stripped_typedef); // this is not exactly the usual meaning of
-                             // stripping typedefs
+          use_dynamic, entries, current_flags.WithStrippedTypedef());
     }
   }
 
   for (lldb::LanguageType language_type :
        GetCandidateLanguages(valobj.GetObjectRuntimeLanguage())) {
     if (Language *language = Language::FindPlugin(language_type)) {
       for (ConstString candidate :
            language->GetPossibleFormattersMatches(valobj, use_dynamic)) {
-        entries.push_back(
-            {candidate,
-             did_strip_ptr, did_strip_ref, did_strip_typedef});
+        entries.push_back({candidate, current_flags});
       }
     }
   }
 
   // try to strip typedef chains
   if (compiler_type.IsTypedefType()) {
     CompilerType deffed_type = compiler_type.GetTypedefedType();
-    GetPossibleMatches(
-        valobj, deffed_type,
-        use_dynamic, entries, did_strip_ptr, did_strip_ref, true);
+    GetPossibleMatches(valobj, deffed_type, use_dynamic, entries,
+                       current_flags.WithStrippedTypedef());
   }
 
   if (root_level) {
     do {
       if (!compiler_type.IsValid())
         break;
 
       CompilerType unqual_compiler_ast_type =
           compiler_type.GetFullyUnqualifiedType();
       if (!unqual_compiler_ast_type.IsValid())
         break;
       if (unqual_compiler_ast_type.GetOpaqueQualType() !=
           compiler_type.GetOpaqueQualType())
-        GetPossibleMatches(valobj, unqual_compiler_ast_type,
-                           use_dynamic, entries, did_strip_ptr, did_strip_ref,
-                           did_strip_typedef);
+        GetPossibleMatches(valobj, unqual_compiler_ast_type, use_dynamic,
+                           entries, current_flags);
     } while (false);
 
     // if all else fails, go to static type
     if (valobj.IsDynamic()) {
       lldb::ValueObjectSP static_value_sp(valobj.GetStaticValue());
       if (static_value_sp)
-        GetPossibleMatches(
-            *static_value_sp.get(), static_value_sp->GetCompilerType(),
-            use_dynamic, entries, did_strip_ptr, did_strip_ref,
-            did_strip_typedef, true);
+        GetPossibleMatches(*static_value_sp.get(),
+                           static_value_sp->GetCompilerType(), use_dynamic,
+                           entries, current_flags, true);
     }
   }
 }
 
 lldb::TypeFormatImplSP
 FormatManager::GetFormatForType(lldb::TypeNameSpecifierImplSP type_sp) {
   if (!type_sp)
     return lldb::TypeFormatImplSP();