Skip to content

Commit

Permalink
Bug 1847182 - Part 4: Throw on duplicates in PrepareTemporalFields. r…
Browse files Browse the repository at this point in the history
…=allstarschh

Implement the changes from <tc39/proposal-temporal#2570>.

Differential Revision: https://phabricator.services.mozilla.com/D185408
  • Loading branch information
anba committed Sep 13, 2023
1 parent c42e48d commit ccd16a2
Show file tree
Hide file tree
Showing 8 changed files with 236 additions and 135 deletions.
2 changes: 2 additions & 0 deletions js/public/friend/ErrorNumbers.msg
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,8 @@ MSG_DEF(JSMSG_TEMPORAL_MISSING_OPTION, 1, JSEXN_RANGEERR, "undefine
MSG_DEF(JSMSG_TEMPORAL_MISSING_PROPERTY, 1, JSEXN_TYPEERR, "{0} property is undefined")
MSG_DEF(JSMSG_TEMPORAL_UNEXPECTED_PROPERTY, 1, JSEXN_TYPEERR, "{0} property is not undefined")
MSG_DEF(JSMSG_TEMPORAL_MISSING_TEMPORAL_FIELDS, 0, JSEXN_TYPEERR, "object must have at least one temporal property")
MSG_DEF(JSMSG_TEMPORAL_DUPLICATE_PROPERTY, 1, JSEXN_RANGEERR, "duplicate property name \"{0}\"")
MSG_DEF(JSMSG_TEMPORAL_INVALID_PROPERTY, 1, JSEXN_RANGEERR, "invalid property name \"{0}\"")
MSG_DEF(JSMSG_TEMPORAL_INSTANT_INVALID, 0, JSEXN_RANGEERR, "epoch nanoseconds too large")
MSG_DEF(JSMSG_TEMPORAL_INSTANT_NONINTEGER, 1, JSEXN_RANGEERR, "Instant must be an integer, but received {0}")
MSG_DEF(JSMSG_TEMPORAL_INSTANT_BAD_DURATION, 1, JSEXN_RANGEERR, "duration \"{0}\" property must be zero")
Expand Down
11 changes: 0 additions & 11 deletions js/src/builtin/temporal/Calendar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1092,12 +1092,6 @@ bool js::temporal::CalendarFields(
}
}

// FIXME: spec issue - provide default implementation similar to
// DefaultMergeFields? Otherwise the input field names are returned as-is,
// including any duplicate field names. (Duplicate names are still possible
// through user-defined calendars, though.)
// https://github.com/tc39/proposal-temporal/issues/2532

auto* array = NewDenseFullyAllocatedArray(cx, fieldNames.size());
if (!array) {
return false;
Expand All @@ -1114,11 +1108,6 @@ bool js::temporal::CalendarFields(
return false;
}

// FIXME: spec issue - sort the result array here instead of in
// PrepareTemporalFields.
// FIXME: spec issue - maybe also check for duplicates here?
// https://github.com/tc39/proposal-temporal/issues/2532

// Steps 3-4.
if (!IterableToListOfStrings(cx, fieldsArray, result)) {
return false;
Expand Down
9 changes: 5 additions & 4 deletions js/src/builtin/temporal/PlainMonthDay.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -872,14 +872,15 @@ static bool PlainMonthDay_toPlainDate(JSContext* cx, const CallArgs& args) {
}

// Step 10.
JS::RootedVector<PropertyKey> mergedFieldNames(cx);
if (!MergeTemporalFieldNames(receiverFieldNames, inputFieldNames,
mergedFieldNames.get())) {
JS::RootedVector<PropertyKey> concatenatedFieldNames(cx);
if (!ConcatTemporalFieldNames(receiverFieldNames, inputFieldNames,
concatenatedFieldNames.get())) {
return false;
}

// Step 11.
mergedFields = PrepareTemporalFields(cx, mergedFields, mergedFieldNames);
mergedFields =
PrepareTemporalFields(cx, mergedFields, concatenatedFieldNames);
if (!mergedFields) {
return false;
}
Expand Down
9 changes: 5 additions & 4 deletions js/src/builtin/temporal/PlainYearMonth.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1421,14 +1421,15 @@ static bool PlainYearMonth_toPlainDate(JSContext* cx, const CallArgs& args) {
}

// Step 10.
JS::RootedVector<PropertyKey> mergedFieldNames(cx);
if (!MergeTemporalFieldNames(receiverFieldNames, inputFieldNames,
mergedFieldNames.get())) {
JS::RootedVector<PropertyKey> concatenatedFieldNames(cx);
if (!ConcatTemporalFieldNames(receiverFieldNames, inputFieldNames,
concatenatedFieldNames.get())) {
return false;
}

// Step 11.
mergedFields = PrepareTemporalFields(cx, mergedFields, mergedFieldNames);
mergedFields =
PrepareTemporalFields(cx, mergedFields, concatenatedFieldNames);
if (!mergedFields) {
return false;
}
Expand Down
Loading

0 comments on commit ccd16a2

Please sign in to comment.