Complete ToJSON for OptionValues (#137375)

da-viper · JDevlieghere · web-flow · commit d1adb0b8cd64 · 2025-04-27T12:11:14.000+01:00
Completes the ToJSON function for `OptionValue` types and make the interface function pure virtual

---------

Co-authored-by: Jonas Devlieghere &lt;jonas@devlieghere.com&gt;
diff --git a/lldb/include/lldb/Interpreter/OptionValue.h b/lldb/include/lldb/Interpreter/OptionValue.h
@@ -93,15 +93,7 @@ class OptionValue {
   virtual void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                          uint32_t dump_mask) = 0;
 
-  // TODO: make this function pure virtual after implementing it in all
-  // child classes.
-  virtual llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) {
-    // Return nullptr which will create a llvm::json::Value() that is a NULL
-    // value. No setting should ever really have a NULL value in JSON. This
-    // indicates an error occurred and if/when we add a FromJSON() it will know
-    // to fail if someone tries to set it with a NULL JSON value.
-    return nullptr;
-  }
+  virtual llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const = 0;
 
   virtual Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValueArch.h b/lldb/include/lldb/Interpreter/OptionValueArch.h
@@ -38,6 +38,8 @@ class OptionValueArch : public Cloneable<OptionValueArch, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
+
   Status
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
diff --git a/lldb/include/lldb/Interpreter/OptionValueArray.h b/lldb/include/lldb/Interpreter/OptionValueArray.h
@@ -29,7 +29,7 @@ class OptionValueArray : public Cloneable<OptionValueArray, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValueBoolean.h b/lldb/include/lldb/Interpreter/OptionValueBoolean.h
@@ -29,7 +29,7 @@ class OptionValueBoolean : public Cloneable<OptionValueBoolean, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {
     return m_current_value;
   }
 
diff --git a/lldb/include/lldb/Interpreter/OptionValueChar.h b/lldb/include/lldb/Interpreter/OptionValueChar.h
@@ -30,7 +30,7 @@ class OptionValueChar : public Cloneable<OptionValueChar, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {
     return m_current_value;
   }
 
diff --git a/lldb/include/lldb/Interpreter/OptionValueDictionary.h b/lldb/include/lldb/Interpreter/OptionValueDictionary.h
@@ -34,7 +34,7 @@ class OptionValueDictionary
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValueEnumeration.h b/lldb/include/lldb/Interpreter/OptionValueEnumeration.h
@@ -41,7 +41,7 @@ class OptionValueEnumeration
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValueFileColonLine.h b/lldb/include/lldb/Interpreter/OptionValueFileColonLine.h
@@ -29,6 +29,8 @@ class OptionValueFileColonLine :
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
+
   Status
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
diff --git a/lldb/include/lldb/Interpreter/OptionValueFileSpec.h b/lldb/include/lldb/Interpreter/OptionValueFileSpec.h
@@ -35,7 +35,7 @@ class OptionValueFileSpec : public Cloneable<OptionValueFileSpec, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {
     return m_current_value.GetPath();
   }
 
