[lldb][NFCI] Remove use of ConstString from OptionValueProperties

[bulbazord]

Summary

In the interest of keeping the ConstString StringPool small, this patch
aims to remove the use of ConstString from OptionValueProperties.

We can maintain quick lookups by using an llvm::StringMap to find the
correct index by name.

[fdeazeve]

LGTM!

[JDevlieghere]

Cool

lldb/include/lldb/Interpreter/OptionValue.h

public:
   }
 
   virtual Status SetSubValue(const ExecutionContext *exe_ctx,
                              VarSetOperationType op, llvm::StringRef name,
                              llvm::StringRef value);
 
   virtual bool IsAggregateValue() const { return false; }
 
-  virtual ConstString GetName() const { return ConstString(); }
+  virtual llvm::StringRef GetName() const { return llvm::StringRef(); }
 
   virtual bool DumpQualifiedName(Stream &strm) const;
 
   // Subclasses should NOT override these functions as they use the above
   // functions to implement functionality
   uint32_t GetTypeAsMask() { return 1u << GetType(); }
 
   static uint32_t ConvertTypeToMask(OptionValue::Type type) {

lldb/include/lldb/Interpreter/OptionValueProperties.h

 #define LLDB_INTERPRETER_OPTIONVALUEPROPERTIES_H
 
 #include <vector>
 
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Core/UniqueCStringMap.h"
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Interpreter/Property.h"
-#include "lldb/Utility/ConstString.h"
 
 namespace lldb_private {
 class Properties;
 
 class OptionValueProperties
     : public Cloneable<OptionValueProperties, OptionValue>,
       public std::enable_shared_from_this<OptionValueProperties> {
 public:
   OptionValueProperties() = default;
 
-  OptionValueProperties(ConstString name);
+  OptionValueProperties(llvm::StringRef name);
 
   ~OptionValueProperties() override = default;
 
   Type GetType() const override { return eTypeProperties; }
 
   void Clear() override;
 
   static lldb::OptionValuePropertiesSP
   CreateLocalCopy(const Properties &global_properties);
 
   lldb::OptionValueSP
   DeepCopy(const lldb::OptionValueSP &new_parent) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
   llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
 
-  ConstString GetName() const override { return m_name; }
+  llvm::StringRef GetName() const override { return m_name; }
 
   virtual Status DumpPropertyValue(const ExecutionContext *exe_ctx,
                                    Stream &strm, llvm::StringRef property_path,
                                    uint32_t dump_mask, bool is_json = false);
 
   virtual void DumpAllDescriptions(CommandInterpreter &interpreter,
                                    Stream &strm) const;
 
   void Apropos(llvm::StringRef keyword,
                std::vector<const Property *> &matching_properties) const;
 
   void Initialize(const PropertyDefinitions &setting_definitions);
 
   // Subclass specific functions
 
   // Get the index of a property given its exact name in this property
   // collection, "name" can't be a path to a property path that refers to a
   // property within a property
-  virtual size_t GetPropertyIndex(ConstString name) const;
+  virtual size_t GetPropertyIndex(llvm::StringRef name) const;
 
   // Get a property by exact name exists in this property collection, name can
   // not be a path to a property path that refers to a property within a
   // property
   virtual const Property *
-  GetProperty(ConstString name,
+  GetProperty(llvm::StringRef name,
               const ExecutionContext *exe_ctx = nullptr) const;
 
   virtual const Property *
   GetPropertyAtIndex(size_t idx,
                      const ExecutionContext *exe_ctx = nullptr) const {
     return ProtectedGetPropertyAtIndex(idx);
   }
 
   // Property can be be a property path like
   // "target.process.extra-startup-command"
   virtual const Property *
   GetPropertyAtPath(const ExecutionContext *exe_ctx,
-
                     llvm::StringRef property_path) const;
 
   virtual lldb::OptionValueSP
   GetPropertyValueAtIndex(size_t idx, const ExecutionContext *exe_ctx) const;
 
   virtual lldb::OptionValueSP GetValueForKey(const ExecutionContext *exe_ctx,
-                                             ConstString key) const;
+                                             llvm::StringRef key) const;
 
   lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
                                   llvm::StringRef name,
                                   Status &error) const override;
 
   Status SetSubValue(const ExecutionContext *exe_ctx, VarSetOperationType op,
                      llvm::StringRef path, llvm::StringRef value) override;
 
OptionValueFileSpec *GetPropertyAtIndexAsOptionValueFileSpec(
       size_t idx, const ExecutionContext *exe_ctx = nullptr) const;
 
   OptionValuePathMappings *GetPropertyAtIndexAsOptionValuePathMappings(
       size_t idx, const ExecutionContext *exe_ctx = nullptr) const;
 
   OptionValueFileSpecList *GetPropertyAtIndexAsOptionValueFileSpecList(
       size_t idx, const ExecutionContext *exe_ctx = nullptr) const;
 
-  void AppendProperty(ConstString name, llvm::StringRef desc, bool is_global,
-                      const lldb::OptionValueSP &value_sp);
+  void AppendProperty(llvm::StringRef name, llvm::StringRef desc,
+                      bool is_global, const lldb::OptionValueSP &value_sp);
 
   lldb::OptionValuePropertiesSP GetSubProperty(const ExecutionContext *exe_ctx,
-                                               ConstString name);
+                                               llvm::StringRef name);
 
   void SetValueChangedCallback(size_t property_idx,
                                std::function<void()> callback);
 
   template <typename T>
   auto GetPropertyAtIndexAs(size_t idx,
                             const ExecutionContext *exe_ctx = nullptr) const {
     if (const Property *property = GetPropertyAtIndex(idx, exe_ctx)) {
Property *ProtectedGetPropertyAtIndex(size_t idx) {
     return ((idx < m_properties.size()) ? &m_properties[idx] : nullptr);
   }
 
   const Property *ProtectedGetPropertyAtIndex(size_t idx) const {
     assert(idx < m_properties.size() && "invalid property index");
     return ((idx < m_properties.size()) ? &m_properties[idx] : nullptr);
   }
 
-  typedef UniqueCStringMap<size_t> NameToIndex;
-
-  ConstString m_name;
+  std::string m_name;
   std::vector<Property> m_properties;
-  NameToIndex m_name_to_index;
+  llvm::StringMap<size_t> m_name_to_index;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEPROPERTIES_H

lldb/source/Interpreter/OptionValueProperties.cpp

 #include "lldb/Interpreter/Property.h"
 #include "lldb/Utility/Args.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
-OptionValueProperties::OptionValueProperties(ConstString name) : m_name(name) {}
+OptionValueProperties::OptionValueProperties(llvm::StringRef name)
+    : m_name(name.str()) {}
 
 void OptionValueProperties::Initialize(const PropertyDefinitions &defs) {
   for (const auto &definition : defs) {
     Property property(definition);
     assert(property.IsValid());
-    m_name_to_index.Append(ConstString(property.GetName()),
-                           m_properties.size());
+    m_name_to_index.insert({property.GetName(), m_properties.size()});
     property.GetValue()->SetParent(shared_from_this());
     m_properties.push_back(property);
   }
-  m_name_to_index.Sort();
 }
 
 void OptionValueProperties::SetValueChangedCallback(
     size_t property_idx, std::function<void()> callback) {
   Property *property = ProtectedGetPropertyAtIndex(property_idx);
   if (property)
     property->SetValueChangedCallback(std::move(callback));
 }
 
-void OptionValueProperties::AppendProperty(ConstString name,
+void OptionValueProperties::AppendProperty(llvm::StringRef name,
                                            llvm::StringRef desc, bool is_global,
                                            const OptionValueSP &value_sp) {
-  Property property(name.GetStringRef(), desc, is_global, value_sp);
-  m_name_to_index.Append(name, m_properties.size());
+  Property property(name, desc, is_global, value_sp);
+  m_name_to_index.insert({name, m_properties.size()});
   m_properties.push_back(property);
   value_sp->SetParent(shared_from_this());
-  m_name_to_index.Sort();
 }
 
 lldb::OptionValueSP
 OptionValueProperties::GetValueForKey(const ExecutionContext *exe_ctx,
-                                      ConstString key) const {
-  lldb::OptionValueSP value_sp;
-  size_t idx = m_name_to_index.Find(key, SIZE_MAX);
-  if (idx < m_properties.size())
-    value_sp = GetPropertyAtIndex(idx, exe_ctx)->GetValue();
-  return value_sp;
+                                      llvm::StringRef key) const {
+  auto iter = m_name_to_index.find(key);
+  if (iter == m_name_to_index.end())
+    return OptionValueSP();
+  const size_t idx = iter->second;
+  if (idx >= m_properties.size())
+    return OptionValueSP();
+  return GetPropertyAtIndex(idx, exe_ctx)->GetValue();
 }
 
 lldb::OptionValueSP
 OptionValueProperties::GetSubValue(const ExecutionContext *exe_ctx,
                                    llvm::StringRef name, Status &error) const {
   lldb::OptionValueSP value_sp;
   if (name.empty())
     return OptionValueSP();
 
   llvm::StringRef sub_name;
-  ConstString key;
+  llvm::StringRef key;
   size_t key_len = name.find_first_of(".[{");
   if (key_len != llvm::StringRef::npos) {
-    key.SetString(name.take_front(key_len));
+    key = name.take_front(key_len);
     sub_name = name.drop_front(key_len);
   } else
-    key.SetString(name);
+    key = name;
 
   value_sp = GetValueForKey(exe_ctx, key);
   if (sub_name.empty() || !value_sp)
     return value_sp;
 
   switch (sub_name[0]) {
   case '.': {
     lldb::OptionValueSP return_val_sp;
else {
     if (!name_contains_experimental && error.AsCString() == nullptr) {
       error.SetErrorStringWithFormat("invalid value path '%s'",
                                      name.str().c_str());
     }
   }
   return error;
 }
 
-size_t OptionValueProperties::GetPropertyIndex(ConstString name) const {
-  return m_name_to_index.Find(name, SIZE_MAX);
+size_t OptionValueProperties::GetPropertyIndex(llvm::StringRef name) const {
+  auto iter = m_name_to_index.find(name);
+  if (iter == m_name_to_index.end())
+    return SIZE_MAX;
+  return iter->second;
 }
 
 const Property *
-OptionValueProperties::GetProperty(ConstString name,
+OptionValueProperties::GetProperty(llvm::StringRef name,
                                    const ExecutionContext *exe_ctx) const {
-  return GetPropertyAtIndex(m_name_to_index.Find(name, SIZE_MAX), exe_ctx);
+  auto iter = m_name_to_index.find(name);
+  if (iter == m_name_to_index.end())
+    return nullptr;
+  return GetPropertyAtIndex(iter->second, exe_ctx);
 }
 
 lldb::OptionValueSP OptionValueProperties::GetPropertyValueAtIndex(
     size_t idx, const ExecutionContext *exe_ctx) const {
   const Property *setting = GetPropertyAtIndex(idx, exe_ctx);
   if (setting)
     return setting->GetValue();
   return OptionValueSP();
for (auto &property : props_value_ptr->m_properties) {
     }
   }
   return copy_sp;
 }
 
 const Property *
 OptionValueProperties::GetPropertyAtPath(const ExecutionContext *exe_ctx,
                                          llvm::StringRef name) const {
-  const Property *property = nullptr;
   if (name.empty())
     return nullptr;
+
+  const Property *property = nullptr;
   llvm::StringRef sub_name;
-  ConstString key;
+  llvm::StringRef key;
   size_t key_len = name.find_first_of(".[{");
 
   if (key_len != llvm::StringRef::npos) {
-    key.SetString(name.take_front(key_len));
+    key = name.take_front(key_len);
     sub_name = name.drop_front(key_len);
   } else
-    key.SetString(name);
+    key = name;
 
   property = GetProperty(key, exe_ctx);
   if (sub_name.empty() || !property)
     return property;
 
   if (sub_name[0] == '.') {
     OptionValueProperties *sub_properties =
         property->GetValue()->GetAsProperties();
if (property) {
         }
       }
     }
   }
 }
 
 lldb::OptionValuePropertiesSP
 OptionValueProperties::GetSubProperty(const ExecutionContext *exe_ctx,
-                                      ConstString name) {
+                                      llvm::StringRef name) {
   lldb::OptionValueSP option_value_sp(GetValueForKey(exe_ctx, name));
   if (option_value_sp) {
     OptionValueProperties *ov_properties = option_value_sp->GetAsProperties();
     if (ov_properties)
       return ov_properties->shared_from_this();
   }
   return lldb::OptionValuePropertiesSP();
 }