Skip to content

Commit a8839d6

Browse files
tkshsbcuetkshsbcue
committed
refactor(temporal): remove unnecessary clone in roundTo option parsing
Refactored the `round` method across 5 Temporal builtins (Duration, Instant, PlainDateTime, PlainTime, ZonedDateTime) to avoid an unnecessary JsVariant-to-JsValue conversion when calling `get_options_object`. Instead of matching on `JsValue::variant()` and reconstructing a JsValue in the fallback arm, the code now works directly with `&JsValue` via `args.get_or_undefined(0)`. This resolves the TODO comments requesting removal of the clone.
1 parent 2437b67 commit a8839d6

File tree

5 files changed

+116
-130
lines changed

5 files changed

+116
-130
lines changed

‎core/engine/src/builtins/temporal/duration/mod.rs‎

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -896,33 +896,30 @@ impl Duration {
896896
JsNativeError::typ().with_message("this value must be a Duration object.")
897897
})?;
898898

899-
let round_to = match args.first().map(JsValue::variant) {
900-
// 3. If roundTo is undefined, then
901-
None | Some(JsVariant::Undefined) => {
902-
return Err(JsNativeError::typ()
903-
.with_message("roundTo cannot be undefined.")
904-
.into());
905-
}
906-
// 4. If Type(roundTo) is String, then
907-
Some(JsVariant::String(rt)) => {
908-
// a. Let paramString be roundTo.
909-
let param_string = rt.clone();
910-
// b. Set roundTo to OrdinaryObjectCreate(null).
911-
let new_round_to = JsObject::with_null_proto();
912-
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
913-
new_round_to.create_data_property_or_throw(
914-
js_string!("smallestUnit"),
915-
param_string,
916-
context,
917-
)?;
918-
new_round_to
919-
}
899+
// 3. If roundTo is undefined, then
900+
let round_to_arg = args.get_or_undefined(0);
901+
if round_to_arg.is_undefined() {
902+
return Err(JsNativeError::typ()
903+
.with_message("roundTo cannot be undefined.")
904+
.into());
905+
}
906+
// 4. If Type(roundTo) is String, then
907+
let round_to = if let Some(param_string) = round_to_arg.as_string() {
908+
// a. Let paramString be roundTo.
909+
let param_string = param_string.clone();
910+
// b. Set roundTo to OrdinaryObjectCreate(null).
911+
let new_round_to = JsObject::with_null_proto();
912+
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
913+
new_round_to.create_data_property_or_throw(
914+
js_string!("smallestUnit"),
915+
param_string,
916+
context,
917+
)?;
918+
new_round_to
919+
} else {
920920
// 5. Else,
921-
Some(round_to) => {
922-
// TODO: remove this clone.
923-
// a. Set roundTo to ? GetOptionsObject(roundTo).
924-
get_options_object(&JsValue::from(round_to))?
925-
}
921+
// a. Set roundTo to ? GetOptionsObject(roundTo).
922+
get_options_object(round_to_arg)?
926923
};
927924

928925
// NOTE: 6 & 7 unused in favor of `is_none()`.

‎core/engine/src/builtins/temporal/instant/mod.rs‎

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
use super::options::{get_difference_settings, get_digits_option};
44
use super::{create_temporal_zoneddatetime, to_temporal_timezone_identifier};
55
use crate::js_error;
6-
use crate::value::JsVariant;
76
use crate::{
87
Context, JsArgs, JsBigInt, JsData, JsNativeError, JsObject, JsResult, JsString, JsSymbol,
98
JsValue,
@@ -495,33 +494,30 @@ impl Instant {
495494
JsNativeError::typ().with_message("the this object must be an instant object.")
496495
})?;
497496

498-
let round_to = match args.first().map(JsValue::variant) {
499-
// 3. If roundTo is undefined, then
500-
None | Some(JsVariant::Undefined) => {
501-
return Err(JsNativeError::typ()
502-
.with_message("roundTo cannot be undefined.")
503-
.into());
504-
}
505-
// 4. If Type(roundTo) is String, then
506-
Some(JsVariant::String(rt)) => {
507-
// a. Let paramString be roundTo.
508-
let param_string = rt.clone();
509-
// b. Set roundTo to OrdinaryObjectCreate(null).
510-
let new_round_to = JsObject::with_null_proto();
511-
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
512-
new_round_to.create_data_property_or_throw(
513-
js_string!("smallestUnit"),
514-
param_string,
515-
context,
516-
)?;
517-
new_round_to
518-
}
497+
// 3. If roundTo is undefined, then
498+
let round_to_arg = args.get_or_undefined(0);
499+
if round_to_arg.is_undefined() {
500+
return Err(JsNativeError::typ()
501+
.with_message("roundTo cannot be undefined.")
502+
.into());
503+
}
504+
// 4. If Type(roundTo) is String, then
505+
let round_to = if let Some(param_string) = round_to_arg.as_string() {
506+
// a. Let paramString be roundTo.
507+
let param_string = param_string.clone();
508+
// b. Set roundTo to OrdinaryObjectCreate(null).
509+
let new_round_to = JsObject::with_null_proto();
510+
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
511+
new_round_to.create_data_property_or_throw(
512+
js_string!("smallestUnit"),
513+
param_string,
514+
context,
515+
)?;
516+
new_round_to
517+
} else {
519518
// 5. Else,
520-
Some(round_to) => {
521-
// TODO: remove this clone.
522-
// a. Set roundTo to ? GetOptionsObject(roundTo).
523-
get_options_object(&JsValue::from(round_to))?
524-
}
519+
// a. Set roundTo to ? GetOptionsObject(roundTo).
520+
get_options_object(round_to_arg)?
525521
};
526522

527523
// 6. NOTE: The following steps read options and perform independent validation in

‎core/engine/src/builtins/temporal/plain_date_time/mod.rs‎

Lines changed: 23 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ use super::{
4242
options::{TemporalUnitGroup, get_difference_settings, get_digits_option, get_temporal_unit},
4343
to_temporal_duration_record, to_temporal_time, to_temporal_timezone_identifier,
4444
};
45-
use crate::value::JsVariant;
4645

4746
/// The `Temporal.PlainDateTime` built-in implementation.
4847
///
@@ -1323,32 +1322,30 @@ impl PlainDateTime {
13231322
JsNativeError::typ().with_message("the this object must be a PlainTime object.")
13241323
})?;
13251324

1326-
let round_to = match args.first().map(JsValue::variant) {
1327-
// 3. If roundTo is undefined, then
1328-
None | Some(JsVariant::Undefined) => {
1329-
return Err(JsNativeError::typ()
1330-
.with_message("roundTo cannot be undefined.")
1331-
.into());
1332-
}
1333-
// 4. If Type(roundTo) is String, then
1334-
Some(JsVariant::String(rt)) => {
1335-
// a. Let paramString be roundTo.
1336-
let param_string = rt.clone();
1337-
// b. Set roundTo to OrdinaryObjectCreate(null).
1338-
let new_round_to = JsObject::with_null_proto();
1339-
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
1340-
new_round_to.create_data_property_or_throw(
1341-
js_string!("smallestUnit"),
1342-
param_string,
1343-
context,
1344-
)?;
1345-
new_round_to
1346-
}
1325+
// 3. If roundTo is undefined, then
1326+
let round_to_arg = args.get_or_undefined(0);
1327+
if round_to_arg.is_undefined() {
1328+
return Err(JsNativeError::typ()
1329+
.with_message("roundTo cannot be undefined.")
1330+
.into());
1331+
}
1332+
// 4. If Type(roundTo) is String, then
1333+
let round_to = if let Some(param_string) = round_to_arg.as_string() {
1334+
// a. Let paramString be roundTo.
1335+
let param_string = param_string.clone();
1336+
// b. Set roundTo to OrdinaryObjectCreate(null).
1337+
let new_round_to = JsObject::with_null_proto();
1338+
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
1339+
new_round_to.create_data_property_or_throw(
1340+
js_string!("smallestUnit"),
1341+
param_string,
1342+
context,
1343+
)?;
1344+
new_round_to
1345+
} else {
13471346
// 5. Else,
1348-
Some(round_to) => {
1349-
// a. Set roundTo to ? GetOptionsObject(roundTo).
1350-
get_options_object(&JsValue::from(round_to))?
1351-
}
1347+
// a. Set roundTo to ? GetOptionsObject(roundTo).
1348+
get_options_object(round_to_arg)?
13521349
};
13531350

13541351
let mut options = RoundingOptions::default();

‎core/engine/src/builtins/temporal/plain_time/mod.rs‎

Lines changed: 23 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -642,32 +642,30 @@ impl PlainTime {
642642
JsNativeError::typ().with_message("the this object must be a PlainTime object.")
643643
})?;
644644

645-
let round_to = match args.first().map(JsValue::variant) {
646-
// 3. If roundTo is undefined, then
647-
None | Some(JsVariant::Undefined) => {
648-
return Err(JsNativeError::typ()
649-
.with_message("roundTo cannot be undefined.")
650-
.into());
651-
}
652-
// 4. If Type(roundTo) is String, then
653-
Some(JsVariant::String(rt)) => {
654-
// a. Let paramString be roundTo.
655-
let param_string = rt.clone();
656-
// b. Set roundTo to OrdinaryObjectCreate(null).
657-
let new_round_to = JsObject::with_null_proto();
658-
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
659-
new_round_to.create_data_property_or_throw(
660-
js_string!("smallestUnit"),
661-
param_string,
662-
context,
663-
)?;
664-
new_round_to
665-
}
645+
// 3. If roundTo is undefined, then
646+
let round_to_arg = args.get_or_undefined(0);
647+
if round_to_arg.is_undefined() {
648+
return Err(JsNativeError::typ()
649+
.with_message("roundTo cannot be undefined.")
650+
.into());
651+
}
652+
// 4. If Type(roundTo) is String, then
653+
let round_to = if let Some(param_string) = round_to_arg.as_string() {
654+
// a. Let paramString be roundTo.
655+
let param_string = param_string.clone();
656+
// b. Set roundTo to OrdinaryObjectCreate(null).
657+
let new_round_to = JsObject::with_null_proto();
658+
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
659+
new_round_to.create_data_property_or_throw(
660+
js_string!("smallestUnit"),
661+
param_string,
662+
context,
663+
)?;
664+
new_round_to
665+
} else {
666666
// 5. Else,
667-
Some(round_to) => {
668-
// a. Set roundTo to ? GetOptionsObject(roundTo).
669-
get_options_object(&JsValue::from(round_to))?
670-
}
667+
// a. Set roundTo to ? GetOptionsObject(roundTo).
668+
get_options_object(round_to_arg)?
671669
};
672670

673671
let mut options = RoundingOptions::default();

‎core/engine/src/builtins/temporal/zoneddatetime/mod.rs‎

Lines changed: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,33 +1469,31 @@ impl ZonedDateTime {
14691469
JsNativeError::typ().with_message("the this object must be a ZonedDateTime object.")
14701470
})?;
14711471

1472-
let round_to = match args.first().map(JsValue::variant) {
1473-
// 3. If roundTo is undefined, then
1474-
None | Some(JsVariant::Undefined) => {
1475-
// a. Throw a TypeError exception.
1476-
return Err(JsNativeError::typ()
1477-
.with_message("roundTo cannot be undefined.")
1478-
.into());
1479-
}
1480-
// 4. If Type(roundTo) is String, then
1481-
Some(JsVariant::String(rt)) => {
1482-
// a. Let paramString be roundTo.
1483-
let param_string = rt.clone();
1484-
// b. Set roundTo to OrdinaryObjectCreate(null).
1485-
let new_round_to = JsObject::with_null_proto();
1486-
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
1487-
new_round_to.create_data_property_or_throw(
1488-
js_string!("smallestUnit"),
1489-
param_string,
1490-
context,
1491-
)?;
1492-
new_round_to
1493-
}
1472+
// 3. If roundTo is undefined, then
1473+
let round_to_arg = args.get_or_undefined(0);
1474+
if round_to_arg.is_undefined() {
1475+
// a. Throw a TypeError exception.
1476+
return Err(JsNativeError::typ()
1477+
.with_message("roundTo cannot be undefined.")
1478+
.into());
1479+
}
1480+
// 4. If Type(roundTo) is String, then
1481+
let round_to = if let Some(param_string) = round_to_arg.as_string() {
1482+
// a. Let paramString be roundTo.
1483+
let param_string = param_string.clone();
1484+
// b. Set roundTo to OrdinaryObjectCreate(null).
1485+
let new_round_to = JsObject::with_null_proto();
1486+
// c. Perform ! CreateDataPropertyOrThrow(roundTo, "smallestUnit", paramString).
1487+
new_round_to.create_data_property_or_throw(
1488+
js_string!("smallestUnit"),
1489+
param_string,
1490+
context,
1491+
)?;
1492+
new_round_to
1493+
} else {
14941494
// 5. Else,
1495-
Some(round_to) => {
1496-
// a. Set roundTo to ? GetOptionsObject(roundTo).
1497-
get_options_object(&JsValue::from(round_to))?
1498-
}
1495+
// a. Set roundTo to ? GetOptionsObject(roundTo).
1496+
get_options_object(round_to_arg)?
14991497
};
15001498

15011499
// 6. NOTE: The following steps read options and perform independent validation

0 commit comments

Comments
 (0)