Fix deep copying for OptionValue classes

[tatyana-krasnukha]

Summary

Some implementations of the DeepCopy function called the copy constructor that copied m_parent member instead of setting a new parent. Others just lived the base class's members (m_parent, m_callback, m_was_set) empty.
One more problem is that not all classes override this function, e.g. OptionValueArgs::DeepCopy produces OptionValueArray instance, and Target[Process/Thread]ValueProperty::DeepCopy produces OptionValueProperty. This makes downcasting via static_cast invalid.

This patch adds a virtual Clone function, which implements well-known idiom "virtual constructor", and overrides it in every derived class. DeepCopy calls Clone to instantiate an object of a correct type. It also sets valid m_parent.
Add a test that checks DeepCopy for correct copying/setting all data members of the base class.

[dblaikie]

Perhaps all these clone implementations could be provided via mixin instead?

struct base {
  virtual std::shared_ptr<base> Clone() = 0;
  ...
};
template<typename Derived, typename IntermediateBase = base>
struct base_clone_helper : IntermediateBase {
  static_assert(std::is_base_of<base, IntermediateBase>::value);
  std::shared_ptr<base> Clone() const override {
    return std::make_shared<Derived>(static_cast<const Derived&>(*this*));
  }
};
struct some_immediate_derivation final : base_clone_helper<some_immediate_derivation> {
};
struct some_intermediate_helper : base {
  ...
};
struct some_concrete_derivation final : base_clone_helper<some_concrete_derivation, some_intermediate_helper> {
};