diff --git a/lldb/include/lldb/Interpreter/OptionValueFileSpecList.h b/lldb/include/lldb/Interpreter/OptionValueFileSpecList.h
@@ -33,7 +33,7 @@ class OptionValueFileSpecList
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValueFormat.h b/lldb/include/lldb/Interpreter/OptionValueFormat.h
@@ -31,7 +31,7 @@ class OptionValueFormat
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValueFormatEntity.h b/lldb/include/lldb/Interpreter/OptionValueFormatEntity.h
@@ -28,7 +28,7 @@ class OptionValueFormatEntity
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValueLanguage.h b/lldb/include/lldb/Interpreter/OptionValueLanguage.h
@@ -33,7 +33,7 @@ class OptionValueLanguage : public Cloneable<OptionValueLanguage, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValuePathMappings.h b/lldb/include/lldb/Interpreter/OptionValuePathMappings.h
@@ -29,7 +29,7 @@ class OptionValuePathMappings
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   Status
   SetValueFromString(llvm::StringRef value,
diff --git a/lldb/include/lldb/Interpreter/OptionValueProperties.h b/lldb/include/lldb/Interpreter/OptionValueProperties.h
@@ -46,7 +46,7 @@ class OptionValueProperties
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override;
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override;
 
   llvm::StringRef GetName() const override { return m_name; }
 
diff --git a/lldb/include/lldb/Interpreter/OptionValueRegex.h b/lldb/include/lldb/Interpreter/OptionValueRegex.h
@@ -28,7 +28,7 @@ class OptionValueRegex : public Cloneable<OptionValueRegex, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {
     return m_regex.GetText();
   }
 
diff --git a/lldb/include/lldb/Interpreter/OptionValueSInt64.h b/lldb/include/lldb/Interpreter/OptionValueSInt64.h
@@ -35,7 +35,7 @@ class OptionValueSInt64 : public Cloneable<OptionValueSInt64, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {
     return m_current_value;
   }
 
diff --git a/lldb/include/lldb/Interpreter/OptionValueString.h b/lldb/include/lldb/Interpreter/OptionValueString.h
@@ -69,7 +69,7 @@ class OptionValueString : public Cloneable<OptionValueString, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {
     return m_current_value;
   }
 
diff --git a/lldb/include/lldb/Interpreter/OptionValueUInt64.h b/lldb/include/lldb/Interpreter/OptionValueUInt64.h
@@ -38,7 +38,7 @@ class OptionValueUInt64 : public Cloneable<OptionValueUInt64, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {
     return m_current_value;
   }
 
diff --git a/lldb/include/lldb/Interpreter/OptionValueUUID.h b/lldb/include/lldb/Interpreter/OptionValueUUID.h
@@ -29,7 +29,7 @@ class OptionValueUUID : public Cloneable<OptionValueUUID, OptionValue> {
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
-  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {
+  llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {
     return m_uuid.GetAsString();
   }
 
diff --git a/lldb/include/lldb/Target/PathMappingList.h b/lldb/include/lldb/Target/PathMappingList.h
@@ -49,7 +49,7 @@ class PathMappingList {
   // By default, dump all pairs.
   void Dump(Stream *s, int pair_index = -1);
 
-  llvm::json::Value ToJSON();
+  llvm::json::Value ToJSON() const;
 
   bool IsEmpty() const {
     std::lock_guard<std::mutex> lock(m_pairs_mutex);
diff --git a/lldb/source/Interpreter/OptionValueArch.cpp b/lldb/source/Interpreter/OptionValueArch.cpp
@@ -33,6 +33,14 @@ void OptionValueArch::DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
   }
 }
 
+llvm::json::Value
+OptionValueArch::ToJSON(const ExecutionContext *exe_ctx) const {
+  if (m_current_value.IsValid())
+    return llvm::json::Value(m_current_value.GetArchitectureName());
+
+  return {};
+}
+
 Status OptionValueArch::SetValueFromString(llvm::StringRef value,
                                            VarSetOperationType op) {
   Status error;
diff --git a/lldb/source/Interpreter/OptionValueArray.cpp b/lldb/source/Interpreter/OptionValueArray.cpp
@@ -75,7 +75,8 @@ void OptionValueArray::DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
   }
 }
 
-llvm::json::Value OptionValueArray::ToJSON(const ExecutionContext *exe_ctx) {
+llvm::json::Value
+OptionValueArray::ToJSON(const ExecutionContext *exe_ctx) const {
   llvm::json::Array json_array;
   const uint32_t size = m_values.size();
   for (uint32_t i = 0; i < size; ++i)
diff --git a/lldb/source/Interpreter/OptionValueDictionary.cpp b/lldb/source/Interpreter/OptionValueDictionary.cpp
@@ -88,7 +88,7 @@ void OptionValueDictionary::DumpValue(const ExecutionContext *exe_ctx,
 }
 
 llvm::json::Value
-OptionValueDictionary::ToJSON(const ExecutionContext *exe_ctx) {
+OptionValueDictionary::ToJSON(const ExecutionContext *exe_ctx) const {
   llvm::json::Object dict;
   for (const auto &value : m_values) {
     dict.try_emplace(value.first(), value.second->ToJSON(exe_ctx));
diff --git a/lldb/source/Interpreter/OptionValueEnumeration.cpp b/lldb/source/Interpreter/OptionValueEnumeration.cpp
@@ -38,7 +38,7 @@ void OptionValueEnumeration::DumpValue(const ExecutionContext *exe_ctx,
 }
 
 llvm::json::Value
-OptionValueEnumeration::ToJSON(const ExecutionContext *exe_ctx) {
+OptionValueEnumeration::ToJSON(const ExecutionContext *exe_ctx) const {
   for (const auto &enums : m_enumerations) {
     if (enums.value.value == m_current_value)
       return enums.cstring.GetStringRef();
diff --git a/lldb/source/Interpreter/OptionValueFileColonLine.cpp b/lldb/source/Interpreter/OptionValueFileColonLine.cpp
@@ -46,6 +46,19 @@ void OptionValueFileColonLine::DumpValue(const ExecutionContext *exe_ctx,
   }
 }
 
+llvm::json::Value
+OptionValueFileColonLine::ToJSON(const ExecutionContext *exe_ctx) const {
+  StreamString stream;
+  if (m_file_spec)
+    stream << '"' << m_file_spec.GetPath().c_str() << '"';
+  if (m_line_number != LLDB_INVALID_LINE_NUMBER)
+    stream.Printf(":%d", m_line_number);
+  if (m_column_number != LLDB_INVALID_COLUMN_NUMBER)
+    stream.Printf(":%d", m_column_number);
+
+  return llvm::json::Value(stream.GetString());
+}
+
 Status OptionValueFileColonLine::SetValueFromString(llvm::StringRef value,
                                                     VarSetOperationType op) {
   Status error;
diff --git a/lldb/source/Interpreter/OptionValueFileSpecList.cpp b/lldb/source/Interpreter/OptionValueFileSpecList.cpp
@@ -42,7 +42,7 @@ void OptionValueFileSpecList::DumpValue(const ExecutionContext *exe_ctx,
 }
 
 llvm::json::Value
-OptionValueFileSpecList::ToJSON(const ExecutionContext *exe_ctx) {
+OptionValueFileSpecList::ToJSON(const ExecutionContext *exe_ctx) const {
   std::lock_guard<std::recursive_mutex> lock(m_mutex);
   llvm::json::Array array;
   for (const auto &file_spec : m_current_value)
diff --git a/lldb/source/Interpreter/OptionValueFormat.cpp b/lldb/source/Interpreter/OptionValueFormat.cpp
@@ -26,7 +26,8 @@ void OptionValueFormat::DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
   }
 }
 
-llvm::json::Value OptionValueFormat::ToJSON(const ExecutionContext *exe_ctx) {
+llvm::json::Value
+OptionValueFormat::ToJSON(const ExecutionContext *exe_ctx) const {
   return FormatManager::GetFormatAsCString(m_current_value);
 }
 
diff --git a/lldb/source/Interpreter/OptionValueFormatEntity.cpp b/lldb/source/Interpreter/OptionValueFormatEntity.cpp
@@ -61,7 +61,7 @@ void OptionValueFormatEntity::DumpValue(const ExecutionContext *exe_ctx,
 }
 
 llvm::json::Value
-OptionValueFormatEntity::ToJSON(const ExecutionContext *exe_ctx) {
+OptionValueFormatEntity::ToJSON(const ExecutionContext *exe_ctx) const {
   std::string escaped;
   EscapeBackticks(m_current_format, escaped);
   return escaped;
diff --git a/lldb/source/Interpreter/OptionValueLanguage.cpp b/lldb/source/Interpreter/OptionValueLanguage.cpp
@@ -29,7 +29,8 @@ void OptionValueLanguage::DumpValue(const ExecutionContext *exe_ctx,
   }
 }
 
-llvm::json::Value OptionValueLanguage::ToJSON(const ExecutionContext *exe_ctx) {
+llvm::json::Value
+OptionValueLanguage::ToJSON(const ExecutionContext *exe_ctx) const {
   return Language::GetNameForLanguageType(m_current_value);
 }
 
diff --git a/lldb/source/Interpreter/OptionValuePathMappings.cpp b/lldb/source/Interpreter/OptionValuePathMappings.cpp
@@ -35,7 +35,7 @@ void OptionValuePathMappings::DumpValue(const ExecutionContext *exe_ctx,
 }
 
 llvm::json::Value
-OptionValuePathMappings::ToJSON(const ExecutionContext *exe_ctx) {
+OptionValuePathMappings::ToJSON(const ExecutionContext *exe_ctx) const {
   return m_path_mappings.ToJSON();
 }
 
diff --git a/lldb/source/Interpreter/OptionValueProperties.cpp b/lldb/source/Interpreter/OptionValueProperties.cpp
@@ -335,7 +335,7 @@ void OptionValueProperties::DumpValue(const ExecutionContext *exe_ctx,
 }
 
 llvm::json::Value
-OptionValueProperties::ToJSON(const ExecutionContext *exe_ctx) {
+OptionValueProperties::ToJSON(const ExecutionContext *exe_ctx) const {
   llvm::json::Object json_properties;
   const size_t num_properties = m_properties.size();
   for (size_t i = 0; i < num_properties; ++i) {
diff --git a/lldb/source/Target/PathMappingList.cpp b/lldb/source/Target/PathMappingList.cpp
@@ -180,7 +180,7 @@ void PathMappingList::Dump(Stream *s, int pair_index) {
   }
 }
 
-llvm::json::Value PathMappingList::ToJSON() {
+llvm::json::Value PathMappingList::ToJSON() const {
   llvm::json::Array entries;
   std::lock_guard<std::mutex> lock(m_pairs_mutex);
   for (const auto &pair : m_pairs) {
diff --git a/lldb/test/API/commands/settings/TestSettings.py b/lldb/test/API/commands/settings/TestSettings.py
@@ -1044,6 +1044,10 @@ def test_settings_api(self):
         # Test OptionValueEnumeration
         self.verify_setting_value_json("target.x86-disassembly-flavor", "intel")
 
+        # Test OptionValueArch
+        self.verify_setting_value_json("target.default-arch", "x86_64")
+        self.runCmd("settings clear target.default-arch")
+
     def test_global_option(self):
         # This command used to crash the settings because -g was signaled by a
         # NULL execution context (not one with an empty Target...) and in the

Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ class OptionValueBoolean : public Cloneable<OptionValueBoolean, OptionValue> {`
`29`	`29`	`void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,`
`30`	`30`	`uint32_t dump_mask) override;`
`31`	`31`
`32`		`- llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {`
	`32`	`+ llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {`
`33`	`33`	`return m_current_value;`
`34`	`34`	`}`
`35`	`35`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ class OptionValueChar : public Cloneable<OptionValueChar, OptionValue> {`
`30`	`30`	`void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,`
`31`	`31`	`uint32_t dump_mask) override;`
`32`	`32`
`33`		`- llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {`
	`33`	`+ llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {`
`34`	`34`	`return m_current_value;`
`35`	`35`	`}`
`36`	`36`
Original file line number	Diff line number	Diff line change
`@@ -35,7 +35,7 @@ class OptionValueFileSpec : public Cloneable<OptionValueFileSpec, OptionValue> {`
`35`	`35`	`void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,`
`36`	`36`	`uint32_t dump_mask) override;`
`37`	`37`
`38`		`- llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) override {`
	`38`	`+ llvm::json::Value ToJSON(const ExecutionContext *exe_ctx) const override {`
`39`	`39`	`return m_current_value.GetPath();`
`40`	`40`	`}`
`41`	`41`