system/serenity
system/serenity
1
0
Fork 0
mirror of https://github.com/SerenityOS/serenity synced 2024-10-15 20:33:10 +00:00
Code Issues Packages Projects Releases Wiki Activity

LibJS+Everywhere: Return strings by value from PrimitiveString

Browse source 
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.
This commit is contained in:
Timothy Flynn 2023-01-13 15:33:31 -05:00 committed by Tim Flynn
parent 9a120d7243
commit a59ebdac2d
10 changed files with 22 additions and 21 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
Userland/Applications/Spreadsheet/JSIntegration.cpp
View file

@ -234,7 +234,7 @@ JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::set_real_cell_contents)
return vm.throw_completion<JS::TypeError>("Expected the second argument of set_real_cell_contents() to be a String");
auto& cell = sheet_object->m_sheet.ensure(position.value());
auto const& new_contents = TRY(new_contents_value.as_string().deprecated_string());
auto new_contents = TRY(new_contents_value.as_string().deprecated_string());
cell.set_data(new_contents);
return JS::js_null();
}
@ -301,7 +301,7 @@ JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::column_index)
if (!column_name.is_string())
return vm.throw_completion<JS::TypeError>(JS::ErrorType::NotAnObjectOfType, "String");
auto const& column_name_str = TRY(column_name.as_string().deprecated_string());
auto column_name_str = TRY(column_name.as_string().deprecated_string());
auto* this_object = TRY(vm.this_value().to_object(vm));
@ -326,7 +326,7 @@ JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::column_arithmetic)
if (!column_name.is_string())
return vm.throw_completion<JS::TypeError>(JS::ErrorType::NotAnObjectOfType, "String");
auto const& column_name_str = TRY(column_name.as_string().deprecated_string());
auto column_name_str = TRY(column_name.as_string().deprecated_string());
auto offset = TRY(vm.argument(1).to_number(vm));
auto offset_number = static_cast<i32>(offset.as_double());
@ -354,7 +354,7 @@ JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::get_column_bound)
if (!column_name.is_string())
return vm.throw_completion<JS::TypeError>(JS::ErrorType::NotAnObjectOfType, "String");
auto const& column_name_str = TRY(column_name.as_string().deprecated_string());
auto column_name_str = TRY(column_name.as_string().deprecated_string());
auto* this_object = TRY(vm.this_value().to_object(vm));
if (!is<SheetGlobalObject>(this_object))
@ -405,7 +405,7 @@ JS_DEFINE_NATIVE_FUNCTION(WorkbookObject::sheet)
auto& workbook = static_cast<WorkbookObject*>(this_object)->m_workbook;
if (name_value.is_string()) {
auto const& name = TRY(name_value.as_string().deprecated_string());
auto name = TRY(name_value.as_string().deprecated_string());
for (auto& sheet : workbook.sheets()) {
if (sheet.name() == name)
return JS::Value(&sheet.global_object());

2
Userland/Libraries/LibJS/Runtime/DatePrototype.cpp
View file

@ -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;

2
Userland/Libraries/LibJS/Runtime/Intl/NumberFormat.cpp
View file

@ -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.

15
Userland/Libraries/LibJS/Runtime/PrimitiveString.cpp
View file

@ -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.

4
Userland/Libraries/LibJS/Runtime/PrimitiveString.h
View file

@ -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(); }

2
Userland/Libraries/LibJS/Runtime/Temporal/AbstractOperations.cpp
View file

@ -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());
}

2
Userland/Libraries/LibJS/Runtime/Temporal/Calendar.cpp
View file

@ -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();

2
Userland/Libraries/LibJS/Runtime/Temporal/CalendarPrototype.cpp
View file

@ -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)) {

2
Userland/Libraries/LibJS/Runtime/Temporal/ZonedDateTimePrototype.cpp
View file

@ -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));

2
Userland/Libraries/LibWeb/WebAssembly/WebAssemblyTableConstructor.cpp
View file

@ -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)