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

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.

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

diff --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h
index cd587ed..b437153 100644 (file)
--- a/lldb/include/lldb/Interpreter/OptionValue.h
+++ b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -125,7 +125,7 @@ public:
 
   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;
 

diff --git a/lldb/include/lldb/Interpreter/OptionValueProperties.h b/lldb/include/lldb/Interpreter/OptionValueProperties.h
index 3f40290..6655c47 100644 (file)
--- a/lldb/include/lldb/Interpreter/OptionValueProperties.h
+++ b/lldb/include/lldb/Interpreter/OptionValueProperties.h
@@ -15,7 +15,6 @@
 #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;
@@ -26,7 +25,7 @@ class OptionValueProperties
 public:
   OptionValueProperties() = default;
 
-  OptionValueProperties(ConstString name);
+  OptionValueProperties(llvm::StringRef name);
 
   ~OptionValueProperties() override = default;
 
@@ -49,7 +48,7 @@ public:
 
   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,
@@ -68,13 +67,13 @@ public:
   // 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 *
@@ -87,14 +86,13 @@ public:
   // "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,
@@ -131,11 +129,11 @@ public:
   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);
@@ -176,11 +174,9 @@ protected:
     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

diff --git a/lldb/source/Interpreter/OptionValueProperties.cpp b/lldb/source/Interpreter/OptionValueProperties.cpp
index c2b5ceb..62cf408 100644 (file)
--- a/lldb/source/Interpreter/OptionValueProperties.cpp
+++ b/lldb/source/Interpreter/OptionValueProperties.cpp
@@ -20,18 +20,17 @@
 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(
@@ -41,24 +40,25 @@ void OptionValueProperties::SetValueChangedCallback(
     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
@@ -69,13 +69,13 @@ OptionValueProperties::GetSubValue(const ExecutionContext *exe_ctx,
     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)
@@ -138,14 +138,20 @@ Status OptionValueProperties::SetSubValue(const ExecutionContext *exe_ctx,
   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(
@@ -399,18 +405,19 @@ OptionValueProperties::DeepCopy(const OptionValueSP &new_parent) const {
 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)
@@ -473,7 +480,7 @@ void OptionValueProperties::Apropos(
 
 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();