Requiring that all such types be copy constructible (I guess the template helper could also provide an rvalue overload of Clone if there's a need to have move cloning, though that seems unlikely to be needed/useful)

lldb/unittests/Interpreter/TestOptionValue.cpp

174

Generally it shouldn't be necessary to write llvm::StringRef(...) around a string literal - StringRef is implicitly convertible from a string literal.

[tatyana-krasnukha]

CRTP was my first implementation, however, I discarded it as more bug-prone. Virtual Clone function at the interface is so common that, I believe, everyone knows it must be overridden by a new derived class. The necessity of inheriting from base_clone_helper is not so obvious.

lldb/unittests/Interpreter/TestOptionValue.cpp

174

OptionValueProperties and OptionValueFileSpecList enforce using StringRef explicitly by specifying the overload SetValueFromString(const char*) as deleted.

[teemperor]

lldb/unittests/Interpreter/TestOptionValue.cpp

174

So I was curious why we added these deleted overloads and from what I can see this was done as part of some C-string -> StringRef conversion in D24847. I don't think these overloads serve any purpose anymore, so I made a review for removing them: D96861

[dblaikie]

CRTP was my first implementation, however, I discarded it as more bug-prone. Virtual Clone function at the interface is so common that, I believe, everyone knows it must be overridden by a new derived class. The necessity of inheriting from base_clone_helper is not so obvious.

I would've thought it'd be pretty easy to accidentally miss either of these - I think the CRTP helper ensures consistency of implementation (harder to accidentally slice/copy the wrong type/etc. But I'm not a code owner/major contributor to lldb specifically, so probably more up to other developers who are.

[tatyana-krasnukha]

Removed explicit conversions to StringRef from the test with respect to D96861.

[clayborg]

In D96817#2569595, @dblaikie wrote:

CRTP was my first implementation, however, I discarded it as more bug-prone. Virtual Clone function at the interface is so common that, I believe, everyone knows it must be overridden by a new derived class. The necessity of inheriting from base_clone_helper is not so obvious.

I would've thought it'd be pretty easy to accidentally miss either of these - I think the CRTP helper ensures consistency of implementation (harder to accidentally slice/copy the wrong type/etc. But I'm not a code owner/major contributor to lldb specifically, so probably more up to other developers who are.

Whatever is the safest and most llvm like is probably the best approach IMHO. So maybe stick with a solution that will be familiar to LLVM developers if this current approach isn't. I am open to other suggestions if others feel strongly otherwise.

[dblaikie]

In D96817#2569852, @clayborg wrote:

In D96817#2569595, @dblaikie wrote:

CRTP was my first implementation, however, I discarded it as more bug-prone. Virtual Clone function at the interface is so common that, I believe, everyone knows it must be overridden by a new derived class. The necessity of inheriting from base_clone_helper is not so obvious.

I would've thought it'd be pretty easy to accidentally miss either of these - I think the CRTP helper ensures consistency of implementation (harder to accidentally slice/copy the wrong type/etc. But I'm not a code owner/major contributor to lldb specifically, so probably more up to other developers who are.

Whatever is the safest and most llvm like is probably the best approach IMHO. So maybe stick with a solution that will be familiar to LLVM developers if this current approach isn't. I am open to other suggestions if others feel strongly otherwise.

I think either would be legible/familiar to LLVM developers. I rather like the CRTP, but it's not a make-or-break.

I found a couple of instances of cloning APIs across LLVM (incomplete results - grep only gets one so far):

clang/include/clang/Sema/TypoCorrection.h:  virtual std::unique_ptr<CorrectionCandidateCallback> clone() = 0;
lldb/include/lldb/Core/SearchFilter.h:  virtual lldb::SearchFilterSP DoCreateCopy() = 0;

And neither uses a CRTP for the derive classes, FWIW.

[tatyana-krasnukha]

I created D96952 - a demonstrative example of the CRTP-based version of this patch. Please, take a look.

[tatyana-krasnukha]

D96952 is landed instead.

lldb/include/lldb/Interpreter/OptionValue.h

virtual void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                          uint32_t dump_mask) = 0;
 
   virtual Status
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign);
 
   virtual void Clear() = 0;
 
-  virtual lldb::OptionValueSP DeepCopy() const = 0;
+  virtual lldb::OptionValueSP
+  DeepCopy(const lldb::OptionValueSP &new_parent) const;
 
   virtual void AutoComplete(CommandInterpreter &interpreter,
                             CompletionRequest &request);
 
   // Subclasses can override these functions
   virtual lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
                                           llvm::StringRef name,
                                           bool will_modify,
public:
   bool OptionWasSet() const { return m_value_was_set; }
 
   void SetOptionWasSet() { m_value_was_set = true; }
 
   void SetParent(const lldb::OptionValueSP &parent_sp) {
     m_parent_wp = parent_sp;
   }
 
+  lldb::OptionValueSP GetParent() const { return m_parent_wp.lock(); }
+
   void SetValueChangedCallback(std::function<void()> callback) {
     assert(!m_callback);
     m_callback = std::move(callback);
   }
 
   void NotifyValueChanged() {
     if (m_callback)
       m_callback();
   }
 
 protected:
+  // Must be overriden by a derived class for correct downcasting the result of
+  // DeepCopy to it.
+  virtual lldb::OptionValueSP Clone() const = 0;
+
   lldb::OptionValueWP m_parent_wp;
   std::function<void()> m_callback;
   bool m_value_was_set; // This can be used to see if a value has been set
                         // by a call to SetValueFromCString(). It is often
                         // handy to know if an option value was set from the
                         // command line or as a setting, versus if we just have
                         // the default value that was already populated in the
                         // option value.
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUE_H

lldb/include/lldb/Interpreter/OptionValueArch.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_current_value = m_default_value;
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   void AutoComplete(CommandInterpreter &interpreter,
                     lldb_private::CompletionRequest &request) override;
 
   // Subclass specific functions
 
   ArchSpec &GetCurrentValue() { return m_current_value; }
 
   const ArchSpec &GetCurrentValue() const { return m_current_value; }
 
   const ArchSpec &GetDefaultValue() const { return m_default_value; }
 
   void SetCurrentValue(const ArchSpec &value, bool set_value_was_set) {
     m_current_value = value;
     if (set_value_was_set)
       m_value_was_set = true;
   }
 
   void SetDefaultValue(const ArchSpec &value) { m_default_value = value; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   ArchSpec m_current_value;
   ArchSpec m_default_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEARCH_H

lldb/include/lldb/Interpreter/OptionValueArgs.h

 namespace lldb_private {
 
 class OptionValueArgs : public OptionValueArray {
 public:
   OptionValueArgs()
       : OptionValueArray(
             OptionValue::ConvertTypeToMask(OptionValue::eTypeString)) {}
 
-  ~OptionValueArgs() override {}
+  ~OptionValueArgs() override = default;
 
-  size_t GetArgs(Args &args);
+  size_t GetArgs(Args &args) const;
 
   Type GetType() const override { return eTypeArgs; }
+
+protected:
+  lldb::OptionValueSP Clone() const override;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEARGS_H

lldb/include/lldb/Interpreter/OptionValueArray.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_values.clear();
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
+  lldb::OptionValueSP
+  DeepCopy(const lldb::OptionValueSP &new_parent) const override;
 
   bool IsAggregateValue() const override { return true; }
 
   lldb::OptionValueSP GetSubValue(const ExecutionContext *exe_ctx,
                                   llvm::StringRef name, bool will_modify,
                                   Status &error) const override;
 
   // Subclass specific functions
bool DeleteValue(size_t idx) {
     return false;
   }
 
   size_t GetArgs(Args &args) const;
 
   Status SetArgs(const Args &args, VarSetOperationType op);
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   typedef std::vector<lldb::OptionValueSP> collection;
 
   uint32_t m_type_mask;
   collection m_values;
   bool m_raw_value_dump;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEARRAY_H

lldb/include/lldb/Interpreter/OptionValueBoolean.h

public:
   bool GetCurrentValue() const { return m_current_value; }
 
   bool GetDefaultValue() const { return m_default_value; }
 
   void SetCurrentValue(bool value) { m_current_value = value; }
 
   void SetDefaultValue(bool value) { m_default_value = value; }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   bool m_current_value;
   bool m_default_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEBOOLEAN_H

lldb/include/lldb/Interpreter/OptionValueChar.h

public:
   char GetCurrentValue() const { return m_current_value; }
 
   char GetDefaultValue() const { return m_default_value; }
 
   void SetCurrentValue(char value) { m_current_value = value; }
 
   void SetDefaultValue(char value) { m_default_value = value; }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   char m_current_value;
   char m_default_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUECHAR_H

lldb/include/lldb/Interpreter/OptionValueDictionary.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_values.clear();
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
+  lldb::OptionValueSP
+  DeepCopy(const lldb::OptionValueSP &new_parent) const override;
 
   bool IsAggregateValue() const override { return true; }
 
   bool IsHomogenous() const {
     return ConvertTypeMaskToType(m_type_mask) != eTypeInvalid;
   }
 
   // Subclass specific functions
public:
 
   bool DeleteValueForKey(ConstString key);
 
   size_t GetArgs(Args &args) const;
 
   Status SetArgs(const Args &args, VarSetOperationType op);
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   typedef std::map<ConstString, lldb::OptionValueSP> collection;
   uint32_t m_type_mask;
   collection m_values;
   bool m_raw_value_dump;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H

lldb/include/lldb/Interpreter/OptionValueEnumeration.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_current_value = m_default_value;
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   void AutoComplete(CommandInterpreter &interpreter,
                     CompletionRequest &request) override;
 
   // Subclass specific functions
 
   enum_type operator=(enum_type value) {
     m_current_value = value;
     return m_current_value;
   }
 
   enum_type GetCurrentValue() const { return m_current_value; }
 
   enum_type GetDefaultValue() const { return m_default_value; }
 
   void SetCurrentValue(enum_type value) { m_current_value = value; }
 
   void SetDefaultValue(enum_type value) { m_default_value = value; }
 
 protected:
   void SetEnumerations(const OptionEnumValues &enumerators);
 
+  lldb::OptionValueSP Clone() const override;
+
   enum_type m_current_value;
   enum_type m_default_value;
   EnumerationMap m_enumerations;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEENUMERATION_H

lldb/include/lldb/Interpreter/OptionValueFileColonLine.h

SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_file_spec.Clear();
     m_line_number = LLDB_INVALID_LINE_NUMBER;
     m_column_number = LLDB_INVALID_COLUMN_NUMBER;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   void AutoComplete(CommandInterpreter &interpreter,
                     CompletionRequest &request) override;
 
   FileSpec &GetFileSpec() { return m_file_spec; }
   uint32_t GetLineNumber() { return m_line_number; }
   uint32_t GetColumnNumber() { return m_column_number; }
 
   void SetCompletionMask(uint32_t mask) { m_completion_mask = mask; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   FileSpec m_file_spec;
   uint32_t m_line_number;
   uint32_t m_column_number;
   uint32_t m_completion_mask;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEFILECOLONLINE_H

lldb/include/lldb/Interpreter/OptionValueFileSpec.h

public:
 
   void Clear() override {
     m_current_value = m_default_value;
     m_value_was_set = false;
     m_data_sp.reset();
     m_data_mod_time = llvm::sys::TimePoint<>();
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   void AutoComplete(CommandInterpreter &interpreter,
                     CompletionRequest &request) override;
 
   // Subclass specific functions
 
   FileSpec &GetCurrentValue() { return m_current_value; }
 
   const FileSpec &GetCurrentValue() const { return m_current_value; }
public:
 
   void SetDefaultValue(const FileSpec &value) { m_default_value = value; }
 
   const lldb::DataBufferSP &GetFileContents();
 
   void SetCompletionMask(uint32_t mask) { m_completion_mask = mask; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   FileSpec m_current_value;
   FileSpec m_default_value;
   lldb::DataBufferSP m_data_sp;
   llvm::sys::TimePoint<> m_data_mod_time;
   uint32_t m_completion_mask;
   bool m_resolve;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEFILESPEC_H

lldb/include/lldb/Interpreter/OptionValueFileSpecList.h

 
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
 class OptionValueFileSpecList : public OptionValue {
 public:
-  OptionValueFileSpecList() : OptionValue(), m_current_value() {}
+  OptionValueFileSpecList() = default;
 
-  OptionValueFileSpecList(const FileSpecList &current_value)
-      : OptionValue(), m_current_value(current_value) {}
+  OptionValueFileSpecList(const OptionValueFileSpecList &other)
+      : OptionValue(other), m_current_value(other.GetCurrentValue()) {}
 
-  ~OptionValueFileSpecList() override {}
+  ~OptionValueFileSpecList() override = default;
 
   // Virtual subclass pure virtual overrides
 
   OptionValue::Type GetType() const override { return eTypeFileSpecList; }
 
   void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
   Status
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     std::lock_guard<std::recursive_mutex> lock(m_mutex);
     m_current_value.Clear();
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   bool IsAggregateValue() const override { return true; }
 
   // Subclass specific functions
 
   FileSpecList GetCurrentValue() const {
     std::lock_guard<std::recursive_mutex> lock(m_mutex);
     return m_current_value;
   }
 
   void SetCurrentValue(const FileSpecList &value) {
     std::lock_guard<std::recursive_mutex> lock(m_mutex);
     m_current_value = value;
   }
 
   void AppendCurrentValue(const FileSpec &value) {
     std::lock_guard<std::recursive_mutex> lock(m_mutex);
     m_current_value.Append(value);
   }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   mutable std::recursive_mutex m_mutex;
   FileSpecList m_current_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEFILESPECLIST_H

lldb/include/lldb/Interpreter/OptionValueFormat.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_current_value = m_default_value;
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   // Subclass specific functions
 
   lldb::Format GetCurrentValue() const { return m_current_value; }
 
   lldb::Format GetDefaultValue() const { return m_default_value; }
 
   void SetCurrentValue(lldb::Format value) { m_current_value = value; }
 
   void SetDefaultValue(lldb::Format value) { m_default_value = value; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   lldb::Format m_current_value;
   lldb::Format m_default_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEFORMAT_H

lldb/include/lldb/Interpreter/OptionValueFormatEntity.h

void DumpValue(const ExecutionContext *exe_ctx, Stream &strm,
                  uint32_t dump_mask) override;
 
   Status
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override;
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   void AutoComplete(CommandInterpreter &interpreter,
                     CompletionRequest &request) override;
 
   // Subclass specific functions
 
   FormatEntity::Entry &GetCurrentValue() { return m_current_entry; }
 
   const FormatEntity::Entry &GetCurrentValue() const { return m_current_entry; }
 
   void SetCurrentValue(const FormatEntity::Entry &value) {
     m_current_entry = value;
   }
 
   FormatEntity::Entry &GetDefaultValue() { return m_default_entry; }
 
   const FormatEntity::Entry &GetDefaultValue() const { return m_default_entry; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   std::string m_current_format;
   std::string m_default_format;
   FormatEntity::Entry m_current_entry;
   FormatEntity::Entry m_default_entry;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEFORMATENTITY_H

lldb/include/lldb/Interpreter/OptionValueLanguage.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_current_value = m_default_value;
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   // Subclass specific functions
 
   lldb::LanguageType GetCurrentValue() const { return m_current_value; }
 
   lldb::LanguageType GetDefaultValue() const { return m_default_value; }
 
   void SetCurrentValue(lldb::LanguageType value) { m_current_value = value; }
 
   void SetDefaultValue(lldb::LanguageType value) { m_default_value = value; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   lldb::LanguageType m_current_value;
   lldb::LanguageType m_default_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUELANGUAGE_H

lldb/include/lldb/Interpreter/OptionValuePathMappings.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_path_mappings.Clear(m_notify_changes);
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   bool IsAggregateValue() const override { return true; }
 
   // Subclass specific functions
 
   PathMappingList &GetCurrentValue() { return m_path_mappings; }
 
   const PathMappingList &GetCurrentValue() const { return m_path_mappings; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   PathMappingList m_path_mappings;
   bool m_notify_changes;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEPATHMAPPINGS_H

lldb/include/lldb/Interpreter/OptionValueProperties.h

 
 #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 OptionValue,
       public std::enable_shared_from_this<OptionValueProperties> {
 public:
-  OptionValueProperties()
-      : OptionValue(), m_name(), m_properties(), m_name_to_index() {}
+  OptionValueProperties() = default;
 
   OptionValueProperties(ConstString name);
 
-  OptionValueProperties(const OptionValueProperties &global_properties);
-
   ~OptionValueProperties() override = default;
 
   Type GetType() const override { return eTypeProperties; }
 
   void Clear() override;
 
-  lldb::OptionValueSP DeepCopy() const 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;
 
protected:
   Property *ProtectedGetPropertyAtIndex(uint32_t idx) {
     return ((idx < m_properties.size()) ? &m_properties[idx] : nullptr);
   }
 
   const Property *ProtectedGetPropertyAtIndex(uint32_t idx) const {
     return ((idx < m_properties.size()) ? &m_properties[idx] : nullptr);
   }
 
+  lldb::OptionValueSP Clone() const override;
+
   typedef UniqueCStringMap<size_t> NameToIndex;
 
   ConstString m_name;
   std::vector<Property> m_properties;
   NameToIndex m_name_to_index;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEPROPERTIES_H

lldb/include/lldb/Interpreter/OptionValueRegex.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_regex = RegularExpression(m_default_regex_str);
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   // Subclass specific functions
   const RegularExpression *GetCurrentValue() const {
     return (m_regex.IsValid() ? &m_regex : nullptr);
   }
 
   void SetCurrentValue(const char *value) {
     if (value && value[0])
       m_regex = RegularExpression(llvm::StringRef(value));
     else
       m_regex = RegularExpression();
   }
 
   bool IsValid() const { return m_regex.IsValid(); }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   RegularExpression m_regex;
   std::string m_default_regex_str;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEREGEX_H

lldb/include/lldb/Interpreter/OptionValueSInt64.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_current_value = m_default_value;
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   // Subclass specific functions
 
   const int64_t &operator=(int64_t value) {
     m_current_value = value;
     return m_current_value;
   }
 
   int64_t GetCurrentValue() const { return m_current_value; }
public:
 
   int64_t GetMinimumValue() const { return m_min_value; }
 
   void SetMaximumValue(int64_t v) { m_max_value = v; }
 
   int64_t GetMaximumValue() const { return m_max_value; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   int64_t m_current_value;
   int64_t m_default_value;
   int64_t m_min_value;
   int64_t m_max_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUESINT64_H

lldb/include/lldb/Interpreter/OptionValueString.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_current_value = m_default_value;
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   // Subclass specific functions
 
   Flags &GetOptions() { return m_options; }
 
   const Flags &GetOptions() const { return m_options; }
 
   const char *operator=(const char *value) {
     SetCurrentValue(llvm::StringRef::withNullAsEmpty(value));
else
       m_default_value.clear();
   }
 
   bool IsCurrentValueEmpty() const { return m_current_value.empty(); }
 
   bool IsDefaultValueEmpty() const { return m_default_value.empty(); }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   std::string m_current_value;
   std::string m_default_value;
   Flags m_options;
   ValidatorCallback m_validator;
   void *m_validator_baton;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUESTRING_H

lldb/include/lldb/Interpreter/OptionValueUInt64.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_current_value = m_default_value;
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   // Subclass specific functions
 
   const uint64_t &operator=(uint64_t value) {
     m_current_value = value;
     return m_current_value;
   }
 
   operator uint64_t() const { return m_current_value; }
 
   uint64_t GetCurrentValue() const { return m_current_value; }
 
   uint64_t GetDefaultValue() const { return m_default_value; }
 
   void SetCurrentValue(uint64_t value) { m_current_value = value; }
 
   void SetDefaultValue(uint64_t value) { m_default_value = value; }
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   uint64_t m_current_value;
   uint64_t m_default_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEUINT64_H

lldb/include/lldb/Interpreter/OptionValueUUID.h

public:
   SetValueFromString(llvm::StringRef value,
                      VarSetOperationType op = eVarSetOperationAssign) override;
 
   void Clear() override {
     m_uuid.Clear();
     m_value_was_set = false;
   }
 
-  lldb::OptionValueSP DeepCopy() const override;
-
   // Subclass specific functions
 
   UUID &GetCurrentValue() { return m_uuid; }
 
   const UUID &GetCurrentValue() const { return m_uuid; }
 
   void SetCurrentValue(const UUID &value) { m_uuid = value; }
 
   void AutoComplete(CommandInterpreter &interpreter,
                     CompletionRequest &request) override;
 
 protected:
+  lldb::OptionValueSP Clone() const override;
+
   UUID m_uuid;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEUUID_H

lldb/include/lldb/Target/Target.h

public:
   bool GetDisplayRuntimeSupportValues() const;
 
   void SetDisplayRuntimeSupportValues(bool b);
 
   bool GetDisplayRecognizedArguments() const;
 
   void SetDisplayRecognizedArguments(bool b);
 
-  const ProcessLaunchInfo &GetProcessLaunchInfo();
+  const ProcessLaunchInfo &GetProcessLaunchInfo() const;
 
   void SetProcessLaunchInfo(const ProcessLaunchInfo &launch_info);
 
   bool GetInjectLocalVariables(ExecutionContext *exe_ctx) const;
 
   void SetInjectLocalVariables(ExecutionContext *exe_ctx, bool b);
 
   void SetRequireHardwareBreakpoints(bool b);

lldb/source/Interpreter/OptionValue.cpp

if (dumped_something)
       strm.PutChar('.');
     else
       dumped_something = true;
     strm << name;
   }
   return dumped_something;
 }
 
+OptionValueSP OptionValue::DeepCopy(const OptionValueSP &new_parent) const {
+  auto &&clone = Clone();
+  clone->SetParent(new_parent);
+  return clone;
+}
+
 void OptionValue::AutoComplete(CommandInterpreter &interpreter,
                                CompletionRequest &request) {}
 
 Status OptionValue::SetValueFromString(llvm::StringRef value,
                                        VarSetOperationType op) {
   Status error;
   switch (op) {
   case eVarSetOperationReplace:

lldb/source/Interpreter/OptionValueArch.cpp

Status OptionValueArch::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueArch::DeepCopy() const {
-  return OptionValueSP(new OptionValueArch(*this));
+OptionValueSP OptionValueArch::Clone() const {
+  return std::make_shared<OptionValueArch>(*this);
 }
 
 void OptionValueArch::AutoComplete(CommandInterpreter &interpreter,
                                    CompletionRequest &request) {
   CommandCompletions::InvokeCommonCompletionCallbacks(
       interpreter, CommandCompletions::eArchitectureCompletion, request,
       nullptr);
 }

lldb/source/Interpreter/OptionValueArgs.cpp

 //===-- OptionValueArgs.cpp -----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "lldb/Interpreter/OptionValueArgs.h"
 
 #include "lldb/Utility/Args.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
-size_t OptionValueArgs::GetArgs(Args &args) {
+size_t OptionValueArgs::GetArgs(Args &args) const {
   args.Clear();
-  for (auto value : m_values) {
+  for (const auto &value : m_values) {
     llvm::StringRef string_value = value->GetStringValue();
     if (!string_value.empty())
       args.AppendArgument(string_value);
   }
 
   return args.GetArgumentCount();
 }
+
+OptionValueSP OptionValueArgs::Clone() const {
+  return std::make_shared<OptionValueArgs>(*this);
+}

lldb/source/Interpreter/OptionValueArray.cpp

for (size_t i = 0; i < argc; ++i) {
             "array of complex types must subclass OptionValueArray");
       }
     }
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueArray::DeepCopy() const {
-  OptionValueArray *copied_array =
-      new OptionValueArray(m_type_mask, m_raw_value_dump);
-  lldb::OptionValueSP copied_value_sp(copied_array);
-  *static_cast<OptionValue *>(copied_array) = *this;
-  copied_array->m_callback = m_callback;
-  const uint32_t size = m_values.size();
-  for (uint32_t i = 0; i < size; ++i) {
-    copied_array->AppendValue(m_values[i]->DeepCopy());
-  }
-  return copied_value_sp;
+OptionValueSP OptionValueArray::Clone() const {
+  return std::make_shared<OptionValueArray>(*this);
+
+OptionValueSP
+OptionValueArray::DeepCopy(const OptionValueSP &new_parent) const {
+  auto copy_sp = OptionValue::DeepCopy(new_parent);
+  // copy_sp->GetAsArray cannot be used here as it doesn't work for derived
+  // types that override GetType returning a different value.
+  auto *array_value_ptr = static_cast<OptionValueArray *>(copy_sp.get());
+  lldbassert(array_value_ptr);
+
+  for (auto &value : array_value_ptr->m_values)
+    value = value->DeepCopy(copy_sp);
+
+  return copy_sp;
 }

lldb/source/Interpreter/OptionValueBoolean.cpp

Status OptionValueBoolean::SetValueFromString(llvm::StringRef value_str,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value_str, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueBoolean::DeepCopy() const {
-  return OptionValueSP(new OptionValueBoolean(*this));
+OptionValueSP OptionValueBoolean::Clone() const {
+  return std::make_shared<OptionValueBoolean>(*this);
 }
 
 void OptionValueBoolean::AutoComplete(CommandInterpreter &interpreter,
                                       CompletionRequest &request) {
   llvm::StringRef autocomplete_entries[] = {"true", "false", "on", "off",
                                             "yes",  "no",    "1",  "0"};
 
   auto entries = llvm::makeArrayRef(autocomplete_entries);
 
   // only suggest "true" or "false" by default
   if (request.GetCursorArgumentPrefix().empty())
     entries = entries.take_front(2);
 
   for (auto entry : entries)
     request.TryCompleteCurrentArg(entry);
 }

lldb/source/Interpreter/OptionValueChar.cpp

Status OptionValueChar::SetValueFromString(llvm::StringRef value,
 
   default:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueChar::DeepCopy() const {
-  return OptionValueSP(new OptionValueChar(*this));
+OptionValueSP OptionValueChar::Clone() const {
+  return std::make_shared<OptionValueChar>(*this);
 }

lldb/source/Interpreter/OptionValueDictionary.cpp

bool OptionValueDictionary::DeleteValueForKey(ConstString key) {
   collection::iterator pos = m_values.find(key);
   if (pos != m_values.end()) {
     m_values.erase(pos);
     return true;
   }
   return false;
 }
 
-lldb::OptionValueSP OptionValueDictionary::DeepCopy() const {
-  OptionValueDictionary *copied_dict =
-      new OptionValueDictionary(m_type_mask, m_raw_value_dump);
-  lldb::OptionValueSP copied_value_sp(copied_dict);
-  collection::const_iterator pos, end = m_values.end();
-  for (pos = m_values.begin(); pos != end; ++pos) {
-    StreamString strm;
-    strm.Printf("%s=", pos->first.GetCString());
-    copied_dict->SetValueForKey(pos->first, pos->second->DeepCopy(), true);
-  }
-  return copied_value_sp;
+OptionValueSP OptionValueDictionary::Clone() const {
+  return std::make_shared<OptionValueDictionary>(*this);
+
+OptionValueSP
+OptionValueDictionary::DeepCopy(const OptionValueSP &new_parent) const {
+  auto copy_sp = OptionValue::DeepCopy(new_parent);
+  // copy_sp->GetAsDictionary cannot be used here as it doesn't work for derived
+  // types that override GetType returning a different value.
+  auto *dict_value_ptr = static_cast<OptionValueDictionary *>(copy_sp.get());
+  lldbassert(dict_value_ptr);
+
+  for (auto &value : dict_value_ptr->m_values)
+    value.second = value.second->DeepCopy(copy_sp);
+
+  return copy_sp;
 }

lldb/source/Interpreter/OptionValueEnumeration.cpp

for (const auto &enumerator : enumerators) {
     ConstString const_enumerator_name(enumerator.string_value);
     EnumeratorInfo enumerator_info = {enumerator.value, enumerator.usage};
     m_enumerations.Append(const_enumerator_name, enumerator_info);
   }
 
   m_enumerations.Sort();
 }
 
-lldb::OptionValueSP OptionValueEnumeration::DeepCopy() const {
-  return OptionValueSP(new OptionValueEnumeration(*this));
+OptionValueSP OptionValueEnumeration::Clone() const {
+  return std::make_shared<OptionValueEnumeration>(*this);
 }
 
 void OptionValueEnumeration::AutoComplete(CommandInterpreter &interpreter,
                                           CompletionRequest &request) {
   const uint32_t num_enumerators = m_enumerations.GetSize();
   if (!request.GetCursorArgumentPrefix().empty()) {
     for (size_t i = 0; i < num_enumerators; ++i) {
       llvm::StringRef name = m_enumerations.GetCStringAtIndex(i).GetStringRef();
       request.TryCompleteCurrentArg(name);
     }
     return;
   }
     for (size_t i = 0; i < num_enumerators; ++i)
       request.AddCompletion(m_enumerations.GetCStringAtIndex(i).GetStringRef());
 }

lldb/source/Interpreter/OptionValueFileColonLine.cpp

Status OptionValueFileColonLine::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueFileColonLine::DeepCopy() const {
-  return OptionValueSP(new OptionValueFileColonLine(*this));
+OptionValueSP OptionValueFileColonLine::Clone() const {
+  return std::make_shared<OptionValueFileColonLine>(*this);
 }
 
 void OptionValueFileColonLine::AutoComplete(CommandInterpreter &interpreter,
                                             CompletionRequest &request) {
   CommandCompletions::InvokeCommonCompletionCallbacks(
       interpreter, m_completion_mask, request, nullptr);
 }

lldb/source/Interpreter/OptionValueFileSpec.cpp

Status OptionValueFileSpec::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueFileSpec::DeepCopy() const {
-  return OptionValueSP(new OptionValueFileSpec(*this));
+OptionValueSP OptionValueFileSpec::Clone() const {
+  return std::make_shared<OptionValueFileSpec>(*this);
 }
 
 void OptionValueFileSpec::AutoComplete(CommandInterpreter &interpreter,
                                        CompletionRequest &request) {
   CommandCompletions::InvokeCommonCompletionCallbacks(
       interpreter, m_completion_mask, request, nullptr);
 }
 

lldb/source/Interpreter/OptionValueFileSpecList.cpp

Status OptionValueFileSpecList::SetValueFromString(llvm::StringRef value,
 
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueFileSpecList::DeepCopy() const {
+OptionValueSP OptionValueFileSpecList::Clone() const {
   std::lock_guard<std::recursive_mutex> lock(m_mutex);
-  return OptionValueSP(new OptionValueFileSpecList(m_current_value));
+  return std::make_shared<OptionValueFileSpecList>(*this);
 }

lldb/source/Interpreter/OptionValueFormat.cpp

Status OptionValueFormat::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueFormat::DeepCopy() const {
-  return OptionValueSP(new OptionValueFormat(*this));
+OptionValueSP OptionValueFormat::Clone() const {
+  return std::make_shared<OptionValueFormat>(*this);
 }

lldb/source/Interpreter/OptionValueFormatEntity.cpp

Status OptionValueFormatEntity::SetValueFromString(llvm::StringRef value_str,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value_str, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueFormatEntity::DeepCopy() const {
-  return OptionValueSP(new OptionValueFormatEntity(*this));
+OptionValueSP OptionValueFormatEntity::Clone() const {
+  return std::make_shared<OptionValueFormatEntity>(*this);
 }
 
 void OptionValueFormatEntity::AutoComplete(CommandInterpreter &interpreter,
                                            CompletionRequest &request) {
   FormatEntity::AutoComplete(request);
 }

lldb/source/Interpreter/OptionValueLanguage.cpp

Status OptionValueLanguage::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueLanguage::DeepCopy() const {
-  return OptionValueSP(new OptionValueLanguage(*this));
+OptionValueSP OptionValueLanguage::Clone() const {
+  return std::make_shared<OptionValueLanguage>(*this);
 }

lldb/source/Interpreter/OptionValuePathMappings.cpp

Status OptionValuePathMappings::SetValueFromString(llvm::StringRef value,
 
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValuePathMappings::DeepCopy() const {
-  return OptionValueSP(new OptionValuePathMappings(*this));
+OptionValueSP OptionValuePathMappings::Clone() const {
+  return std::make_shared<OptionValuePathMappings>(*this);
 }

lldb/source/Interpreter/OptionValueProperties.cpp

 #include "lldb/Utility/StringList.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 OptionValueProperties::OptionValueProperties(ConstString name)
     : OptionValue(), m_name(name), m_properties(), m_name_to_index() {}
 
-OptionValueProperties::OptionValueProperties(
-    const OptionValueProperties &global_properties)
-    : OptionValue(global_properties),
-      std::enable_shared_from_this<OptionValueProperties>(),
-      m_name(global_properties.m_name),
-      m_properties(global_properties.m_properties),
-      m_name_to_index(global_properties.m_name_to_index) {
-  // We now have an exact copy of "global_properties". We need to now find all
-  // non-global settings and copy the property values so that all non-global
-  // settings get new OptionValue instances created for them.
-  const size_t num_properties = m_properties.size();
-  for (size_t i = 0; i < num_properties; ++i) {
-    // Duplicate any values that are not global when constructing properties
-    // from a global copy
-    if (!m_properties[i].IsGlobal()) {
-      lldb::OptionValueSP new_value_sp(m_properties[i].GetValue()->DeepCopy());
-      m_properties[i].SetOptionValue(new_value_sp);
-    }
-  }
-}
-
 size_t OptionValueProperties::GetNumProperties() const {
   return m_properties.size();
 }
 
 void OptionValueProperties::Initialize(const PropertyDefinitions &defs) {
   for (const auto &definition : defs) {
     Property property(definition);
     assert(property.IsValid());
OptionValueProperties::GetPropertyAtIndexAsOptionValueLanguage(
   if (property)
     return property->GetValue()->GetAsLanguage();
   return nullptr;
 }
 
 bool OptionValueProperties::GetPropertyAtIndexAsArgs(
     const ExecutionContext *exe_ctx, uint32_t idx, Args &args) const {
   const Property *property = GetPropertyAtIndex(exe_ctx, false, idx);
-  if (property) {
-    OptionValue *value = property->GetValue().get();
-    if (value) {
-      const OptionValueArray *array = value->GetAsArray();
-      if (array)
-        return array->GetArgs(args);
-      else {
-        const OptionValueDictionary *dict = value->GetAsDictionary();
-        if (dict)
-          return dict->GetArgs(args);
-      }
-    }
-  }
+  if (!property)
+    return false;
+
+  if (!value)
+    return false;
+
+  const OptionValueArgs *arguments = value->GetAsArgs();
+  if (arguments)
+    return arguments->GetArgs(args);
+
+
+
   return false;
 }
 
 bool OptionValueProperties::SetPropertyAtIndexFromArgs(
     const ExecutionContext *exe_ctx, uint32_t idx, const Args &args) {
   const Property *property = GetPropertyAtIndex(exe_ctx, true, idx);
-  if (property) {
-    OptionValue *value = property->GetValue().get();
-    if (value) {
-      OptionValueArray *array = value->GetAsArray();
-      if (array)
-        return array->SetArgs(args, eVarSetOperationAssign).Success();
-      else {
-        OptionValueDictionary *dict = value->GetAsDictionary();
-        if (dict)
-          return dict->SetArgs(args, eVarSetOperationAssign).Success();
-      }
-    }
-  }
+  if (!property)
+    return false;
+
+  if (!value)
+    return false;
+
+  OptionValueArgs *arguments = value->GetAsArgs();
+  if (arguments)
+    return arguments->SetArgs(args, eVarSetOperationAssign).Success();
+
+
+
   return false;
 }
 
 bool OptionValueProperties::GetPropertyAtIndexAsBoolean(
     const ExecutionContext *exe_ctx, uint32_t idx, bool fail_value) const {
   const Property *property = GetPropertyAtIndex(exe_ctx, false, idx);
   if (property) {
     OptionValue *value = property->GetValue().get();
if (!value_sp->ValueIsTransparent()) {
       if (dump_mask & ~eDumpOptionName)
         strm.PutChar(' ');
     }
     value_sp->DumpValue(exe_ctx, strm, dump_mask);
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueProperties::DeepCopy() const {
-  llvm_unreachable("this shouldn't happen");
+OptionValuePropertiesSP
+OptionValueProperties::CreateLocalCopy(const Properties &global_properties) {
+  auto global_props_sp = global_properties.GetValueProperties();
+  lldbassert(global_props_sp);
+
+  auto copy_sp = global_props_sp->DeepCopy(global_props_sp->GetParent());
+  return std::static_pointer_cast<OptionValueProperties>(copy_sp);
+}
+
+OptionValueSP OptionValueProperties::Clone() const {
+  return std::make_shared<OptionValueProperties>(*this);
+}
+
+OptionValueSP
+OptionValueProperties::DeepCopy(const OptionValueSP &new_parent) const {
+  auto copy_sp = OptionValue::DeepCopy(new_parent);
+  // copy_sp->GetAsProperties cannot be used here as it doesn't work for derived
+  // types that override GetType returning a different value.
+  auto *props_value_ptr = static_cast<OptionValueProperties *>(copy_sp.get());
+  lldbassert(props_value_ptr);
+
+  for (auto &property : props_value_ptr->m_properties) {
+    // Duplicate any values that are not global when constructing properties
+    // from a global copy.
+    if (!property.IsGlobal()) {
+      auto value_sp = property.GetValue()->DeepCopy(copy_sp);
+      property.SetOptionValue(value_sp);
+    }
+  }
+  return copy_sp;
 }
 
 const Property *OptionValueProperties::GetPropertyAtPath(
     const ExecutionContext *exe_ctx, bool will_modify, llvm::StringRef name) const {
   const Property *property = nullptr;
   if (name.empty())
     return nullptr;
   llvm::StringRef sub_name;

lldb/source/Interpreter/OptionValueRegex.cpp

case eVarSetOperationAssign:
     } else {
       error.SetErrorString("regex error");
     }
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueRegex::DeepCopy() const {
-  return OptionValueSP(new OptionValueRegex(m_regex.GetText().str().c_str()));
+OptionValueSP OptionValueRegex::Clone() const {
+  return std::make_shared<OptionValueRegex>(m_regex.GetText().str().c_str());
 }

lldb/source/Interpreter/OptionValueSInt64.cpp

Status OptionValueSInt64::SetValueFromString(llvm::StringRef value_ref,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value_ref, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueSInt64::DeepCopy() const {
-  return OptionValueSP(new OptionValueSInt64(*this));
+OptionValueSP OptionValueSInt64::Clone() const {
+  return std::make_shared<OptionValueSInt64>(*this);
 }

lldb/source/Interpreter/OptionValueString.cpp

if (m_options.Test(eOptionEncodeCharacterEscapeSequences)) {
       SetCurrentValue(value_str);
     }
     NotifyValueChanged();
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueString::DeepCopy() const {
-  return OptionValueSP(new OptionValueString(*this));
+OptionValueSP OptionValueString::Clone() const {
+  return std::make_shared<OptionValueString>(*this);
 }
 
 Status OptionValueString::SetCurrentValue(llvm::StringRef value) {
   if (m_validator) {
     Status error(m_validator(value.str().c_str(), m_validator_baton));
     if (error.Fail())
       return error;
   }

lldb/source/Interpreter/OptionValueUInt64.cpp

Status OptionValueUInt64::SetValueFromString(llvm::StringRef value_ref,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value_ref, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueUInt64::DeepCopy() const {
-  return OptionValueSP(new OptionValueUInt64(*this));
+OptionValueSP OptionValueUInt64::Clone() const {
+  return std::make_shared<OptionValueUInt64>(*this);
 }

lldb/source/Interpreter/OptionValueUUID.cpp

Status OptionValueUUID::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
 
-lldb::OptionValueSP OptionValueUUID::DeepCopy() const {
-  return OptionValueSP(new OptionValueUUID(*this));
+OptionValueSP OptionValueUUID::Clone() const {
+  return std::make_shared<OptionValueUUID>(*this);
 }
 
 void OptionValueUUID::AutoComplete(CommandInterpreter &interpreter,
                                    CompletionRequest &request) {
   ExecutionContext exe_ctx(interpreter.GetExecutionContext());
   Target *target = exe_ctx.GetTargetPtr();
   if (!target)
     return;

lldb/source/Target/Process.cpp

 #define DISABLE_MEM_CACHE_DEFAULT true
 #endif
 
 class ProcessOptionValueProperties : public OptionValueProperties {
 public:
   ProcessOptionValueProperties(ConstString name)
       : OptionValueProperties(name) {}
 
-  // This constructor is used when creating ProcessOptionValueProperties when
-  // it is part of a new lldb_private::Process instance. It will copy all
-  // current global property values as needed
-  ProcessOptionValueProperties(ProcessProperties *global_properties)
-      : OptionValueProperties(*global_properties->GetValueProperties()) {}
-
   const Property *GetPropertyAtIndex(const ExecutionContext *exe_ctx,
                                      bool will_modify,
                                      uint32_t idx) const override {
     // When getting the value for a key from the process options, we will
     // always try and grab the setting from the current process if there is
     // one. Else we just use the one from this instance.
     if (exe_ctx) {
       Process *process = exe_ctx->GetProcessPtr();
       if (process) {
         ProcessOptionValueProperties *instance_properties =
             static_cast<ProcessOptionValueProperties *>(
                 process->GetValueProperties().get());
         if (this != instance_properties)
           return instance_properties->ProtectedGetPropertyAtIndex(idx);
       }
     }
     return ProtectedGetPropertyAtIndex(idx);
   }
+
+private:
+  lldb::OptionValueSP Clone() const override {
+    return std::make_shared<ProcessOptionValueProperties>(*this);
+  }
 };
 
 #define LLDB_PROPERTIES_process
 #include "TargetProperties.inc"
 
 enum {
 #define LLDB_PROPERTIES_process
 #include "TargetPropertiesEnum.inc"
   ePropertyExperimental,
 };
 
 #define LLDB_PROPERTIES_process_experimental
 #include "TargetProperties.inc"
 
 enum {
 #define LLDB_PROPERTIES_process_experimental
 #include "TargetPropertiesEnum.inc"
 };
 
 class ProcessExperimentalOptionValueProperties : public OptionValueProperties {
 public:
   ProcessExperimentalOptionValueProperties()
       : OptionValueProperties(
             ConstString(Properties::GetExperimentalSettingsName())) {}
+
+private:
+  lldb::OptionValueSP Clone() const override {
+    return std::make_shared<ProcessExperimentalOptionValueProperties>(*this);
+  }
 };
 
 ProcessExperimentalProperties::ProcessExperimentalProperties()
     : Properties(OptionValuePropertiesSP(
           new ProcessExperimentalOptionValueProperties())) {
   m_collection_sp->Initialize(g_process_experimental_properties);
 }
 
 ProcessProperties::ProcessProperties(lldb_private::Process *process)
     : Properties(),
       m_process(process) // Can be nullptr for global ProcessProperties
 {
   if (process == nullptr) {
     // Global process properties, set them up one time
     m_collection_sp =
         std::make_shared<ProcessOptionValueProperties>(ConstString("process"));
     m_collection_sp->Initialize(g_process_properties);
     m_collection_sp->AppendProperty(
         ConstString("thread"), ConstString("Settings specific to threads."),
         true, Thread::GetGlobalProperties()->GetValueProperties());
   } else {
-    m_collection_sp = std::make_shared<ProcessOptionValueProperties>(
-        Process::GetGlobalProperties().get());
+    m_collection_sp =
+        OptionValueProperties::CreateLocalCopy(*Process::GetGlobalProperties());
     m_collection_sp->SetValueChangedCallback(
         ePropertyPythonOSPluginPath,
         [this] { m_process->LoadOperatingSystemPlugin(true); });
   }
 
   m_experimental_properties_up =
       std::make_unique<ProcessExperimentalProperties>();
   m_collection_sp->AppendProperty(

lldb/source/Target/Target.cpp

 #include "TargetPropertiesEnum.inc"
   ePropertyExperimental,
 };
 
 class TargetOptionValueProperties : public OptionValueProperties {
 public:
   TargetOptionValueProperties(ConstString name) : OptionValueProperties(name) {}
 
-  // This constructor is used when creating TargetOptionValueProperties when it
-  // is part of a new lldb_private::Target instance. It will copy all current
-  // global property values as needed
-  TargetOptionValueProperties(const TargetPropertiesSP &target_properties_sp)
-      : OptionValueProperties(*target_properties_sp->GetValueProperties()) {}
-
   const Property *GetPropertyAtIndex(const ExecutionContext *exe_ctx,
                                      bool will_modify,
                                      uint32_t idx) const override {
     // When getting the value for a key from the target options, we will always
     // try and grab the setting from the current target if there is one. Else
     // we just use the one from this instance.
     if (exe_ctx) {
       Target *target = exe_ctx->GetTargetPtr();
       if (target) {
         TargetOptionValueProperties *target_properties =
             static_cast<TargetOptionValueProperties *>(
                 target->GetValueProperties().get());
         if (this != target_properties)
           return target_properties->ProtectedGetPropertyAtIndex(idx);
       }
     }
     return ProtectedGetPropertyAtIndex(idx);
   }
+
+private:
+  lldb::OptionValueSP Clone() const override {
+    return std::make_shared<TargetOptionValueProperties>(*this);
+  }
 };
 
 // TargetProperties
 #define LLDB_PROPERTIES_target_experimental
 #include "TargetProperties.inc"
 
 enum {
 #define LLDB_PROPERTIES_target_experimental
 #include "TargetPropertiesEnum.inc"
 };
 
 class TargetExperimentalOptionValueProperties : public OptionValueProperties {
 public:
   TargetExperimentalOptionValueProperties()
       : OptionValueProperties(
             ConstString(Properties::GetExperimentalSettingsName())) {}
+
+private:
+  lldb::OptionValueSP Clone() const override {
+    return std::make_shared<TargetExperimentalOptionValueProperties>(*this);
+  }
 };
 
 TargetExperimentalProperties::TargetExperimentalProperties()
     : Properties(OptionValuePropertiesSP(
           new TargetExperimentalOptionValueProperties())) {
   m_collection_sp->Initialize(g_target_experimental_properties);
 }
 
 // TargetProperties
 TargetProperties::TargetProperties(Target *target)
     : Properties(), m_launch_info(), m_target(target) {
   if (target) {
-    m_collection_sp = std::make_shared<TargetOptionValueProperties>(
-        Target::GetGlobalProperties());
+    m_collection_sp =
+        OptionValueProperties::CreateLocalCopy(*Target::GetGlobalProperties());
 
     // Set callbacks to update launch_info whenever "settins set" updated any
     // of these properties
     m_collection_sp->SetValueChangedCallback(
         ePropertyArg0, [this] { Arg0ValueChangedCallback(); });
     m_collection_sp->SetValueChangedCallback(
         ePropertyRunArgs, [this] { RunArgsValueChangedCallback(); });
     m_collection_sp->SetValueChangedCallback(
bool TargetProperties::GetNonStopModeEnabled() const {
   return m_collection_sp->GetPropertyAtIndexAsBoolean(nullptr, idx, false);
 }
 
 void TargetProperties::SetNonStopModeEnabled(bool b) {
   const uint32_t idx = ePropertyNonStopModeEnabled;
   m_collection_sp->SetPropertyAtIndexAsBoolean(nullptr, idx, b);
 }
 
-const ProcessLaunchInfo &TargetProperties::GetProcessLaunchInfo() {
-  m_launch_info.SetArg0(GetArg0()); // FIXME: Arg0 callback doesn't work
+const ProcessLaunchInfo &TargetProperties::GetProcessLaunchInfo() const {
   return m_launch_info;
 }
 
 void TargetProperties::SetProcessLaunchInfo(
     const ProcessLaunchInfo &launch_info) {
   m_launch_info = launch_info;
   SetArg0(launch_info.GetArg0());
   SetRunArguments(launch_info.GetArguments());

lldb/source/Target/Thread.cpp

 #include "TargetPropertiesEnum.inc"
 };
 
 class ThreadOptionValueProperties : public OptionValueProperties {
 public:
   ThreadOptionValueProperties(ConstString name)
       : OptionValueProperties(name) {}
 
-  // This constructor is used when creating ThreadOptionValueProperties when it
-  // is part of a new lldb_private::Thread instance. It will copy all current
-  // global property values as needed
-  ThreadOptionValueProperties(ThreadProperties *global_properties)
-      : OptionValueProperties(*global_properties->GetValueProperties()) {}
-
   const Property *GetPropertyAtIndex(const ExecutionContext *exe_ctx,
                                      bool will_modify,
                                      uint32_t idx) const override {
     // When getting the value for a key from the thread options, we will always
     // try and grab the setting from the current thread if there is one. Else
     // we just use the one from this instance.
     if (exe_ctx) {
       Thread *thread = exe_ctx->GetThreadPtr();
       if (thread) {
         ThreadOptionValueProperties *instance_properties =
             static_cast<ThreadOptionValueProperties *>(
                 thread->GetValueProperties().get());
         if (this != instance_properties)
           return instance_properties->ProtectedGetPropertyAtIndex(idx);
       }
     }
     return ProtectedGetPropertyAtIndex(idx);
   }
+
+private:
+  lldb::OptionValueSP Clone() const override {
+    return std::make_shared<ThreadOptionValueProperties>(*this);
+  }
 };
 
 ThreadProperties::ThreadProperties(bool is_global) : Properties() {
   if (is_global) {
     m_collection_sp =
         std::make_shared<ThreadOptionValueProperties>(ConstString("thread"));
     m_collection_sp->Initialize(g_thread_properties);
   } else
-    m_collection_sp = std::make_shared<ThreadOptionValueProperties>(
-        Thread::GetGlobalProperties().get());
+    m_collection_sp =
+        OptionValueProperties::CreateLocalCopy(*Thread::GetGlobalProperties());
 }
 
 ThreadProperties::~ThreadProperties() = default;
 
 const RegularExpression *ThreadProperties::GetSymbolsToAvoidRegexp() {
   const uint32_t idx = ePropertyStepAvoidRegex;
   return m_collection_sp->GetPropertyAtIndexAsOptionValueRegex(nullptr, idx);
 }

lldb/test/API/commands/settings/TestSettings.py

def do_test_run_args_and_env_vars(self, use_launchsimple):
         found_env_var = False
         for i in range(0, launch_info.GetNumEnvironmentEntries()):
             if launch_info.GetEnvironmentEntryAtIndex(i) == "MY_ENV_VAR=YES":
                 found_env_var = True
                 break
         self.assertTrue(found_env_var,
                         "MY_ENV_VAR was not set in LunchInfo object")
 
+        self.assertEqual(launch_info.GetNumArguments(), 3)
+        self.assertEqual(launch_info.GetArgumentAtIndex(0), "A")
+        self.assertEqual(launch_info.GetArgumentAtIndex(1), "B")
+        self.assertEqual(launch_info.GetArgumentAtIndex(2), "C")
+        
         self.expect(
             'target show-launch-environment',
             substrs=["MY_ENV_VAR=YES"])
 
         wd = self.get_process_working_directory()
         if use_launchsimple:
             process = target.LaunchSimple(None, None, wd)
             self.assertTrue(process)

lldb/unittests/Interpreter/CMakeLists.txt

 add_lldb_unittest(InterpreterTests
   TestCompletion.cpp
   TestOptionArgParser.cpp
+  TestOptionValue.cpp
   TestOptionValueFileColonLine.cpp
 
   LINK_LIBS
     lldbInterpreter
     lldbUtilityHelpers
   )

lldb/unittests/Interpreter/TestOptionValue.cpp

+//===-- TestOptionValue.cpp --------        -------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "lldb/Interpreter/OptionValues.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+using namespace lldb_private;
+
+class Callback {
+public:
+  virtual void Invoke() const {}
+  void operator()() const { Invoke(); }
+};
+
+class MockCallback : public Callback {
+public:
+  MOCK_CONST_METHOD0(Invoke, void());
+};
+
+// Test a single-value class.
+TEST(OptionValueString, DeepCopy) {
+  OptionValueString str;
+  str.SetValueFromString("ab");
+
+  MockCallback callback;
+  str.SetValueChangedCallback([&callback] { callback(); });
+  EXPECT_CALL(callback, Invoke());
+
+  auto copy_sp = str.DeepCopy(nullptr);
+
+  // Test that the base class data members are copied/set correctly.
+  ASSERT_TRUE(copy_sp);
+  ASSERT_EQ(copy_sp->GetParent().get(), nullptr);
+  ASSERT_TRUE(copy_sp->OptionWasSet());
+  ASSERT_EQ(copy_sp->GetStringValue(), "ab");
+
+  // Trigger the callback.
+  copy_sp->SetValueFromString("c", eVarSetOperationAppend);
+  ASSERT_EQ(copy_sp->GetStringValue(), "abc");
+}
+
+// Test an aggregate class.
+TEST(OptionValueArgs, DeepCopy) {
+  OptionValueArgs args;
+  args.SetValueFromString("A B");
+
+  MockCallback callback;
+  args.SetValueChangedCallback([&callback] { callback(); });
+  EXPECT_CALL(callback, Invoke());
+
+  auto copy_sp = args.DeepCopy(nullptr);
+
+  // Test that the base class data members are copied/set correctly.
+  ASSERT_TRUE(copy_sp);
+  ASSERT_EQ(copy_sp->GetParent(), nullptr);
+  ASSERT_TRUE(copy_sp->OptionWasSet());
+
+  auto *args_copy_ptr = copy_sp->GetAsArgs();
+  ASSERT_EQ(args_copy_ptr->GetSize(), 2U);
+  ASSERT_EQ((*args_copy_ptr)[0]->GetParent(), copy_sp);
+  ASSERT_EQ((*args_copy_ptr)[0]->GetStringValue(), "A");
+  ASSERT_EQ((*args_copy_ptr)[1]->GetParent(), copy_sp);
+  ASSERT_EQ((*args_copy_ptr)[1]->GetStringValue(), "B");
+
+  // Trigger the callback.
+  copy_sp->SetValueFromString("C", eVarSetOperationAppend);
+  ASSERT_TRUE(args_copy_ptr);
+  ASSERT_EQ(args_copy_ptr->GetSize(), 3U);
+  ASSERT_EQ((*args_copy_ptr)[2]->GetStringValue(), "C");
+}
+
+class TestProperties : public OptionValueProperties {
+public:
+  static std::shared_ptr<TestProperties> CreateGlobal() {
+    auto props_sp = std::make_shared<TestProperties>();
+    const bool is_global = false;
+
+    auto dict_sp = std::make_shared<OptionValueDictionary>(1 << eTypeUInt64);
+    props_sp->AppendProperty(ConstString("dict"), ConstString(), is_global,
+                             dict_sp);
+
+    auto file_list_sp = std::make_shared<OptionValueFileSpecList>();
+    props_sp->AppendProperty(ConstString("file-list"), ConstString(), is_global,
+                             file_list_sp);
+    return props_sp;
+  }
+
+  void SetDictionaryChangedCallback(const MockCallback &callback) {
+    SetValueChangedCallback(m_dict_index, [&callback] { callback(); });
+  }
+
+  void SetFileListChangedCallback(const MockCallback &callback) {
+    SetValueChangedCallback(m_file_list_index, [&callback] { callback(); });
+  }
+
+  OptionValueDictionary *GetDictionary() {
+    return GetPropertyAtIndexAsOptionValueDictionary(nullptr, m_dict_index);
+  }
+
+  OptionValueFileSpecList *GetFileList() {
+    return GetPropertyAtIndexAsOptionValueFileSpecList(nullptr, true,
+                                                       m_file_list_index);
+  }
+
+private:
+  lldb::OptionValueSP Clone() const {
+    return std::make_shared<TestProperties>(*this);
+  }
+
+  uint32_t m_dict_index = 0;
+  uint32_t m_file_list_index = 1;
+};
+
+// Test a user-defined propery class.
+TEST(TestProperties, DeepCopy) {
+  auto props_sp = TestProperties::CreateGlobal();
+  props_sp->GetDictionary()->SetValueFromString("A=1 B=2");
+  props_sp->GetFileList()->SetValueFromString("path/to/file");
+
+  MockCallback callback;
+  props_sp->SetDictionaryChangedCallback(callback);
+  props_sp->SetFileListChangedCallback(callback);
+  EXPECT_CALL(callback, Invoke()).Times(2);
+
+  auto copy_sp = props_sp->DeepCopy(nullptr);
+
+  // Test that the base class data members are copied/set correctly.
+  ASSERT_TRUE(copy_sp);
+  ASSERT_EQ(copy_sp->GetParent(), nullptr);
+
+  // This cast is safe only if the class overrides Clone().
+  auto *props_copy_ptr = static_cast<TestProperties *>(copy_sp.get());
+  ASSERT_TRUE(props_copy_ptr);
+
+  // Test the first child.
+  auto dict_copy_ptr = props_copy_ptr->GetDictionary();
+  ASSERT_TRUE(dict_copy_ptr);
+  ASSERT_EQ(dict_copy_ptr->GetParent(), copy_sp);
+  ASSERT_TRUE(dict_copy_ptr->OptionWasSet());
+  ASSERT_EQ(dict_copy_ptr->GetNumValues(), 2U);
+
+  auto value_ptr = dict_copy_ptr->GetValueForKey(ConstString("A"));
+  ASSERT_TRUE(value_ptr);
+  ASSERT_EQ(value_ptr->GetParent().get(), dict_copy_ptr);
+  ASSERT_EQ(value_ptr->GetUInt64Value(), 1U);
+
+  value_ptr = dict_copy_ptr->GetValueForKey(ConstString("B"));
+  ASSERT_TRUE(value_ptr);
+  ASSERT_EQ(value_ptr->GetParent().get(), dict_copy_ptr);
+  ASSERT_EQ(value_ptr->GetUInt64Value(), 2U);
+
+  // Test the second child.
+  auto file_list_copy_ptr = props_copy_ptr->GetFileList();
+  ASSERT_TRUE(file_list_copy_ptr);
+  ASSERT_EQ(file_list_copy_ptr->GetParent(), copy_sp);
+  ASSERT_TRUE(file_list_copy_ptr->OptionWasSet());
+
+  auto file_list_copy = file_list_copy_ptr->GetCurrentValue();
+  ASSERT_EQ(file_list_copy.GetSize(), 1U);
+  ASSERT_EQ(file_list_copy.GetFileSpecAtIndex(0), FileSpec("path/to/file"));
+
+  // Trigger the callback first time.
+  dict_copy_ptr->SetValueFromString("C=3", eVarSetOperationAppend);
+
+  // Trigger the callback second time.
+  file_list_copy_ptr->SetValueFromString("0 another/path", eVarSetOperationReplace);
+}