CRTP-based version of D96817

[tatyana-krasnukha]

Summary

Being graceful, this approach has a few drawbacks. As I wrote, this pattern is not very popular, and when a developer defines a derived class they may not know that the class must inherit from the Clonable.
There is also a hardly probable but still possible pitfall of copy-pasting an existing class. For example, we define OptionValueSInt64 by copying OptionValueUInt64. If we forget to rename the template argument, we will not have any compile-time errors:

class OptionValueSInt64 : public Cloneable<OptionValueUInt64, OptionValue> {
...
};

Now our Clone function turn into:

std::shared_ptr<OptionValue> OptionValueSInt64 ::Clone() const override {
  return std::make_shared<OptionValueUInt64>(static_cast<const OptionValueUInt64&>(*this));
}

So, we get static_cast from OptionValueSInt64 to OptionValueUInt64 and undefined behaviour.

[dblaikie]

I guess a lot of the things I highlighted as "unrelated refactoring" are actually the various bugfixes the patch is about?

It might be worth separating those into separate patches in a series to make it clear what issues are being fixed in each change.

Fair point about the risk of mistyping something. I'm not super fussed either way - just like to avoid rewriting these functions over and over again.

(also, would it be possible for the non-trivial impelmentations to be made trivial by moving the work into the copy constructor?)

lldb/include/lldb/Interpreter/OptionValueArch.h

29–30

