LibJS+Everywhere: Return strings by value from PrimitiveString

It turns out return a ThrowCompletionOr<T const&> is flawed, as the GCC
expansion trick used with TRY will always make a copy. PrimitiveString
is luckily the only such use case.

9 files changed, 17 insertions, 16 deletions

diff --git a/Userland/Libraries/LibJS/Runtime/DatePrototype.cpp b/Userland/Libraries/LibJS/Runtime/DatePrototype.cpp
index fe854aea78..604fb9cac8 100644
--- a/Userland/Libraries/LibJS/Runtime/DatePrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/DatePrototype.cpp
@@ -1257,7 +1257,7 @@ JS_DEFINE_NATIVE_FUNCTION(DatePrototype::symbol_to_primitive)
     auto hint_value = vm.argument(0);
     if (!hint_value.is_string())
         return vm.throw_completion<TypeError>(ErrorType::InvalidHint, hint_value.to_string_without_side_effects());
-    auto const& hint = TRY(hint_value.as_string().deprecated_string());
+    auto hint = TRY(hint_value.as_string().deprecated_string());
     Value::PreferredType try_first;
     if (hint == "string" || hint == "default")
         try_first = Value::PreferredType::String;
diff --git a/Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp b/Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp
index 2ef7f05645..f7879d8491 100644
--- a/Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp
@@ -1604,7 +1604,7 @@ ThrowCompletionOr<MathematicalValue> to_intl_mathematical_value(VM& vm, Value va
 
     // 3. If Type(primValue) is String,
     // a.     Let str be primValue.
-    auto const& string = TRY(primitive_value.as_string().deprecated_string());
+    auto string = TRY(primitive_value.as_string().deprecated_string());
 
     // Step 4 handled separately by the FIXME above.
 
diff --git a/Userland/Libraries/LibJS/Runtime/PrimitiveString.cpp b/Userland/Libraries/LibJS/Runtime/PrimitiveString.cpp
index d962e2128a..bc310d1cc5 100644
--- a/Userland/Libraries/LibJS/Runtime/PrimitiveString.cpp
+++ b/Userland/Libraries/LibJS/Runtime/PrimitiveString.cpp
@@ -64,7 +64,7 @@ bool PrimitiveString::is_empty() const
     VERIFY_NOT_REACHED();
 }
 
-ThrowCompletionOr<DeprecatedString const&> PrimitiveString::deprecated_string() const
+ThrowCompletionOr<DeprecatedString> PrimitiveString::deprecated_string() const
 {
     TRY(resolve_rope_if_needed());
 
@@ -76,7 +76,7 @@ ThrowCompletionOr<DeprecatedString const&> PrimitiveString::deprecated_string()
     return *m_utf8_string;
 }
 
