diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp b/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp index 59e21a7d8b..72eb7ebadd 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomValue.cpp @@ -61,7 +61,7 @@ namespace AZ::Dom return m_children; } - Value::Value(AZStd::shared_ptr string) + Value::Value(AZStd::shared_ptr string) : m_value(string) { } @@ -233,7 +233,7 @@ namespace AZ::Dom case 4: // bool return AZStd::get(m_value) ? Type::True : Type::False; case 5: // AZStd::string_view - case 6: // AZStd::shared_ptr + case 6: // AZStd::shared_ptr case 7: // ShortStringType return Type::String; case 8: // ObjectPtr @@ -900,7 +900,7 @@ namespace AZ::Dom m_value = aznumeric_cast(value); } - void Value::SetString(AZStd::shared_ptr string) + void Value::SetString(AZStd::shared_ptr string) { m_value = string; } @@ -911,8 +911,8 @@ namespace AZ::Dom { case 5: // AZStd::string_view return AZStd::get(m_value); - case 6: // AZStd::shared_ptr - return *AZStd::get>(m_value); + case 6: // AZStd::shared_ptr + return *AZStd::get>(m_value); case 7: // ShortStringType { const ShortStringType& ShortString = AZStd::get(m_value); @@ -948,7 +948,7 @@ namespace AZ::Dom } else { - m_value = AZStd::allocate_shared(AZStdAlloc(), value); + m_value = AZStd::allocate_shared(AZStdAlloc(), value); } } @@ -1000,7 +1000,7 @@ namespace AZ::Dom { result = visitor.String(arg, copyStrings ? Lifetime::Temporary : Lifetime::Persistent); } - else if constexpr (AZStd::is_same_v>) + else if constexpr (AZStd::is_same_v>) { result = visitor.RefCountedString(arg, copyStrings ? Lifetime::Temporary : Lifetime::Persistent); } @@ -1096,7 +1096,7 @@ namespace AZ::Dom if (IsString() && other.IsString()) { // If we both hold the same ref counted string we don't need to do a full comparison - if (AZStd::holds_alternative>(m_value) && m_value == other.m_value) + if (AZStd::holds_alternative>(m_value) && m_value == other.m_value) { return true; } diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValue.h b/Code/Framework/AzCore/AzCore/DOM/DomValue.h index 4b09068ec4..1a09b9a3c8 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValue.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomValue.h @@ -39,8 +39,7 @@ namespace AZ::Dom }; //! The allocator used by Value. - //! Value heap allocates shared_ptrs for its container storage (Array / Object / Node) alongside the vector - //! contents of its container storage. + //! Value heap allocates shared_ptrs for its container storage (Array / Object / Node) alongside class ValueAllocator final : public SimpleSchemaAllocator { public: @@ -131,7 +130,7 @@ namespace AZ::Dom Value(const Value&); Value(Value&&) noexcept; Value(AZStd::string_view string, bool copy); - Value(AZStd::shared_ptr string); + Value(AZStd::shared_ptr string); Value(int32_t value); Value(uint32_t value); @@ -242,10 +241,6 @@ namespace AZ::Dom const Array::ContainerType& GetArray() const; // Node API (supports both object + array API, plus a dedicated NodeName)... - // bool CanConvertToNodeFromObject() const; - // Value& ConvertToNodeFromObject(); - // Value& ConvertToObjectFromNode(); - void SetNode(AZ::Name name); void SetNode(AZStd::string_view name); @@ -282,14 +277,14 @@ namespace AZ::Dom float GetFloat() const; void SetFloat(float); - // string API... + // String API... AZStd::string_view GetString() const; size_t GetStringLength() const; void SetString(AZStd::string_view); - void SetString(AZStd::shared_ptr); + void SetString(AZStd::shared_ptr); void CopyFromString(AZStd::string_view); - // opaque type API... + // Opaque type API... AZStd::any& GetOpaqueValue() const; //! This sets this Value to represent a value of an type that the DOM has //! no formal knowledge of. Where possible, it should be preferred to @@ -298,10 +293,10 @@ namespace AZ::Dom //! values. void SetOpaqueValue(AZStd::any&); - // null API... + // Null API... void SetNull(); - // Visitor API + // Visitor API... Visitor::Result Accept(Visitor& visitor, bool copyStrings) const; AZStd::unique_ptr GetWriteHandler(); @@ -330,12 +325,9 @@ namespace AZ::Dom } }; - // If using the the copy on write model, anything stored internally as a shared_ptr will - // detach and copy when doing a mutating operation if use_count() > 1. - - // This internal storage will not have a 1:1 mapping to the public Type, as there may be - // multiple storage options (e.g. strings being stored as non-owning string_view or - // owning shared_ptr) + //! The internal storage type for Value. + //! These types do not correspond one-to-one with the Value's external Type as there may be multiple storage classes + //! for the same type in some instances, such as string storage using ValueType = AZStd::variant< // NullType AZStd::monostate, @@ -347,7 +339,7 @@ namespace AZ::Dom bool, // StringType AZStd::string_view, - AZStd::shared_ptr, + AZStd::shared_ptr, ShortStringType, // ObjectType ObjectPtr, diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp index 3e48c097c8..ef99809290 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.cpp @@ -68,7 +68,7 @@ namespace AZ::Dom return FinishWrite(); } - Visitor::Result ValueWriter::RefCountedString(AZStd::shared_ptr value, [[maybe_unused]] Lifetime lifetime) + Visitor::Result ValueWriter::RefCountedString(AZStd::shared_ptr value, [[maybe_unused]] Lifetime lifetime) { CurrentValue().SetString(value); return FinishWrite(); diff --git a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h index 4c1d5a26ee..6086ff4fe5 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomValueWriter.h @@ -26,7 +26,7 @@ namespace AZ::Dom Result Double(double value) override; Result String(AZStd::string_view value, Lifetime lifetime) override; - Result RefCountedString(AZStd::shared_ptr value, Lifetime lifetime) override; + Result RefCountedString(AZStd::shared_ptr value, Lifetime lifetime) override; Result StartObject() override; Result EndObject(AZ::u64 attributeCount) override; Result Key(AZ::Name key) override; diff --git a/Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp b/Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp index af64c4cea8..5e3d6c2882 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp +++ b/Code/Framework/AzCore/AzCore/DOM/DomVisitor.cpp @@ -105,7 +105,7 @@ namespace AZ::Dom return VisitorSuccess(); } - Visitor::Result Visitor::RefCountedString(AZStd::shared_ptr value, Lifetime lifetime) + Visitor::Result Visitor::RefCountedString(AZStd::shared_ptr value, Lifetime lifetime) { return String(*value, lifetime); } diff --git a/Code/Framework/AzCore/AzCore/DOM/DomVisitor.h b/Code/Framework/AzCore/AzCore/DOM/DomVisitor.h index 3439a566e2..938df1bb5a 100644 --- a/Code/Framework/AzCore/AzCore/DOM/DomVisitor.h +++ b/Code/Framework/AzCore/AzCore/DOM/DomVisitor.h @@ -168,10 +168,15 @@ namespace AZ::Dom virtual Result Uint64(AZ::u64 value); //! Operates on a double precision, 64 bit floating point value. virtual Result Double(double value); - //! Operates on a string value. As strings are a reference type. - //! Storage semantics are provided to indicate where the value may be stored persistently or requires a copy. + //! Operates on a string value. As strings are a reference type, + //! storage semantics are provided to indicate where the value may be stored persistently or requires a copy. + //! \param lifetime Specifies the lifetime of this string - if the string has a temporary lifetime, it cannot + //! safely be stored as a reference. virtual Result String(AZStd::string_view value, Lifetime lifetime); - virtual Result RefCountedString(AZStd::shared_ptr value, Lifetime lifetime); + //! Operates on a ref-counted string value. S + //! \param lifetime Specifies the lifetime of this string. If the string has a temporary lifetime, it may not + //! be safely stored as a reference, but may still be safely stored as a ref-counted shared_ptr. + virtual Result RefCountedString(AZStd::shared_ptr value, Lifetime lifetime); //! Operates on an opaque value. As opaque values are a reference type, storage semantics are provided to //! indicate where the value may be stored persistently or requires a copy. //! The base implementation of OpaqueValue rejects the operation, as opaque values are meant for special