[Lldb-commits] [lldb] [lldb] Devirtualize GetValueProperties (NFC) (PR #126583)

[via lldb-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Jonas Devlieghere (JDevlieghere)

<details>
<summary>Changes</summary>

Nobody is overriding GetValueProperties, so in practice we're always using `m_collection_sp`, which means we don't need to check the pointer. The temlated helpers were already operating on `m_collection_sp` directly so this makes the rest of the class consistent.

---
Full diff: https://github.com/llvm/llvm-project/pull/126583.diff


2 Files Affected:

- (modified) lldb/include/lldb/Core/UserSettingsController.h (+1-3) 
- (modified) lldb/source/Core/UserSettingsController.cpp (+7-26) 


``````````diff
diff --git a/lldb/include/lldb/Core/UserSettingsController.h b/lldb/include/lldb/Core/UserSettingsController.h
index 32da7e05f7040f7..29e892fdba45bf3 100644
--- a/lldb/include/lldb/Core/UserSettingsController.h
+++ b/lldb/include/lldb/Core/UserSettingsController.h
@@ -38,9 +38,7 @@ class Properties {
 
   virtual ~Properties();
 
-  virtual lldb::OptionValuePropertiesSP GetValueProperties() const {
-    // This function is virtual in case subclasses want to lazily implement
-    // creating the properties.
+  lldb::OptionValuePropertiesSP GetValueProperties() const {
     return m_collection_sp;
   }
 
diff --git a/lldb/source/Core/UserSettingsController.cpp b/lldb/source/Core/UserSettingsController.cpp
index b57c1b0eef9b472..5408d64b406471f 100644
--- a/lldb/source/Core/UserSettingsController.cpp
+++ b/lldb/source/Core/UserSettingsController.cpp
@@ -40,64 +40,45 @@ Properties::~Properties() = default;
 lldb::OptionValueSP
 Properties::GetPropertyValue(const ExecutionContext *exe_ctx,
                              llvm::StringRef path, Status &error) const {
-  OptionValuePropertiesSP properties_sp(GetValueProperties());
-  if (properties_sp)
-    return properties_sp->GetSubValue(exe_ctx, path, error);
-  return lldb::OptionValueSP();
+  return m_collection_sp->GetSubValue(exe_ctx, path, error);
 }
 
 Status Properties::SetPropertyValue(const ExecutionContext *exe_ctx,
                                     VarSetOperationType op,
                                     llvm::StringRef path,
                                     llvm::StringRef value) {
-  OptionValuePropertiesSP properties_sp(GetValueProperties());
-  if (properties_sp)
-    return properties_sp->SetSubValue(exe_ctx, op, path, value);
-  return Status::FromErrorString("no properties");
+  return m_collection_sp->SetSubValue(exe_ctx, op, path, value);
 }
 
 void Properties::DumpAllPropertyValues(const ExecutionContext *exe_ctx,
                                        Stream &strm, uint32_t dump_mask,
                                        bool is_json) {
-  OptionValuePropertiesSP properties_sp(GetValueProperties());
-  if (!properties_sp)
-    return;
-
   if (is_json) {
-    llvm::json::Value json = properties_sp->ToJSON(exe_ctx);
+    llvm::json::Value json = m_collection_sp->ToJSON(exe_ctx);
     strm.Printf("%s", llvm::formatv("{0:2}", json).str().c_str());
   } else
-    properties_sp->DumpValue(exe_ctx, strm, dump_mask);
+    m_collection_sp->DumpValue(exe_ctx, strm, dump_mask);
 }
 
 void Properties::DumpAllDescriptions(CommandInterpreter &interpreter,
                                      Stream &strm) const {
   strm.PutCString("Top level variables:\n\n");
 
-  OptionValuePropertiesSP properties_sp(GetValueProperties());
-  if (properties_sp)
-    return properties_sp->DumpAllDescriptions(interpreter, strm);
+  return m_collection_sp->DumpAllDescriptions(interpreter, strm);
 }
 
 Status Properties::DumpPropertyValue(const ExecutionContext *exe_ctx,
                                      Stream &strm,
                                      llvm::StringRef property_path,
                                      uint32_t dump_mask, bool is_json) {
-  OptionValuePropertiesSP properties_sp(GetValueProperties());
-  if (properties_sp) {
-    return properties_sp->DumpPropertyValue(exe_ctx, strm, property_path,
+  return m_collection_sp->DumpPropertyValue(exe_ctx, strm, property_path,
                                             dump_mask, is_json);
-  }
-  return Status::FromErrorString("empty property list");
 }
 
 size_t
 Properties::Apropos(llvm::StringRef keyword,
                     std::vector<const Property *> &matching_properties) const {
-  OptionValuePropertiesSP properties_sp(GetValueProperties());
-  if (properties_sp) {
-    properties_sp->Apropos(keyword, matching_properties);
-  }
+  m_collection_sp->Apropos(keyword, matching_properties);
   return matching_properties.size();
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/126583