There's no need to include "Cloneable(), " here, it's implicit. (indeed you could remove the "OptionValue(), " here in a separate preliminary commit to make this patch's diff smaller/easier to read.

lldb/include/lldb/Interpreter/OptionValueArray.h

41–43

Why does this need to be here?

lldb/include/lldb/Utility/Cloneable.h

47–48

I think you can put the static_assert at the class scope, rather than needing to put it inside a function - having it inside a function here makes the type non-trivially destructible which would be unfortunate/unnecessary overhead.

lldb/source/Interpreter/OptionValueArgs.cpp

16–18 ↗

(On Diff #324604)

Is this unrelated refactoring?

lldb/source/Interpreter/OptionValueProperties.cpp

231–275

Is this unrelated refactoring/changes?

545–571

And this?

lldb/source/Target/Process.cpp

91–95

unrelated refactoring?

lldb/source/Target/Target.cpp

3652–3658

This class didn't have a DeepCopy function before - does it need one now? Or is this an intermediate abstract base, in which case it doesn't need to use Cloneable - only the most derived classes (well, the ones that are instantiated, be they most derived or not) need Cloneable, I think?

3670–3672

Why this change?

Oh, is the previous code incorrect - binding two unrelated shared_ptrs to the same memory? Might be best to commit this as a separate change/bugfix?

4200

Unrelated refactoring?

[JDevlieghere]

Everything being equal I think I slightly prefer this patch over the original.

[tatyana-krasnukha]

Thanks for such a detailed review, David!
I would love to split off the changes into separate patches, but if I do so, this patch just will not be compileable.

lldb/include/lldb/Interpreter/OptionValueArray.h

41–43

DeepCopy is not trivial for aggregate OptionValue{} types. They must call DeepCopy on every element of their collection.

lldb/include/lldb/Utility/Cloneable.h

47–48

I wrote in the comment that static_assert cannot be put at the class scope because std::is_base_of requires derived type to be complete. Probably, it will be better to put it right into the Clone.

lldb/source/Interpreter/OptionValueProperties.cpp

545–571

OptionValueProperties had this code in the constructor, however, now DeepCopy requires a new parent as the argument. It just is not possible to call shared_from_this from the constructor. That's why I moved this code into the DeepCopy and wrapped the call into CreateLocalCopy.

Another benefit is that now one can create a local copy of their SomeOptionValueProperties even if it contains sub-properties as elements.

lldb/source/Target/Process.cpp

91–95

OptionValueProperties's changes require appropriate changes to the Process/Thread/Target.

lldb/source/Target/Target.cpp

3652–3658

This class is the most derived but it doesn't need to override OptionValueProperties's DeepCopy implementation.

3670–3672

No, it was correct. The answer is the same here: OptionValueProperties's changes require appropriate changes to the Process/Thread/Target.

[tatyana-krasnukha]

Removed unrelated changes from the patch.

[dblaikie]

Looks close enough for my level of interest/knowledge. I haven't looked closely at the related cleanups to better understand why they're needed, but I'd take your word for it.

I'd prefer/like if a more seasoned LLDB developer could give the final approval here, though.

[JDevlieghere]

LGTM

lldb/source/Interpreter/OptionValueFileSpecList.cpp

169

I would call the Clone from Cloneable here, it's a bit messy with the templates but it makes it makes it clear we're not hand-rolling an implementation but instead are just protecting it wiht a mutex.

[tatyana-krasnukha]

lldb/source/Interpreter/OptionValueFileSpecList.cpp

169

Thanks! Did this before landing.

lldb/include/lldb/Interpreter/OptionValue.h

 //===-- OptionValue.h -------------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUE_H
 #define LLDB_INTERPRETER_OPTIONVALUE_H
 
 #include "lldb/Core/FormatEntity.h"
+#include "lldb/Utility/Cloneable.h"
 #include "lldb/Utility/CompletionRequest.h"
 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-private-enumerations.h"
 #include "lldb/lldb-private-interfaces.h"
 
 namespace lldb_private {
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:
+  using TopmostBase = OptionValue;
+
+  // Must be overriden by a derived class for correct downcasting the result of
+  // DeepCopy to it. Inherit from Cloneable to avoid doing this manually.
+  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

 #define LLDB_INTERPRETER_OPTIONVALUEARCH_H
 
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Utility/ArchSpec.h"
 #include "lldb/Utility/CompletionRequest.h"
 
 namespace lldb_private {
 
-class OptionValueArch : public OptionValue {
+class OptionValueArch : public Cloneable<OptionValueArch, OptionValue> {
 public:
   OptionValueArch() = default;
 
   OptionValueArch(const char *triple) : m_current_value(triple) {
     m_default_value = m_current_value;
   }
 
   OptionValueArch(const ArchSpec &value)
       : m_current_value(value), m_default_value(value) {}
 
   OptionValueArch(const ArchSpec &current_value, const ArchSpec &default_value)
       : m_current_value(current_value), m_default_value(default_value) {}
 
   ~OptionValueArch() override = default;
 
   // Virtual subclass pure virtual overrides
 
   OptionValue::Type GetType() const override { return eTypeArch; }
 
   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 {
     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; }

lldb/include/lldb/Interpreter/OptionValueArgs.h

 //===-- OptionValueArgs.h --------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEARGS_H
 #define LLDB_INTERPRETER_OPTIONVALUEARGS_H
 
 #include "lldb/Interpreter/OptionValueArray.h"
 
 namespace lldb_private {
 
-class OptionValueArgs : public OptionValueArray {
+class OptionValueArgs : public Cloneable<OptionValueArgs, OptionValueArray> {
 public:
   OptionValueArgs()
-      : OptionValueArray(
-            OptionValue::ConvertTypeToMask(OptionValue::eTypeString)) {}
+      : Cloneable(OptionValue::ConvertTypeToMask(OptionValue::eTypeString)) {}
 
   ~OptionValueArgs() override = default;
 
   size_t GetArgs(Args &args) const;
 
   Type GetType() const override { return eTypeArgs; }
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEARGS_H

lldb/include/lldb/Interpreter/OptionValueArray.h

 #define LLDB_INTERPRETER_OPTIONVALUEARRAY_H
 
 #include <vector>
 
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueArray : public OptionValue {
+class OptionValueArray : public Cloneable<OptionValueArray, OptionValue> {
 public:
   OptionValueArray(uint32_t type_mask = UINT32_MAX, bool raw_value_dump = false)
       : m_type_mask(type_mask), m_values(), m_raw_value_dump(raw_value_dump) {}
 
   ~OptionValueArray() override = default;
 
   // Virtual subclass pure virtual overrides
 
   OptionValue::Type GetType() const override { return eTypeArray; }
 
   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 {
     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
 

lldb/include/lldb/Interpreter/OptionValueBoolean.h

 //===-- OptionValueBoolean.h ------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEBOOLEAN_H
 #define LLDB_INTERPRETER_OPTIONVALUEBOOLEAN_H
 
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueBoolean : public OptionValue {
+class OptionValueBoolean : public Cloneable<OptionValueBoolean, OptionValue> {
 public:
   OptionValueBoolean(bool value)
       : m_current_value(value), m_default_value(value) {}
   OptionValueBoolean(bool current_value, bool default_value)
       : m_current_value(current_value), m_default_value(default_value) {}
 
   ~OptionValueBoolean() override = default;
 
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:
   bool m_current_value;
   bool m_default_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUEBOOLEAN_H

lldb/include/lldb/Interpreter/OptionValueChar.h

 //===-- OptionValueChar.h ---------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUECHAR_H
 #define LLDB_INTERPRETER_OPTIONVALUECHAR_H
 
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueChar : public OptionValue {
+class OptionValueChar : public Cloneable<OptionValueChar, OptionValue> {
 public:
   OptionValueChar(char value)
       : m_current_value(value), m_default_value(value) {}
 
   OptionValueChar(char current_value, char default_value)
       : m_current_value(current_value), m_default_value(default_value) {}
 
   ~OptionValueChar() override = default;
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:
   char m_current_value;
   char m_default_value;
 };
 
 } // namespace lldb_private
 
 #endif // LLDB_INTERPRETER_OPTIONVALUECHAR_H

lldb/include/lldb/Interpreter/OptionValueDictionary.h

 #define LLDB_INTERPRETER_OPTIONVALUEDICTIONARY_H
 
 #include <map>
 
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueDictionary : public OptionValue {
+class OptionValueDictionary
+    : public Cloneable<OptionValueDictionary, OptionValue> {
 public:
   OptionValueDictionary(uint32_t type_mask = UINT32_MAX,
                         bool raw_value_dump = true)
       : m_type_mask(type_mask), m_raw_value_dump(raw_value_dump) {}
 
   ~OptionValueDictionary() override = default;
 
   // Virtual subclass pure virtual overrides
 
   OptionValue::Type GetType() const override { return eTypeDictionary; }
 
   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 {
     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

lldb/include/lldb/Interpreter/OptionValueEnumeration.h

 #include "lldb/Utility/ConstString.h"
 #include "lldb/Utility/Status.h"
 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StreamString.h"
 #include "lldb/lldb-private-types.h"
 
 namespace lldb_private {
 
-class OptionValueEnumeration : public OptionValue {
+class OptionValueEnumeration
+    : public Cloneable<OptionValueEnumeration, OptionValue> {
 public:
   typedef int64_t enum_type;
   struct EnumeratorInfo {
     enum_type value;
     const char *description;
   };
   typedef UniqueCStringMap<EnumeratorInfo> EnumerationMap;
   typedef EnumerationMap::Entry EnumerationMapEntry;
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;

lldb/include/lldb/Interpreter/OptionValueFileColonLine.h

 
 #include "lldb/Interpreter/OptionValue.h"
 
 #include "lldb/Utility/FileSpec.h"
 #include "llvm/Support/Chrono.h"
 
 namespace lldb_private {
 
-class OptionValueFileColonLine : public OptionValue {
+class OptionValueFileColonLine :
+    public Cloneable<OptionValueFileColonLine, OptionValue> {
 public:
   OptionValueFileColonLine();
   OptionValueFileColonLine(const llvm::StringRef input);
 
   ~OptionValueFileColonLine() override = default;
 
   OptionValue::Type GetType() const override { return eTypeFileLineColumn; }
 
   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 {
     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; }

lldb/include/lldb/Interpreter/OptionValueFileSpec.h

 
 #include "lldb/Interpreter/OptionValue.h"
 
 #include "lldb/Utility/FileSpec.h"
 #include "llvm/Support/Chrono.h"
 
 namespace lldb_private {
 
-class OptionValueFileSpec : public OptionValue {
+class OptionValueFileSpec : public Cloneable<OptionValueFileSpec, OptionValue> {
 public:
   OptionValueFileSpec(bool resolve = true);
 
   OptionValueFileSpec(const FileSpec &value, bool resolve = true);
 
   OptionValueFileSpec(const FileSpec &current_value,
                       const FileSpec &default_value, bool resolve = true);
 
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; }

lldb/include/lldb/Interpreter/OptionValueFileSpecList.h

 
 #include <mutex>
 
 #include "lldb/Core/FileSpecList.h"
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueFileSpecList : public OptionValue {
+class OptionValueFileSpecList
+    : public Cloneable<OptionValueFileSpecList, OptionValue> {
 public:
   OptionValueFileSpecList() = default;
 
-  OptionValueFileSpecList(const FileSpecList &current_value)
-      : m_current_value(current_value) {}
+  OptionValueFileSpecList(const OptionValueFileSpecList &other)
+      : Cloneable(other), m_current_value(other.GetCurrentValue()) {}
 
   ~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

 //===-- OptionValueFormat.h -------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEFORMAT_H
 #define LLDB_INTERPRETER_OPTIONVALUEFORMAT_H
 
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueFormat : public OptionValue {
+class OptionValueFormat
+    : public Cloneable<OptionValueFormat, OptionValue> {
 public:
   OptionValueFormat(lldb::Format value)
       : m_current_value(value), m_default_value(value) {}
 
   OptionValueFormat(lldb::Format current_value, lldb::Format default_value)
       : m_current_value(current_value), m_default_value(default_value) {}
 
   ~OptionValueFormat() override = default;
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; }
 

lldb/include/lldb/Interpreter/OptionValueFormatEntity.h

 //===-- OptionValueFormatEntity.h --------------------------------*- C++-*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEFORMATENTITY_H
 #define LLDB_INTERPRETER_OPTIONVALUEFORMATENTITY_H
 
 #include "lldb/Core/FormatEntity.h"
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueFormatEntity : public OptionValue {
+class OptionValueFormatEntity
+    : public Cloneable<OptionValueFormatEntity, OptionValue> {
 public:
   OptionValueFormatEntity(const char *default_format);
 
   ~OptionValueFormatEntity() override = default;
 
   // Virtual subclass pure virtual overrides
 
   OptionValue::Type GetType() const override { return eTypeFormatEntity; }
 
   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; }

lldb/include/lldb/Interpreter/OptionValueLanguage.h

 #ifndef LLDB_INTERPRETER_OPTIONVALUELANGUAGE_H
 #define LLDB_INTERPRETER_OPTIONVALUELANGUAGE_H
 
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/lldb-enumerations.h"
 
 namespace lldb_private {
 
-class OptionValueLanguage : public OptionValue {
+class OptionValueLanguage : public Cloneable<OptionValueLanguage, OptionValue> {
 public:
   OptionValueLanguage(lldb::LanguageType value)
       : m_current_value(value), m_default_value(value) {}
 
   OptionValueLanguage(lldb::LanguageType current_value,
                       lldb::LanguageType default_value)
       : m_current_value(current_value), m_default_value(default_value) {}
 
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; }
 

lldb/include/lldb/Interpreter/OptionValuePathMappings.h

 //===-- OptionValuePathMappings.h -------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEPATHMAPPINGS_H
 #define LLDB_INTERPRETER_OPTIONVALUEPATHMAPPINGS_H
 
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Target/PathMappingList.h"
 
 namespace lldb_private {
 
-class OptionValuePathMappings : public OptionValue {
+class OptionValuePathMappings
+    : public Cloneable<OptionValuePathMappings, OptionValue> {
 public:
   OptionValuePathMappings(bool notify_changes)
       : m_notify_changes(notify_changes) {}
 
   ~OptionValuePathMappings() override = default;
 
   // Virtual subclass pure virtual overrides
 
   OptionValue::Type GetType() const override { return eTypePathMap; }
 
   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 {
     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:
   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 Cloneable<OptionValueProperties, OptionValue>,
       public std::enable_shared_from_this<OptionValueProperties> {
 public:
   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;
 

lldb/include/lldb/Interpreter/OptionValueRegex.h

 //===-- OptionValueRegex.h --------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEREGEX_H
 #define LLDB_INTERPRETER_OPTIONVALUEREGEX_H
 
 #include "lldb/Interpreter/OptionValue.h"
 #include "lldb/Utility/RegularExpression.h"
 
 namespace lldb_private {
 
-class OptionValueRegex : public OptionValue {
+class OptionValueRegex : public Cloneable<OptionValueRegex, OptionValue> {
 public:
   OptionValueRegex(const char *value = nullptr)
       : m_regex(llvm::StringRef::withNullAsEmpty(value)),
         m_default_regex_str(llvm::StringRef::withNullAsEmpty(value).str()) {}
 
   ~OptionValueRegex() override = default;
 
   // Virtual subclass pure virtual overrides
 
   OptionValue::Type GetType() const override { return eTypeRegex; }
 
   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 {
     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));

lldb/include/lldb/Interpreter/OptionValueSInt64.h

 //===-- OptionValueSInt64.h --------------------------------------*- C++
 //-*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUESINT64_H
 #define LLDB_INTERPRETER_OPTIONVALUESINT64_H
 
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueSInt64 : public OptionValue {
+class OptionValueSInt64 : public Cloneable<OptionValueSInt64, OptionValue> {
 public:
   OptionValueSInt64() = default;
 
   OptionValueSInt64(int64_t value)
       : m_current_value(value), m_default_value(value) {}
 
   OptionValueSInt64(int64_t current_value, int64_t default_value)
       : m_current_value(current_value), m_default_value(default_value) {}
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; }

lldb/include/lldb/Interpreter/OptionValueString.h

 #include <string>
 
 #include "lldb/Utility/Flags.h"
 
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueString : public OptionValue {
+class OptionValueString : public Cloneable<OptionValueString, OptionValue> {
 public:
   typedef Status (*ValidatorCallback)(const char *string, void *baton);
 
   enum Options { eOptionEncodeCharacterEscapeSequences = (1u << 0) };
 
   OptionValueString() = default;
 
   OptionValueString(ValidatorCallback validator, void *baton = nullptr)
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));

lldb/include/lldb/Interpreter/OptionValueUInt64.h

 //===-- OptionValueUInt64.h --------------------------------------*- C++
 //-*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEUINT64_H
 #define LLDB_INTERPRETER_OPTIONVALUEUINT64_H
 
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueUInt64 : public OptionValue {
+class OptionValueUInt64 : public Cloneable<OptionValueUInt64, OptionValue> {
 public:
   OptionValueUInt64() = default;
 
   OptionValueUInt64(uint64_t value)
       : m_current_value(value), m_default_value(value) {}
 
   OptionValueUInt64(uint64_t current_value, uint64_t default_value)
       : m_current_value(current_value), m_default_value(default_value) {}
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; }

lldb/include/lldb/Interpreter/OptionValueUUID.h

 //===-- OptionValueUUID.h --------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLDB_INTERPRETER_OPTIONVALUEUUID_H
 #define LLDB_INTERPRETER_OPTIONVALUEUUID_H
 
 #include "lldb/Utility/UUID.h"
 #include "lldb/Interpreter/OptionValue.h"
 
 namespace lldb_private {
 
-class OptionValueUUID : public OptionValue {
+class OptionValueUUID : public Cloneable<OptionValueUUID, OptionValue> {
 public:
   OptionValueUUID() = default;
 
   OptionValueUUID(const UUID &uuid) : m_uuid(uuid) {}
 
   ~OptionValueUUID() override = default;
 
   // Virtual subclass pure virtual overrides
 
   OptionValue::Type GetType() const override { return eTypeUUID; }
 
   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 {
     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; }
 

lldb/include/lldb/Utility/Cloneable.h

+//===-- Cloneable.h ---------------------------------------------*- C++ -*-===//
+//
+// 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
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLDB_UTILITY_CLONEABLE_H
+#define LLDB_UTILITY_CLONEABLE_H
+
+#include <type_traits>
+
+namespace lldb_private {
+
+/// \class Cloneable Cloneable.h "lldb/Utility/Cloneable.h"
+/// A class that implements CRTP-based "virtual constructor" idiom.
+///
+/// Example:
+/// @code
+/// class Base {
+///   using TopmostBase = Base;
+/// public:
+///   virtual std::shared_ptr<Base> Clone() const = 0;
+/// };
+/// @endcode
+///
+/// To define a class derived from the Base with overridden Clone:
+/// @code
+/// class Intermediate : public Cloneable<Intermediate, Base> {};
+/// @endcode
+///
+/// To define a class at the next level of inheritance with overridden Clone:
+/// @code
+/// class Derived : public Cloneable<Derived, Intermediate> {};
+/// @endcode
+
+template <typename Derived, typename Base>
+class Cloneable : public Base {
+public:
+  using Base::Base;
+
+  std::shared_ptr<typename Base::TopmostBase> Clone() const override {
+    // std::is_base_of requires derived type to be complete, that's why class
+    // scope static_assert cannot be used.
+    static_assert(std::is_base_of<Cloneable, Derived>::value,
+                  "Derived class must be derived from this.");
+
+    return std::make_shared<Derived>(static_cast<const Derived &>(*this));
+  }
+};
+
+} // namespace lldb_private
+
+#endif // LLDB_UTILITY_CLONEABLE_H

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));
-}
-
 void OptionValueArch::AutoComplete(CommandInterpreter &interpreter,
                                    CompletionRequest &request) {
   CommandCompletions::InvokeCommonCompletionCallbacks(
       interpreter, CommandCompletions::eArchitectureCompletion, request,
       nullptr);
 }

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::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));
-}
-
 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,
   } break;
 
   default:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
-
-lldb::OptionValueSP OptionValueChar::DeepCopy() const {
-  return OptionValueSP(new 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::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));
-}
-
 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));
-}
-
 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));
-}
-
 void OptionValueFileSpec::AutoComplete(CommandInterpreter &interpreter,
                                        CompletionRequest &request) {
   CommandCompletions::InvokeCommonCompletionCallbacks(
       interpreter, m_completion_mask, request, nullptr);
 }
 
 const lldb::DataBufferSP &OptionValueFileSpec::GetFileContents() {
   if (m_current_value) {

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 Cloneable::Clone();
 }

lldb/source/Interpreter/OptionValueFormat.cpp

Status OptionValueFormat::SetValueFromString(llvm::StringRef value,
   case eVarSetOperationRemove:
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
-
-lldb::OptionValueSP OptionValueFormat::DeepCopy() const {
-  return OptionValueSP(new 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));
-}
-
 void OptionValueFormatEntity::AutoComplete(CommandInterpreter &interpreter,
                                            CompletionRequest &request) {
   FormatEntity::AutoComplete(request);
 }

lldb/source/Interpreter/OptionValueLanguage.cpp

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

lldb/source/Interpreter/OptionValuePathMappings.cpp

case eVarSetOperationRemove:
     break;
 
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value, op);
     break;
   }
   return error;
 }
-
-lldb::OptionValueSP OptionValuePathMappings::DeepCopy() const {
-  return OptionValueSP(new OptionValuePathMappings(*this));
-}

lldb/source/Interpreter/OptionValueProperties.cpp

 #include "lldb/Utility/Stream.h"
 #include "lldb/Utility/StringList.h"
 
 using namespace lldb;
 using namespace lldb_private;
 
 OptionValueProperties::OptionValueProperties(ConstString name) : m_name(name) {}
 
-OptionValueProperties::OptionValueProperties(
-    const OptionValueProperties &global_properties)
-    : OptionValue(global_properties),
-      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)
     return false;
 
   OptionValue *value = property->GetValue().get();
   if (!value)
     return false;
 
   const OptionValueArgs *arguments = value->GetAsArgs();
   if (arguments)
     return arguments->GetArgs(args);
 
   const OptionValueArray *array = value->GetAsArray();
   if (array)
     return array->GetArgs(args);
 
   const OptionValueDictionary *dict = value->GetAsDictionary();
   if (dict)
     return dict->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)
     return false;
 
   OptionValue *value = property->GetValue().get();
   if (!value)
     return false;
 
   OptionValueArgs *arguments = value->GetAsArgs();
   if (arguments)
     return arguments->SetArgs(args, eVarSetOperationAssign).Success();
 
   OptionValueArray *array = value->GetAsArray();
   if (array)
     return array->SetArgs(args, eVarSetOperationAssign).Success();
 
   OptionValueDictionary *dict = value->GetAsDictionary();
   if (dict)
     return dict->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)
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::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;
   ConstString key;

lldb/source/Interpreter/OptionValueRegex.cpp

if (m_regex.IsValid()) {
       error.SetErrorString(llvm::toString(std::move(err)));
     } else {
       error.SetErrorString("regex error");
     }
     break;
   }
   return error;
 }
-
-lldb::OptionValueSP OptionValueRegex::DeepCopy() const {
-  return OptionValueSP(new OptionValueRegex(m_regex.GetText().str().c_str()));
-}

lldb/source/Interpreter/OptionValueSInt64.cpp

Status OptionValueSInt64::SetValueFromString(llvm::StringRef value_ref,
   case eVarSetOperationRemove:
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value_ref, op);
     break;
   }
   return error;
 }
-
-lldb::OptionValueSP OptionValueSInt64::DeepCopy() const {
-  return OptionValueSP(new 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));
-}
-
 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;
   }
   m_current_value.assign(std::string(value));
   return Status();

lldb/source/Interpreter/OptionValueUInt64.cpp

Status OptionValueUInt64::SetValueFromString(llvm::StringRef value_ref,
   case eVarSetOperationRemove:
   case eVarSetOperationAppend:
   case eVarSetOperationInvalid:
     error = OptionValue::SetValueFromString(value_ref, op);
     break;
   }
   return error;
 }
-
-lldb::OptionValueSP OptionValueUInt64::DeepCopy() const {
-  return OptionValueSP(new 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));
-}
-
 void OptionValueUUID::AutoComplete(CommandInterpreter &interpreter,
                                    CompletionRequest &request) {
   ExecutionContext exe_ctx(interpreter.GetExecutionContext());
   Target *target = exe_ctx.GetTargetPtr();
   if (!target)
     return;
   auto prefix = request.GetCursorArgumentPrefix();
   llvm::SmallVector<uint8_t, 20> uuid_bytes;

lldb/source/Target/Process.cpp

 #define ENABLE_MEMORY_CACHING
 
 #ifdef ENABLE_MEMORY_CACHING
 #define DISABLE_MEM_CACHE_DEFAULT false
 #else
 #define DISABLE_MEM_CACHE_DEFAULT true
 #endif
 
-class ProcessOptionValueProperties : public OptionValueProperties {
+class ProcessOptionValueProperties
+    : public Cloneable<ProcessOptionValueProperties, 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()) {}
+  ProcessOptionValueProperties(ConstString name) : Cloneable(name) {}
 
   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) {

 #define LLDB_PROPERTIES_process_experimental
 #include "TargetProperties.inc"
 
 enum {
 #define LLDB_PROPERTIES_process_experimental
 #include "TargetPropertiesEnum.inc"
 };
 
-class ProcessExperimentalOptionValueProperties : public OptionValueProperties {
+class ProcessExperimentalOptionValueProperties
+    : public Cloneable<ProcessExperimentalOptionValueProperties,
+                       OptionValueProperties> {
 public:
   ProcessExperimentalOptionValueProperties()
-      : OptionValueProperties(
+      : Cloneable(
             ConstString(Properties::GetExperimentalSettingsName())) {}
 };
 
 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 "TargetProperties.inc"
 
 enum {
 #define LLDB_PROPERTIES_target
 #include "TargetPropertiesEnum.inc"
   ePropertyExperimental,
 };
 
-class TargetOptionValueProperties : public OptionValueProperties {
+class TargetOptionValueProperties
+    : public Cloneable<TargetOptionValueProperties, 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()) {}
+  TargetOptionValueProperties(ConstString name) : Cloneable(name) {}
 
   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) {

 #define LLDB_PROPERTIES_target_experimental
 #include "TargetProperties.inc"
 
 enum {
 #define LLDB_PROPERTIES_target_experimental
 #include "TargetPropertiesEnum.inc"
 };
 
-class TargetExperimentalOptionValueProperties : public OptionValueProperties {
+class TargetExperimentalOptionValueProperties
+    : public Cloneable<TargetExperimentalOptionValueProperties,
+                       OptionValueProperties> {
 public:
   TargetExperimentalOptionValueProperties()
-      : OptionValueProperties(
-            ConstString(Properties::GetExperimentalSettingsName())) {}
+      : Cloneable(ConstString(Properties::GetExperimentalSettingsName())) {}
 };
 
 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(
         ePropertyEnvVars, [this] { EnvVarsValueChangedCallback(); });

 }
 
 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
   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

 #define LLDB_PROPERTIES_thread
 #include "TargetProperties.inc"
 
 enum {
 #define LLDB_PROPERTIES_thread
 #include "TargetPropertiesEnum.inc"
 };
 
-class ThreadOptionValueProperties : public OptionValueProperties {
+class ThreadOptionValueProperties
+    : public Cloneable<ThreadOptionValueProperties, 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()) {}
+  ThreadOptionValueProperties(ConstString name) : Cloneable(name) {}
 
   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) {

 };
 
 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);
+}