-ThrowCompletionOr<Utf16String const&> PrimitiveString::utf16_string() const
+ThrowCompletionOr<Utf16String> PrimitiveString::utf16_string() const
 {
     TRY(resolve_rope_if_needed());
 
@@ -90,7 +90,8 @@ ThrowCompletionOr<Utf16String const&> PrimitiveString::utf16_string() const
 
 ThrowCompletionOr<Utf16View> PrimitiveString::utf16_string_view() const
 {
-    return TRY(utf16_string()).view();
+    (void)TRY(utf16_string());
+    return m_utf16_string->view();
 }
 
 ThrowCompletionOr<Optional<Value>> PrimitiveString::get(VM& vm, PropertyKey const& property_key) const
@@ -178,8 +179,8 @@ ThrowCompletionOr<void> PrimitiveString::resolve_rope_if_needed() const
     // NOTE: Special case for two concatenated UTF-16 strings.
     //       This is here as an optimization, although I'm unsure how valuable it is.
     if (m_lhs->has_utf16_string() && m_rhs->has_utf16_string()) {
-        auto const& lhs_string = TRY(m_lhs->utf16_string());
-        auto const& rhs_string = TRY(m_rhs->utf16_string());
+        auto const& lhs_string = m_lhs->m_utf16_string.value();
+        auto const& rhs_string = m_rhs->m_utf16_string.value();
 
         Utf16Data combined;
         TRY_OR_THROW_OOM(vm, combined.try_ensure_capacity(lhs_string.length_in_code_units() + rhs_string.length_in_code_units()));
@@ -226,8 +227,8 @@ ThrowCompletionOr<void> PrimitiveString::resolve_rope_if_needed() const
         }
 
         // Get the UTF-8 representations for both strings.
-        auto const& previous_string_as_utf8 = TRY(previous->deprecated_string());
-        auto const& current_string_as_utf8 = TRY(current->deprecated_string());
+        auto previous_string_as_utf8 = TRY(previous->deprecated_string());
+        auto current_string_as_utf8 = TRY(current->deprecated_string());
 
         // NOTE: Now we need to look at the end of the previous string and the start
         //       of the current string, to see if they should be combined into a surrogate.
diff --git a/Userland/Libraries/LibJS/Runtime/PrimitiveString.h b/Userland/Libraries/LibJS/Runtime/PrimitiveString.h
index ea004323fa..d22bdae0cb 100644
--- a/Userland/Libraries/LibJS/Runtime/PrimitiveString.h
+++ b/Userland/Libraries/LibJS/Runtime/PrimitiveString.h
@@ -34,10 +34,10 @@ public:
     bool is_empty() const;
     u32 hash() const;
 
-    ThrowCompletionOr<DeprecatedString const&> deprecated_string() const;
+    ThrowCompletionOr<DeprecatedString> deprecated_string() const;
     bool has_utf8_string() const { return m_utf8_string.has_value(); }
 
-    ThrowCompletionOr<Utf16String const&> utf16_string() const;
+    ThrowCompletionOr<Utf16String> utf16_string() const;
     ThrowCompletionOr<Utf16View> utf16_string_view() const;
     bool has_utf16_string() const { return m_utf16_string.has_value(); }
 
diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp
index 5a0506e021..c69c947610 100644
--- a/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp
@@ -146,7 +146,7 @@ ThrowCompletionOr<Value> get_option(VM& vm, Object const& options, PropertyKey c
     if (!values.is_empty()) {
         // NOTE: Every location in the spec that invokes GetOption with type=boolean also has values=undefined.
         VERIFY(value.is_string());
-        if (auto const& value_string = TRY(value.as_string().deprecated_string()); !values.contains_slow(value_string))
+        if (auto value_string = TRY(value.as_string().deprecated_string()); !values.contains_slow(value_string))
             return vm.throw_completion<RangeError>(ErrorType::OptionIsNotValidValue, value_string, property.as_string());
     }
 
diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/Calendar.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/Calendar.cpp
index baa0dbcae2..3922263ae3 100644
--- a/Userland/Libraries/LibJS/Runtime/Temporal/Calendar.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Temporal/Calendar.cpp
@@ -779,7 +779,7 @@ ThrowCompletionOr<double> resolve_iso_month(VM& vm, Object const& fields)
 
     // 6. Assert: Type(monthCode) is String.
     VERIFY(month_code.is_string());
-    auto const& month_code_string = TRY(month_code.as_string().deprecated_string());
+    auto month_code_string = TRY(month_code.as_string().deprecated_string());
 
     // 7. If the length of monthCode is not 3, throw a RangeError exception.
     auto month_length = month_code_string.length();
diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp
index beaf9b76a3..886c058e32 100644
--- a/Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp
@@ -559,7 +559,7 @@ JS_DEFINE_NATIVE_FUNCTION(CalendarPrototype::fields)
             return TRY(iterator_close(vm, iterator_record, move(completion)));
         }
 
-        auto const& next_value_string = TRY(next_value.as_string().deprecated_string());
+        auto next_value_string = TRY(next_value.as_string().deprecated_string());
 
         // iii. If fieldNames contains nextValue, then
         if (field_names.contains_slow(next_value)) {
diff --git a/Userland/Libraries/LibJS/Runtime/Temporal/ZonedDateTimePrototype.cpp b/Userland/Libraries/LibJS/Runtime/Temporal/ZonedDateTimePrototype.cpp
index 8ecb6526ba..1d6325ab0a 100644
--- a/Userland/Libraries/LibJS/Runtime/Temporal/ZonedDateTimePrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/Temporal/ZonedDateTimePrototype.cpp
@@ -801,7 +801,7 @@ JS_DEFINE_NATIVE_FUNCTION(ZonedDateTimePrototype::with)
 
     // 18. Assert: Type(offsetString) is String.
     VERIFY(offset_string_value.is_string());
-    auto const& offset_string = TRY(offset_string_value.as_string().deprecated_string());
+    auto offset_string = TRY(offset_string_value.as_string().deprecated_string());
 
     // 19. Let dateTimeResult be ? InterpretTemporalDateTimeFields(calendar, fields, options).
     auto date_time_result = TRY(interpret_temporal_date_time_fields(vm, calendar, *fields, *options));
diff --git a/Userland/Libraries/LibWeb/WebAssembly/WebAssemblyTableConstructor.cpp b/Userland/Libraries/LibWeb/WebAssembly/WebAssemblyTableConstructor.cpp
index 23bc35df7b..15b50e9d65 100644
--- a/Userland/Libraries/LibWeb/WebAssembly/WebAssemblyTableConstructor.cpp
+++ b/Userland/Libraries/LibWeb/WebAssembly/WebAssemblyTableConstructor.cpp
@@ -35,7 +35,7 @@ JS::ThrowCompletionOr<JS::NonnullGCPtr<JS::Object>> WebAssemblyTableConstructor:
     auto element_value = TRY(descriptor->get("element"));
     if (!element_value.is_string())
         return vm.throw_completion<JS::TypeError>(JS::ErrorType::InvalidHint, element_value.to_string_without_side_effects());
-    auto const& element = TRY(element_value.as_string().deprecated_string());
+    auto element = TRY(element_value.as_string().deprecated_string());
 
     Optional<Wasm::ValueType> reference_type;
     if (element == "anyfunc"sv)