Index: lldb/include/lldb/Interpreter/OptionValue.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValue.h +++ lldb/include/lldb/Interpreter/OptionValue.h @@ -10,6 +10,7 @@ #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" @@ -87,7 +88,8 @@ 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); @@ -306,6 +308,8 @@ m_parent_wp = parent_sp; } + lldb::OptionValueSP GetParent() const { return m_parent_wp.lock(); } + void SetValueChangedCallback(std::function callback) { assert(!m_callback); m_callback = std::move(callback); @@ -317,6 +321,12 @@ } 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 m_callback; bool m_value_was_set; // This can be used to see if a value has been set Index: lldb/include/lldb/Interpreter/OptionValueArch.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueArch.h +++ lldb/include/lldb/Interpreter/OptionValueArch.h @@ -15,7 +15,7 @@ namespace lldb_private { -class OptionValueArch : public OptionValue { +class OptionValueArch : public Cloneable { public: OptionValueArch() = default; @@ -47,8 +47,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - void AutoComplete(CommandInterpreter &interpreter, lldb_private::CompletionRequest &request) override; Index: lldb/include/lldb/Interpreter/OptionValueArgs.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueArgs.h +++ lldb/include/lldb/Interpreter/OptionValueArgs.h @@ -13,11 +13,10 @@ namespace lldb_private { -class OptionValueArgs : public OptionValueArray { +class OptionValueArgs : public Cloneable { public: OptionValueArgs() - : OptionValueArray( - OptionValue::ConvertTypeToMask(OptionValue::eTypeString)) {} + : Cloneable(OptionValue::ConvertTypeToMask(OptionValue::eTypeString)) {} ~OptionValueArgs() override = default; Index: lldb/include/lldb/Interpreter/OptionValueArray.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueArray.h +++ lldb/include/lldb/Interpreter/OptionValueArray.h @@ -15,7 +15,7 @@ namespace lldb_private { -class OptionValueArray : public OptionValue { +class OptionValueArray : public Cloneable { 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) {} @@ -38,7 +38,8 @@ 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; } Index: lldb/include/lldb/Interpreter/OptionValueBoolean.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueBoolean.h +++ lldb/include/lldb/Interpreter/OptionValueBoolean.h @@ -13,7 +13,7 @@ namespace lldb_private { -class OptionValueBoolean : public OptionValue { +class OptionValueBoolean : public Cloneable { public: OptionValueBoolean(bool value) : m_current_value(value), m_default_value(value) {} @@ -71,8 +71,6 @@ void SetDefaultValue(bool value) { m_default_value = value; } - lldb::OptionValueSP DeepCopy() const override; - protected: bool m_current_value; bool m_default_value; Index: lldb/include/lldb/Interpreter/OptionValueChar.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueChar.h +++ lldb/include/lldb/Interpreter/OptionValueChar.h @@ -13,7 +13,7 @@ namespace lldb_private { -class OptionValueChar : public OptionValue { +class OptionValueChar : public Cloneable { public: OptionValueChar(char value) : m_current_value(value), m_default_value(value) {} @@ -54,8 +54,6 @@ void SetDefaultValue(char value) { m_default_value = value; } - lldb::OptionValueSP DeepCopy() const override; - protected: char m_current_value; char m_default_value; Index: lldb/include/lldb/Interpreter/OptionValueDictionary.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueDictionary.h +++ lldb/include/lldb/Interpreter/OptionValueDictionary.h @@ -15,7 +15,8 @@ namespace lldb_private { -class OptionValueDictionary : public OptionValue { +class OptionValueDictionary + : public Cloneable { public: OptionValueDictionary(uint32_t type_mask = UINT32_MAX, bool raw_value_dump = true) @@ -39,7 +40,8 @@ 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; } Index: lldb/include/lldb/Interpreter/OptionValueEnumeration.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueEnumeration.h +++ lldb/include/lldb/Interpreter/OptionValueEnumeration.h @@ -19,7 +19,8 @@ namespace lldb_private { -class OptionValueEnumeration : public OptionValue { +class OptionValueEnumeration + : public Cloneable { public: typedef int64_t enum_type; struct EnumeratorInfo { @@ -49,8 +50,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - void AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) override; Index: lldb/include/lldb/Interpreter/OptionValueFileColonLine.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueFileColonLine.h +++ lldb/include/lldb/Interpreter/OptionValueFileColonLine.h @@ -16,7 +16,8 @@ namespace lldb_private { -class OptionValueFileColonLine : public OptionValue { +class OptionValueFileColonLine : + public Cloneable { public: OptionValueFileColonLine(); OptionValueFileColonLine(const llvm::StringRef input); @@ -38,8 +39,6 @@ m_column_number = LLDB_INVALID_COLUMN_NUMBER; } - lldb::OptionValueSP DeepCopy() const override; - void AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) override; Index: lldb/include/lldb/Interpreter/OptionValueFileSpec.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueFileSpec.h +++ lldb/include/lldb/Interpreter/OptionValueFileSpec.h @@ -16,7 +16,7 @@ namespace lldb_private { -class OptionValueFileSpec : public OptionValue { +class OptionValueFileSpec : public Cloneable { public: OptionValueFileSpec(bool resolve = true); @@ -45,8 +45,6 @@ m_data_mod_time = llvm::sys::TimePoint<>(); } - lldb::OptionValueSP DeepCopy() const override; - void AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) override; Index: lldb/include/lldb/Interpreter/OptionValueFileSpecList.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueFileSpecList.h +++ lldb/include/lldb/Interpreter/OptionValueFileSpecList.h @@ -16,12 +16,13 @@ namespace lldb_private { -class OptionValueFileSpecList : public OptionValue { +class OptionValueFileSpecList + : public Cloneable { public: OptionValueFileSpecList() = default; - OptionValueFileSpecList(const FileSpecList ¤t_value) - : m_current_value(current_value) {} + OptionValueFileSpecList(const OptionValueFileSpecList &other) + : Cloneable(other), m_current_value(other.GetCurrentValue()) {} ~OptionValueFileSpecList() override = default; @@ -42,8 +43,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - bool IsAggregateValue() const override { return true; } // Subclass specific functions @@ -64,6 +63,8 @@ } protected: + lldb::OptionValueSP Clone() const override; + mutable std::recursive_mutex m_mutex; FileSpecList m_current_value; }; Index: lldb/include/lldb/Interpreter/OptionValueFormat.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueFormat.h +++ lldb/include/lldb/Interpreter/OptionValueFormat.h @@ -13,7 +13,8 @@ namespace lldb_private { -class OptionValueFormat : public OptionValue { +class OptionValueFormat + : public Cloneable { public: OptionValueFormat(lldb::Format value) : m_current_value(value), m_default_value(value) {} @@ -39,8 +40,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - // Subclass specific functions lldb::Format GetCurrentValue() const { return m_current_value; } Index: lldb/include/lldb/Interpreter/OptionValueFormatEntity.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueFormatEntity.h +++ lldb/include/lldb/Interpreter/OptionValueFormatEntity.h @@ -14,7 +14,8 @@ namespace lldb_private { -class OptionValueFormatEntity : public OptionValue { +class OptionValueFormatEntity + : public Cloneable { public: OptionValueFormatEntity(const char *default_format); @@ -33,8 +34,6 @@ void Clear() override; - lldb::OptionValueSP DeepCopy() const override; - void AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) override; Index: lldb/include/lldb/Interpreter/OptionValueLanguage.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueLanguage.h +++ lldb/include/lldb/Interpreter/OptionValueLanguage.h @@ -15,7 +15,7 @@ namespace lldb_private { -class OptionValueLanguage : public OptionValue { +class OptionValueLanguage : public Cloneable { public: OptionValueLanguage(lldb::LanguageType value) : m_current_value(value), m_default_value(value) {} @@ -42,8 +42,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - // Subclass specific functions lldb::LanguageType GetCurrentValue() const { return m_current_value; } Index: lldb/include/lldb/Interpreter/OptionValuePathMappings.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValuePathMappings.h +++ lldb/include/lldb/Interpreter/OptionValuePathMappings.h @@ -14,7 +14,8 @@ namespace lldb_private { -class OptionValuePathMappings : public OptionValue { +class OptionValuePathMappings + : public Cloneable { public: OptionValuePathMappings(bool notify_changes) : m_notify_changes(notify_changes) {} @@ -37,8 +38,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - bool IsAggregateValue() const override { return true; } // Subclass specific functions Index: lldb/include/lldb/Interpreter/OptionValueProperties.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueProperties.h +++ lldb/include/lldb/Interpreter/OptionValueProperties.h @@ -18,24 +18,27 @@ #include "lldb/Utility/ConstString.h" namespace lldb_private { +class Properties; class OptionValueProperties - : public OptionValue, + : public Cloneable, public std::enable_shared_from_this { 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, Index: lldb/include/lldb/Interpreter/OptionValueRegex.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueRegex.h +++ lldb/include/lldb/Interpreter/OptionValueRegex.h @@ -14,7 +14,7 @@ namespace lldb_private { -class OptionValueRegex : public OptionValue { +class OptionValueRegex : public Cloneable { public: OptionValueRegex(const char *value = nullptr) : m_regex(llvm::StringRef::withNullAsEmpty(value)), @@ -38,8 +38,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - // Subclass specific functions const RegularExpression *GetCurrentValue() const { return (m_regex.IsValid() ? &m_regex : nullptr); Index: lldb/include/lldb/Interpreter/OptionValueSInt64.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueSInt64.h +++ lldb/include/lldb/Interpreter/OptionValueSInt64.h @@ -14,7 +14,7 @@ namespace lldb_private { -class OptionValueSInt64 : public OptionValue { +class OptionValueSInt64 : public Cloneable { public: OptionValueSInt64() = default; @@ -44,8 +44,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - // Subclass specific functions const int64_t &operator=(int64_t value) { Index: lldb/include/lldb/Interpreter/OptionValueString.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueString.h +++ lldb/include/lldb/Interpreter/OptionValueString.h @@ -17,7 +17,7 @@ namespace lldb_private { -class OptionValueString : public OptionValue { +class OptionValueString : public Cloneable { public: typedef Status (*ValidatorCallback)(const char *string, void *baton); @@ -78,8 +78,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - // Subclass specific functions Flags &GetOptions() { return m_options; } Index: lldb/include/lldb/Interpreter/OptionValueUInt64.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueUInt64.h +++ lldb/include/lldb/Interpreter/OptionValueUInt64.h @@ -14,7 +14,7 @@ namespace lldb_private { -class OptionValueUInt64 : public OptionValue { +class OptionValueUInt64 : public Cloneable { public: OptionValueUInt64() = default; @@ -47,8 +47,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - // Subclass specific functions const uint64_t &operator=(uint64_t value) { Index: lldb/include/lldb/Interpreter/OptionValueUUID.h =================================================================== --- lldb/include/lldb/Interpreter/OptionValueUUID.h +++ lldb/include/lldb/Interpreter/OptionValueUUID.h @@ -14,7 +14,7 @@ namespace lldb_private { -class OptionValueUUID : public OptionValue { +class OptionValueUUID : public Cloneable { public: OptionValueUUID() = default; @@ -38,8 +38,6 @@ m_value_was_set = false; } - lldb::OptionValueSP DeepCopy() const override; - // Subclass specific functions UUID &GetCurrentValue() { return m_uuid; } Index: lldb/include/lldb/Utility/Cloneable.h =================================================================== --- /dev/null +++ lldb/include/lldb/Utility/Cloneable.h @@ -0,0 +1,55 @@ +//===-- 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 + +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 Clone() const = 0; +/// }; +/// @endcode +/// +/// To define a class derived from the Base with overridden Clone: +/// @code +/// class Intermediate : public Cloneable {}; +/// @endcode +/// +/// To define a class at the next level of inheritance with overridden Clone: +/// @code +/// class Derived : public Cloneable {}; +/// @endcode + +template +class Cloneable : public Base { +public: + using Base::Base; + + std::shared_ptr 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::value, + "Derived class must be derived from this."); + + return std::make_shared(static_cast(*this)); + } +}; + +} // namespace lldb_private + +#endif // LLDB_UTILITY_CLONEABLE_H Index: lldb/source/Interpreter/OptionValue.cpp =================================================================== --- lldb/source/Interpreter/OptionValue.cpp +++ lldb/source/Interpreter/OptionValue.cpp @@ -568,6 +568,12 @@ 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) {} Index: lldb/source/Interpreter/OptionValueArch.cpp =================================================================== --- lldb/source/Interpreter/OptionValueArch.cpp +++ lldb/source/Interpreter/OptionValueArch.cpp @@ -64,10 +64,6 @@ return error; } -lldb::OptionValueSP OptionValueArch::DeepCopy() const { - return OptionValueSP(new OptionValueArch(*this)); -} - void OptionValueArch::AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) { CommandCompletions::InvokeCommonCompletionCallbacks( Index: lldb/source/Interpreter/OptionValueArray.cpp =================================================================== --- lldb/source/Interpreter/OptionValueArray.cpp +++ lldb/source/Interpreter/OptionValueArray.cpp @@ -303,15 +303,16 @@ 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(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(copy_sp.get()); + lldbassert(array_value_ptr); + + for (auto &value : array_value_ptr->m_values) + value = value->DeepCopy(copy_sp); + + return copy_sp; } Index: lldb/source/Interpreter/OptionValueBoolean.cpp =================================================================== --- lldb/source/Interpreter/OptionValueBoolean.cpp +++ lldb/source/Interpreter/OptionValueBoolean.cpp @@ -67,10 +67,6 @@ 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", Index: lldb/source/Interpreter/OptionValueChar.cpp =================================================================== --- lldb/source/Interpreter/OptionValueChar.cpp +++ lldb/source/Interpreter/OptionValueChar.cpp @@ -57,7 +57,3 @@ } return error; } - -lldb::OptionValueSP OptionValueChar::DeepCopy() const { - return OptionValueSP(new OptionValueChar(*this)); -} Index: lldb/source/Interpreter/OptionValueDictionary.cpp =================================================================== --- lldb/source/Interpreter/OptionValueDictionary.cpp +++ lldb/source/Interpreter/OptionValueDictionary.cpp @@ -311,15 +311,16 @@ 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(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; } Index: lldb/source/Interpreter/OptionValueEnumeration.cpp =================================================================== --- lldb/source/Interpreter/OptionValueEnumeration.cpp +++ lldb/source/Interpreter/OptionValueEnumeration.cpp @@ -95,10 +95,6 @@ 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(); Index: lldb/source/Interpreter/OptionValueFileColonLine.cpp =================================================================== --- lldb/source/Interpreter/OptionValueFileColonLine.cpp +++ lldb/source/Interpreter/OptionValueFileColonLine.cpp @@ -134,10 +134,6 @@ return error; } -lldb::OptionValueSP OptionValueFileColonLine::DeepCopy() const { - return OptionValueSP(new OptionValueFileColonLine(*this)); -} - void OptionValueFileColonLine::AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) { CommandCompletions::InvokeCommonCompletionCallbacks( Index: lldb/source/Interpreter/OptionValueFileSpec.cpp =================================================================== --- lldb/source/Interpreter/OptionValueFileSpec.cpp +++ lldb/source/Interpreter/OptionValueFileSpec.cpp @@ -84,10 +84,6 @@ return error; } -lldb::OptionValueSP OptionValueFileSpec::DeepCopy() const { - return OptionValueSP(new OptionValueFileSpec(*this)); -} - void OptionValueFileSpec::AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) { CommandCompletions::InvokeCommonCompletionCallbacks( Index: lldb/source/Interpreter/OptionValueFileSpecList.cpp =================================================================== --- lldb/source/Interpreter/OptionValueFileSpecList.cpp +++ lldb/source/Interpreter/OptionValueFileSpecList.cpp @@ -164,7 +164,7 @@ return error; } -lldb::OptionValueSP OptionValueFileSpecList::DeepCopy() const { +OptionValueSP OptionValueFileSpecList::Clone() const { std::lock_guard lock(m_mutex); - return OptionValueSP(new OptionValueFileSpecList(m_current_value)); + return std::make_shared(*this); } Index: lldb/source/Interpreter/OptionValueFormat.cpp =================================================================== --- lldb/source/Interpreter/OptionValueFormat.cpp +++ lldb/source/Interpreter/OptionValueFormat.cpp @@ -56,7 +56,3 @@ } return error; } - -lldb::OptionValueSP OptionValueFormat::DeepCopy() const { - return OptionValueSP(new OptionValueFormat(*this)); -} Index: lldb/source/Interpreter/OptionValueFormatEntity.cpp =================================================================== --- lldb/source/Interpreter/OptionValueFormatEntity.cpp +++ lldb/source/Interpreter/OptionValueFormatEntity.cpp @@ -109,10 +109,6 @@ return error; } -lldb::OptionValueSP OptionValueFormatEntity::DeepCopy() const { - return OptionValueSP(new OptionValueFormatEntity(*this)); -} - void OptionValueFormatEntity::AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) { FormatEntity::AutoComplete(request); Index: lldb/source/Interpreter/OptionValueLanguage.cpp =================================================================== --- lldb/source/Interpreter/OptionValueLanguage.cpp +++ lldb/source/Interpreter/OptionValueLanguage.cpp @@ -69,7 +69,3 @@ } return error; } - -lldb::OptionValueSP OptionValueLanguage::DeepCopy() const { - return OptionValueSP(new OptionValueLanguage(*this)); -} Index: lldb/source/Interpreter/OptionValuePathMappings.cpp =================================================================== --- lldb/source/Interpreter/OptionValuePathMappings.cpp +++ lldb/source/Interpreter/OptionValuePathMappings.cpp @@ -196,7 +196,3 @@ } return error; } - -lldb::OptionValueSP OptionValuePathMappings::DeepCopy() const { - return OptionValueSP(new OptionValuePathMappings(*this)); -} Index: lldb/source/Interpreter/OptionValueProperties.cpp =================================================================== --- lldb/source/Interpreter/OptionValueProperties.cpp +++ lldb/source/Interpreter/OptionValueProperties.cpp @@ -22,26 +22,6 @@ 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(); } @@ -562,8 +542,32 @@ 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(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(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( Index: lldb/source/Interpreter/OptionValueRegex.cpp =================================================================== --- lldb/source/Interpreter/OptionValueRegex.cpp +++ lldb/source/Interpreter/OptionValueRegex.cpp @@ -59,7 +59,3 @@ } return error; } - -lldb::OptionValueSP OptionValueRegex::DeepCopy() const { - return OptionValueSP(new OptionValueRegex(m_regex.GetText().str().c_str())); -} Index: lldb/source/Interpreter/OptionValueSInt64.cpp =================================================================== --- lldb/source/Interpreter/OptionValueSInt64.cpp +++ lldb/source/Interpreter/OptionValueSInt64.cpp @@ -70,7 +70,3 @@ } return error; } - -lldb::OptionValueSP OptionValueSInt64::DeepCopy() const { - return OptionValueSP(new OptionValueSInt64(*this)); -} Index: lldb/source/Interpreter/OptionValueString.cpp =================================================================== --- lldb/source/Interpreter/OptionValueString.cpp +++ lldb/source/Interpreter/OptionValueString.cpp @@ -117,10 +117,6 @@ 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)); Index: lldb/source/Interpreter/OptionValueUInt64.cpp =================================================================== --- lldb/source/Interpreter/OptionValueUInt64.cpp +++ lldb/source/Interpreter/OptionValueUInt64.cpp @@ -68,7 +68,3 @@ } return error; } - -lldb::OptionValueSP OptionValueUInt64::DeepCopy() const { - return OptionValueSP(new OptionValueUInt64(*this)); -} Index: lldb/source/Interpreter/OptionValueUUID.cpp =================================================================== --- lldb/source/Interpreter/OptionValueUUID.cpp +++ lldb/source/Interpreter/OptionValueUUID.cpp @@ -58,10 +58,6 @@ return error; } -lldb::OptionValueSP OptionValueUUID::DeepCopy() const { - return OptionValueSP(new OptionValueUUID(*this)); -} - void OptionValueUUID::AutoComplete(CommandInterpreter &interpreter, CompletionRequest &request) { ExecutionContext exe_ctx(interpreter.GetExecutionContext()); Index: lldb/source/Target/Process.cpp =================================================================== --- lldb/source/Target/Process.cpp +++ lldb/source/Target/Process.cpp @@ -83,16 +83,10 @@ #define DISABLE_MEM_CACHE_DEFAULT true #endif -class ProcessOptionValueProperties : public OptionValueProperties { +class ProcessOptionValueProperties + : public Cloneable { 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, @@ -131,10 +125,12 @@ #include "TargetPropertiesEnum.inc" }; -class ProcessExperimentalOptionValueProperties : public OptionValueProperties { +class ProcessExperimentalOptionValueProperties + : public Cloneable { public: ProcessExperimentalOptionValueProperties() - : OptionValueProperties( + : Cloneable( ConstString(Properties::GetExperimentalSettingsName())) {} }; @@ -157,8 +153,8 @@ ConstString("thread"), ConstString("Settings specific to threads."), true, Thread::GetGlobalProperties()->GetValueProperties()); } else { - m_collection_sp = std::make_shared( - Process::GetGlobalProperties().get()); + m_collection_sp = + OptionValueProperties::CreateLocalCopy(*Process::GetGlobalProperties()); m_collection_sp->SetValueChangedCallback( ePropertyPythonOSPluginPath, [this] { m_process->LoadOperatingSystemPlugin(true); }); Index: lldb/source/Target/Target.cpp =================================================================== --- lldb/source/Target/Target.cpp +++ lldb/source/Target/Target.cpp @@ -3615,15 +3615,10 @@ ePropertyExperimental, }; -class TargetOptionValueProperties : public OptionValueProperties { +class TargetOptionValueProperties + : public Cloneable { 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, @@ -3654,11 +3649,12 @@ #include "TargetPropertiesEnum.inc" }; -class TargetExperimentalOptionValueProperties : public OptionValueProperties { +class TargetExperimentalOptionValueProperties + : public Cloneable { public: TargetExperimentalOptionValueProperties() - : OptionValueProperties( - ConstString(Properties::GetExperimentalSettingsName())) {} + : Cloneable(ConstString(Properties::GetExperimentalSettingsName())) {} }; TargetExperimentalProperties::TargetExperimentalProperties() @@ -3671,8 +3667,8 @@ TargetProperties::TargetProperties(Target *target) : Properties(), m_launch_info(), m_target(target) { if (target) { - m_collection_sp = std::make_shared( - Target::GetGlobalProperties()); + m_collection_sp = + OptionValueProperties::CreateLocalCopy(*Target::GetGlobalProperties()); // Set callbacks to update launch_info whenever "settins set" updated any // of these properties Index: lldb/source/Target/Thread.cpp =================================================================== --- lldb/source/Target/Thread.cpp +++ lldb/source/Target/Thread.cpp @@ -71,16 +71,10 @@ #include "TargetPropertiesEnum.inc" }; -class ThreadOptionValueProperties : public OptionValueProperties { +class ThreadOptionValueProperties + : public Cloneable { 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, @@ -108,8 +102,8 @@ std::make_shared(ConstString("thread")); m_collection_sp->Initialize(g_thread_properties); } else - m_collection_sp = std::make_shared( - Thread::GetGlobalProperties().get()); + m_collection_sp = + OptionValueProperties::CreateLocalCopy(*Thread::GetGlobalProperties()); } ThreadProperties::~ThreadProperties() = default; Index: lldb/test/API/commands/settings/TestSettings.py =================================================================== --- lldb/test/API/commands/settings/TestSettings.py +++ lldb/test/API/commands/settings/TestSettings.py @@ -238,6 +238,11 @@ 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"]) Index: lldb/unittests/Interpreter/CMakeLists.txt =================================================================== --- lldb/unittests/Interpreter/CMakeLists.txt +++ lldb/unittests/Interpreter/CMakeLists.txt @@ -1,6 +1,7 @@ add_lldb_unittest(InterpreterTests TestCompletion.cpp TestOptionArgParser.cpp + TestOptionValue.cpp TestOptionValueFileColonLine.cpp LINK_LIBS Index: lldb/unittests/Interpreter/TestOptionValue.cpp =================================================================== --- /dev/null +++ lldb/unittests/Interpreter/TestOptionValue.cpp @@ -0,0 +1,173 @@ +//===-- 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 CreateGlobal() { + auto props_sp = std::make_shared(); + const bool is_global = false; + + auto dict_sp = std::make_shared(1 << eTypeUInt64); + props_sp->AppendProperty(ConstString("dict"), ConstString(), is_global, + dict_sp); + + auto file_list_sp = std::make_shared(); + 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(*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(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); +}