Implement the Temporal proposal
Categories
(Core :: JavaScript Engine, enhancement, P3)
Tracking
()
People
(Reporter: alex.fdm, Assigned: anba)
References
(Blocks 2 open bugs, )
Details
(Keywords: dev-doc-needed)
Attachments
(106 files)
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
Bug 1519167 - Part 66: Implement ZonedDateTime.p.with{Calendar,TimeZone}. r=#spidermonkey-reviewers!
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review | |
48 bytes,
text/x-phabricator-request
|
Details | Review |
The proposal is currently in Stage 2.
Updated•5 years ago
|
Updated•4 years ago
|
Assignee | ||
Comment 1•2 years ago
|
||
Taking for now because I've already written some code.
Comment 3•2 years ago
|
||
This is currently blocked from shipping unflagged on IETF Standardization work: https://github.com/tc39/proposal-temporal/issues/1450
Assignee | ||
Comment 4•2 years ago
|
||
FWIW, I have a local prototype which includes everything from the current proposal apart from https://tc39.es/proposal-temporal/#sec-temporal-legacy-date-objects and https://tc39.es/proposal-temporal/#sec-temporal-intl. Unsurprisingly a proposal as large as Temporal
has numerous issues, I've already filed some at https://github.com/tc39/proposal-temporal/issues/1502, but in the meantime I've found more issues. (There are more than 350 FIXME and TODO annotations referring to spec issues in that prototype.) My current plan is to wait until the spec authors have addressed this feedback before I'll upload the patches for review, because I don't think it makes sense to spend time reviewing code which is still so volatile. And waiting a bit longer gives me additional time to clean-up the code, add comments, etc. :-)
Assignee | ||
Comment 5•2 years ago
|
||
In its current form, without any optimisations or overly large comments except spec-step comments, the number of lines already exceeds "builtin/intl", so reviewers will have their, err..., fun! :-)
~/hg/mozilla-inbound$ LANG=C wc -l js/src/builtin/temporal/*
2819 js/src/builtin/temporal/Calendar.cpp
149 js/src/builtin/temporal/Calendar.h
5500 js/src/builtin/temporal/Duration.cpp
107 js/src/builtin/temporal/Duration.h
2006 js/src/builtin/temporal/Instant.cpp
80 js/src/builtin/temporal/Instant.h
35 js/src/builtin/temporal/moz.build
2419 js/src/builtin/temporal/PlainDate.cpp
85 js/src/builtin/temporal/PlainDate.h
2940 js/src/builtin/temporal/PlainDateTime.cpp
110 js/src/builtin/temporal/PlainDateTime.h
1174 js/src/builtin/temporal/PlainMonthDay.cpp
58 js/src/builtin/temporal/PlainMonthDay.h
2277 js/src/builtin/temporal/PlainTime.cpp
84 js/src/builtin/temporal/PlainTime.h
1943 js/src/builtin/temporal/PlainYearMonth.cpp
59 js/src/builtin/temporal/PlainYearMonth.h
2558 js/src/builtin/temporal/Temporal.cpp
334 js/src/builtin/temporal/Temporal.h
494 js/src/builtin/temporal/TemporalNow.cpp
31 js/src/builtin/temporal/TemporalNow.h
2584 js/src/builtin/temporal/TemporalParser.cpp
61 js/src/builtin/temporal/TemporalParser.h
76 js/src/builtin/temporal/TemporalTypes.h
67 js/src/builtin/temporal/TemporalTypes-inl.h
1912 js/src/builtin/temporal/TimeZone.cpp
96 js/src/builtin/temporal/TimeZone.h
22 js/src/builtin/temporal/Wrapped.cpp
183 js/src/builtin/temporal/Wrapped.h
3902 js/src/builtin/temporal/ZonedDateTime.cpp
131 js/src/builtin/temporal/ZonedDateTime.h
34296 total
Comment 6•2 years ago
|
||
Wow, amazing stuff. Can't wait to review ;) maybe we can have a couple of us do it..
Assignee | ||
Comment 7•2 years ago
|
||
Assignee | ||
Comment 8•2 years ago
|
||
Depends on D124224
Assignee | ||
Comment 9•2 years ago
|
||
Depends on D124225
Assignee | ||
Comment 10•2 years ago
|
||
Depends on D124226
Assignee | ||
Comment 11•2 years ago
|
||
This is the current prototype, not yet ready for review. There is currently one large patch which covers the whole proposal, for review it will be split into smaller, reviewable parts. The --with-temporal-api
flag must be set in the "mozconfig" resp. passed to configure
to enable the Temporal code.
Assignee | ||
Comment 12•11 months ago
|
||
Temporal.TimeZone
needs these additional methods.
Updated•11 months ago
|
Updated•11 months ago
|
Updated•11 months ago
|
Assignee | ||
Comment 13•11 months ago
|
||
Depends on D124226
Updated•11 months ago
|
Assignee | ||
Comment 14•11 months ago
|
||
Depends on D124227
Updated•8 months ago
|
Assignee | ||
Comment 15•2 months ago
•
|
||
Notes for reviewers:
The implementation is based on the current spec text (at commit 84d1b184ac200a81a6648f2d148082f69ae0547b). There are a couple of major spec refactorings planned, so at some point the implementation will no longer match the rendered spec at https://tc39.es/proposal-temporal/. When that happens, you either have to build the rendered spec locally or use the Wayback Machine copy (https://web.archive.org/web/20230316163651/https://tc39.es/proposal-temporal).
The current implementation supports different time zones, but not different calendars, because we still plan to implement support for calendars other than the ISO-8601 calendar through ICU4X. So most changes from https://tc39.es/proposal-temporal/#sec-temporal-intl aren't yet implemented.
I'll upload the patches in small, reviewable parts, because I don't want to force anyone to review the 8000 lines for Temporal.Duration
all at once. (Temporal.Duration
is currently by far the largest file, the next largest is Temporal.Calendar
, which has only 4000 lines.) When reviewing these small patches, you may notice that:
- Some functions are declared in a header, even though they're currently only used in a single cpp file, so normally they should be declared as
static
in the cpp file. This is most likely a sign that in a later patch the function will be called from other cpp files, too. I could have declared the function asstatic
and only later moved the declaration to the header, but that would require extra work for me when I create the review patches. - Similarly, sometimes there are excess
#include
statements. I used IWYU (include-what-you-use) during development, so the includes are (likely) correct for the final version, but when splitting the files into reviewable parts, I don't want to run IWYU for each individual patch I upload. - I'll try to group similar functions into a single patch, so the same kind of function doesn't need to be reviewed multiple times. This can lead to adding apparently unused function. Later patches will eventually add a caller for those functions.
- Some optimisations are already implemented. This may seem strange for an initial implementation, which should normally strive to only implement the spec correctly, but leave optimisations for later. But the spec authors like to know if the various abstractions used in
Temporal
can actually be implemented efficiently. - Rooted/Handle uses avoid the typedef versions, see bug 1773366. For example
Handle<JSObject*>
instead ofHandleObject
. - Header files try to avoid using "NamespaceImports.h" typedefs, for example
JS::Value
instead ofValue
. (bug 1773721)
The review patches are generated from the implementation which is already uploaded at https://phabricator.services.mozilla.com/D124226.
When during review there are any change requests for (mostly) stylistic issues, I'd like to fix those issues after the complete patches have been reviewed, because the same kind of stylistic issues will probably appear in other patches, too. And it probably makes sense to handle all code style issues at once.
Assignee | ||
Comment 16•2 months ago
|
||
Add a new configure option "--with-temporal-api" to enable the Temporal API.
This option is off by default, because the Temporal spec is still work-in-progress.
Time zone and calendrical operations will require ICU, so we don't support
Temporal when "--without-intl-api" is specified.
Assignee | ||
Comment 17•2 months ago
|
||
Add the source files for all Temporal types, using a blank class definition.
Depends on D174153
Assignee | ||
Comment 18•2 months ago
|
||
Add internal representations for Temporal types.
Instant
:
Represents a time since the epoch value, measured in nanoseconds. The number of
epoch nanoseconds exceeds int64_t
storage, therefore it's split into a full
seconds and fractional seconds part. This representation is used so we don't
have to allocate BigInt values for any Instant value computation.
PlainDate
, PlainTime
, and PlainDateTime
:
Representation for date-time values. The spec often passes each individual
date-time value separately, so some spec operations use nine or more
parameters. We pack date-time values into struct
s to avoid having functions
with excess parameters.
Duration
:
Representation of a duration amount. Similar to date-time values, using a
struct
avoids writing functions with excess parameters.
Also add Temporal unit enum
with some helpers for conversion between types.
Depends on D174154
Assignee | ||
Comment 19•2 months ago
|
||
Temporal supports nine different rounding modes. This adds the implementation
for int64_t
rounding, later patches will add similar functions for BigInt
values.
Depends on D174155
Assignee | ||
Comment 20•2 months ago
|
||
Add functions to convert Temporal objects to their internal representation.
Depends on D174156
Assignee | ||
Comment 21•2 months ago
|
||
Abstraction to work with possibly wrapped objects in a type-safe way. Strictly
speaking the class name should be "PossiblyWrapped", but that's a bit too long,
so it was shortened to just "Wrapped".
Useful when the Temporal spec uses different code paths for built-in objects.
For example when Wrapped<DurationObject*>
to represent a possibly wrapped
DurationObject*
. Alternatives currently used in SpiderMonkey code are:
- Unwrapped
DurationObject*
with a scary name to document that the object is
possibly from a different compartment.
For exampleunsafeTypedArrayCrossCompartment
andDangerouslyUnwrapTypedArray
in SelfHosting.cpp. JSObject*
with a descriptive name to document the object is aDurationObject*
.
For example: Callers toUnwrapAndDowncastObject
or similar functions.
Wrapped<DurationObject*>
avoids passing unwrapped objects around, so we can
more easily avoid compartment mismatches. And because it includes the concrete
JSObject
subclass as part of its type, it's more easy to see which actual
JSObject
class is used. And we can also use the C++ type system more
effectively.
For example:
void consumer(JSObject* maybeWrappedInstant);
// Type only part of the variable name.
JSObject* maybeWrappedDuration = producer();
// Compiles, but will crash at runtime.
consumer(maybeWrappedDuration);
Compare to:
void consumer(Wrapped<InstantObject*> maybeWrappedInstant);
// Type part of the variable type.
Wrapped<DurationObject*> maybeWrappedDuration = producer();
// Doesn't even compile.
consumer(maybeWrappedDuration);
There's one downside of this approach: This is yet another set of tools to work
with possibly wrapped objects, in addition to the already existing functions:
JSObject::{canUnwrapAs, unwrapAs, maybeUnwrapAs, maybeUnwrapIf}()
UnwrapAndDowncastObject
and friends- Direct calls to either
UncheckedUnwrap
orCheckedUnwrapStatic
- etc.
Depends on D174157
Assignee | ||
Comment 22•2 months ago
|
||
Adds the following functions:
js::temporal::IsValidEpochNanoseconds
:
- Implementation of the
IsValidEpochNanoseconds
spec abstract operation. - Implemented for
Instant
andBigInt
values. - The naive implementation to compare a
BigInt
against a constantBigInt
requires to allocate the constant, which is (a) slow and (b) makes the
operation fallible when the allocation fails. Instead implement it through
direct comparison of the BigInt digits.
js::temporal::IsValidInstantDifference
:
- Functions to validate an
Instant
orBigInt
value doesn't exceed the
maximum difference between two valid epoch nanoseconds values.
js::temporal::ToInstant
and js::temporal::ToInstantDifference
:
- Create an
Instant
value from aBigInt
. - More
BigInt::Digit
surgery to make the operation infallible and performant.
js::temporal::ToEpochNanoseconds
and js::temporal::ToEpochDifferenceNanoseconds
:
- Create a
BigInt
from anInstant
value. - Last
BigInt::Digit
fun until the implementation of theAddInstant
abstract operation.
Depends on D174158
Assignee | ||
Comment 23•2 months ago
|
||
The Temporal.Instant
constructor and some Temporal.Instant
methods are
ready to be implemented.
Depends on D174159
Assignee | ||
Comment 24•2 months ago
|
||
Depends on D174160
Assignee | ||
Comment 25•2 months ago
|
||
Depends on D174161
Assignee | ||
Comment 26•2 months ago
|
||
Most functions are present two times: The version which takes double
inputs is
called for user-inputs (double
for the JS Number
type). The version which
takes PlainDate
, PlainTime
, or PlainDateTime
is used for internally created
objects.
Calendar.cpp:
- The
static
ISODaysInMonth
isconstexpr
in preparation for a later patch.
Depends on D174162
Assignee | ||
Comment 27•2 months ago
|
||
The Temporal API supports parsing strings into various Temporal types. We
start with parsing calendar strings, because that's a prerequisite for other
Temporal
constructors like Temporal.PlainDate
.
The parser is split into two classes:
StringReader
provides access to the underlying characters. The
characters are wrapped into amozilla::Span
for extra safety, any
out-of-bounds access will crash.TemporalParser
is the actual parser implementation. The interface is
Rust-like where all fallible methods return amozilla::Result
object.
Parse errors are stored inParserError
, which is a simple wrapper around
JSErrNum
.ParserError
implementsmozilla::detail::UnusedZero
to ensure
efficient packing in themozilla::Result
object.
Depends on D174267
Assignee | ||
Comment 28•2 months ago
|
||
Implements parsing the TemporalDateTimeString
parse goal.
Depends on D174268
Assignee | ||
Comment 29•2 months ago
|
||
Implements parsing the TemporalInstantString
parse goal.
Depends on D174269
Assignee | ||
Comment 30•2 months ago
|
||
Implements parsing the TemporalTimeString
parse goal.
Depends on D174270
Assignee | ||
Comment 31•2 months ago
|
||
Implements parsing the TemporalZonedDateTimeString
parse goal.
Depends on D174271
Assignee | ||
Comment 32•2 months ago
|
||
Implements parsing the TemporalMonthDayString
parse goal.
Depends on D174272
Assignee | ||
Comment 33•2 months ago
|
||
Implements parsing the TemporalYearMonthString
parse goal.
Depends on D174273
Assignee | ||
Comment 34•2 months ago
|
||
Implements the following abstract operations:
- GetISO8601Calendar
- ToTemporalCalendar
- ToTemporalCalendarWithISODefault
- GetTemporalCalendarWithISODefault
Note: The PlainTimeObject::getOrCreateCalendar()
is used to lazily create the
calendar object of a Temporal.PlainTime
object. This method will likely be
removed when the spec is changed to represent built-in calendars as strings.
Depends on D174274
Assignee | ||
Comment 35•2 months ago
|
||
Implements constructors for Temporal.Plain{Date,DateTime,Time,MonthDay,YearMonth}
.
Additionally implements the CreateTemporalX
operations for later patches.
Depends on D174275
Assignee | ||
Comment 36•2 months ago
|
||
This actually just ports the canonicalisation code from builtin/intl/DateTimeFormat.js
to C++. The self-hosted code can be removed after Temporal is enabled by default.
Depends on D174276
Assignee | ||
Comment 37•2 months ago
|
||
Implement the Temporal.TimeZone
constructor and the corresponding
CreateTemporalTimeZone abstract operations.
Depends on D174277
Assignee | ||
Comment 38•2 months ago
|
||
Implements the ToTemporalTimeZone
abstract operation, which is needed to
implement the Temporal.ZonedDateTime
constructor.
Depends on D174278
Assignee | ||
Comment 39•2 months ago
|
||
Implement the Temporal.ZonedDateTime
constructor and the corresponding
CreateTemporalZonedDateTime abstract operation.
Depends on D174279
Assignee | ||
Comment 40•2 months ago
|
||
Implement the various ISO-8601 calendar methods and abstract operations.
Depends on D174280
Assignee | ||
Comment 41•2 months ago
|
||
Depends on D174282
Assignee | ||
Comment 42•2 months ago
|
||
These three functions are now trivial to implement after the last part added ToTemporalInstant
.
Depends on D174283
Assignee | ||
Comment 43•2 months ago
|
||
Depends on D174284
Assignee | ||
Comment 44•2 months ago
|
||
And additionally implement toJSON
, id
getter, and @@toString
tag.
Depends on D174285
Assignee | ||
Comment 45•2 months ago
|
||
The GetNamedTimeZoneEpochNanoseconds
, GetNamedTimeZoneNextTransition
, and
GetNamedTimeZonePreviousTransition
abstract operations from the Temporal API
proposal require these additional methods.
Depends on D174286
Assignee | ||
Comment 46•2 months ago
|
||
Depends on D174287
Assignee | ||
Comment 47•2 months ago
|
||
Depends on D174288
Assignee | ||
Comment 48•2 months ago
|
||
Depends on D174289
Assignee | ||
Comment 49•2 months ago
|
||
Depends on D174393
Assignee | ||
Comment 50•2 months ago
|
||
Also implement the "CalendarMergeFields" abstract operation with a fast-path
when the built-in Temporal.Calendar.prototype.mergeFields
is called.
The "CalendarFields" abstract operation needs some additional setup, so it'll
be implemented later.
Depends on D174394
Assignee | ||
Comment 51•2 months ago
|
||
Depends on D174395
Assignee | ||
Comment 52•2 months ago
|
||
Including a fast-path when the built-in Temporal.Calendar.prototype.fields
function is called.
Depends on D174396
Assignee | ||
Comment 53•2 months ago
|
||
Later patches will add more PrepareTemporalFields
implementations, suitable
for each different call site.
Depends on D174397
Assignee | ||
Comment 54•2 months ago
|
||
Implement Temporal.Calendar.prototype.dateFromFields
and the corresponding
"CalendarDateFromFields" abstract operation.
Depends on D174398
Assignee | ||
Comment 55•2 months ago
|
||
And implement the corresponding "CalendarYearMonthFromFields" and
"CalendarMonthDayFromFields" abstract operations.
Depends on D174399
Assignee | ||
Comment 56•2 months ago
|
||
Including any abstract operation which is called from "GetPlainDateTimeFor".
BalanceISODateNew
implements the "BalanceISODate" abstract operation according
to the current spec text. The current definition doesn't correctly handle large
years values, though. Therefore we still have to implement it based on the
definition from an old spec revision.
Depends on D174400
Assignee | ||
Comment 57•2 months ago
|
||
And the corresponding ToTemporalDate abstract operation.
Depends on D174401
Assignee | ||
Comment 58•2 months ago
|
||
And the corresponding ToTemporalMonthDay abstract operation.
Depends on D174402
Assignee | ||
Comment 59•2 months ago
|
||
And the corresponding ToTemporalYearMonth abstract operation.
Depends on D174403
Assignee | ||
Comment 60•2 months ago
|
||
And the corresponding ToTemporalTime abstract operation.
Depends on D174404
Assignee | ||
Comment 61•2 months ago
|
||
And the corresponding ToTemporalDateTime abstract operation.
Depends on D174405
Assignee | ||
Comment 62•2 months ago
|
||
Depends on D174406
Assignee | ||
Comment 63•2 months ago
|
||
Depends on D174407
Assignee | ||
Comment 64•2 months ago
|
||
Depends on D174408
Assignee | ||
Comment 65•2 months ago
|
||
Depends on D174409
Assignee | ||
Comment 66•2 months ago
|
||
Depends on D174410
Assignee | ||
Comment 67•2 months ago
|
||
Depends on D174411
Assignee | ||
Comment 68•2 months ago
|
||
And additionally the @@toStringTag
property.
Depends on D174412
Assignee | ||
Comment 69•2 months ago
|
||
Depends on D174413
Assignee | ||
Comment 70•2 months ago
|
||
Temporal.PlainMonthDay
is now completely implemented!
Depends on D174414
Assignee | ||
Comment 71•2 months ago
|
||
Depends on D174415
Assignee | ||
Comment 72•2 months ago
|
||
Depends on D174416
Assignee | ||
Comment 73•2 months ago
|
||
Abstract operations needed for the next patch.
Depends on D174417
Assignee | ||
Comment 74•2 months ago
|
||
The spec operation "RoundTime" rounds the mathematical value quantity
to the
input increment. To implement this operation without loss of precision, we
can't use C++ double
for the quantity
variable, but instead need to
interpret the mathematical value, i.e. a real number ℝ, as a rational number ℚ.
For example when unit="microseconds"
:
9.a Let quantity be nanosecond × 10^-3 + microsecond.
...
11. Let result be RoundNumberToIncrement(quantity, increment, roundingMode).
Where RoundNumberToIncrement is spec'ed as:
- Let quotient be x / increment.
2-8. Let rounded be RoundOp(quotient). - Return rounded × increment.
We compute quantity
as the rational number (nanosecond + microsecond × 10^3) / (10^3)
,
i.e. we scale it to nanoseconds. The quotient is then computed with
x = (nanosecond + microsecond × 10^3) / (10^3)
:
quotient
= x / increment
= ((nanosecond + microsecond × 10^3) / (10^3)) / increment
= (nanosecond + microsecond × 10^3) / (increment × 10^3)
The same approach will be used in later patches when "RoundDuration" gets
implemented. The RoundDuration implementation has code comments explaining this
approach, that's why there's a comment in the RoundTime code which points to
RoundDuration.
Depends on D174418
Assignee | ||
Comment 75•2 months ago
|
||
Depends on D174419
Assignee | ||
Comment 76•2 months ago
|
||
Depends on D174420
Assignee | ||
Comment 77•2 months ago
|
||
The Divide
function implements the different rounding modes which were already
implemented for int64_t
in "TemporalRoundingMode.h".
BigInt::divmod
computes the quotient and remainder of a BigInt division in a
single operation. The implementation is based on the existing BigInt::div
and
BigInt::mod
methods.
Depends on D174421
Assignee | ||
Comment 78•2 months ago
|
||
Also add the missing Temporal[@@toStringTag]
property.
Depends on D174422
Assignee | ||
Comment 79•2 months ago
|
||
Depends on D174423
Assignee | ||
Comment 80•2 months ago
|
||
Depends on D174424
Assignee | ||
Comment 81•2 months ago
|
||
Depends on D174425
Assignee | ||
Comment 82•2 months ago
|
||
Implement all accessor properties except for hoursInDay
, because that one
requires more Temporal.Duration
support, which hasn't landed yet.
Depends on D174426
Assignee | ||
Comment 83•2 months ago
|
||
Temporal.Duration.from
calls the "ToTemporalDuration{Record}" abstract
operation, which in turn calls "ParseTemporalDurationString". So we need to
finish the last missing bits of the Temporal parser to add support for parsing
duration strings. The duration type is placed last in a duration string
component, for example when parsing the digits 123
in the string "P123Y"
,
we need to parse until the "Y"
to know that 123
is the years part. This
makes it a bit different when compared to date-time parsing.
js::temporal::ParseTemporalDurationString
computes the fractional parts
using int64_t
variables to ensure there's no loss of precision when
compared to using double
variables.
Depends on D174427
Assignee | ||
Comment 84•2 months ago
|
||
And the correspoding CalendarDateUntil abstract operation.
Depends on D174428
Assignee | ||
Comment 85•2 months ago
|
||
Depends on D174429
Assignee | ||
Comment 86•2 months ago
|
||
In the spec, duration fields are stored as mathematical values. The "AddInstant"
operation first converts the mathematical values into BigInts and then adds
them together. We can avoid the extra BigInt allocations as follows:
- Any hours, minutes, seconds, or milliseconds component which isn't
equivalent to anint64_t
(tested withmozilla::NumberEqualsInt64
)
will make the result overflow the maximum instant value 8.64 × 10^21. - The maximum instant difference in microseconds resolution doesn't fit into
anint64_t
, so we can't use the same approach. The absolute value does fit
into anuint64_t
, though. There's nomozilla::NumberEqualsUint64
, so we
have to perform manual range checks before converting the absolute value to
anuint64_t
. - Nanoseconds are more complicated, because they fit neither in
int64_t
nor
inuint64_t
. So we just copy the existing double-to-BigInt code from
BigInt::createFromDouble()
to convert the nanoseconds and then use the
existingToInstant
function to convert the BigInt digits into anInstant
.
Then we add the hours, minutes, and seconds to the milliseconds and convert the
result into an instant, too. If the sum is a valid instant difference value, we
add the total milliseconds to the earlier computed microseconds and nanoseconds,
check again for instant difference limits, then add that sum to the input
instant. If that result is a valid epoch instant, we're finished. Otherwise the
input duration is too large and we have to report an error.
Depends on D174430
(In reply to André Bargull [:anba] from comment #15)
The current implementation supports different time zones, but not different calendars, because we still plan to implement support for calendars other than the ISO-8601 calendar through ICU4X.
Do I understand correctly that the only new dependency on ICU4C APIs that is being added is calculating the previous and next time zone changes in https://phabricator.services.mozilla.com/D174287 ?
Do I understand correctly that the ISO-8601 calendar is algorithmic enough that it is being implemented within the Temporal code itself without delegation to either ICU4C or ICU4X?
Is there a tracker for how well ICU4X covers the non-ISO-8601 calendar needs of Temporal?
Assignee | ||
Comment 88•2 months ago
|
||
(In reply to Henri Sivonen (:hsivonen) from comment #87)
Do I understand correctly that the only new dependency on ICU4C APIs that is being added is calculating the previous and next time zone changes in https://phabricator.services.mozilla.com/D174287 ?
Yes, that's currently the only new dependency on ICU4C APIs.
Do I understand correctly that the ISO-8601 calendar is algorithmic enough that it is being implemented within the Temporal code itself without delegation to either ICU4C or ICU4X?
Correct. ISO-8601 calendar algorithms are actually even defined in the Temporal spec text, for example ToISODayOfYear or ToISOWeekOfYear.
Is there a tracker for how well ICU4X covers the non-ISO-8601 calendar needs of Temporal?
I haven't yet looked into the ECMA-402 extensions for Temporal, which is where support for calendars other than ISO-8601 is defined. So I can't really tell which functionality will be required. And I also don't know how well ICU4X supports different calendars. This issue makes it sound like there's still some major work to be done, though.
Assignee | ||
Comment 89•2 months ago
|
||
Implements the "BalanceDuration" abstract operation when the relativeTo
argument isn't present.
Everything is essentially implemented two times: Once for the normal case, when
the operation can be implemented using int64_t
value. And then another time
when we need to use BigInts to ensure there's no loss of precision because the
spec actually defines the operation to work on mathematical values.
Depends on D174431
Assignee | ||
Comment 90•2 months ago
|
||
Implements the "RoundDuration" abstract operation when the relativeTo
argument isn't present.
As with previous parts, we need to make sure to compute the correct results
without any floating-point loss of precision, because the spec is defined in
terms of exact mathematical values. That means fractionalSeconds
is actually
a real number (ℝ), so we need to convert it into the rational number (ℚ)
totalNanoseconds / unit
, where totalNanoseconds
and unit
are both
integers. totalNanoseconds
should fit into int64_t
in the normal case, so
we optimise for that case. When totalNanoseconds
doesn't fit into int64_t
,
we need to compute everything using BigInt values.
I don't know if TruncateNumber
actually always computes the correct result
or if the result is incorrect by 1 ULP when compared to the exact result. But
this part of the spec is probably going to change when duration fields are
restricted to smaller values, so an in-depth numerical analysis isn't yet
needed. :-)
Also adds another Divide
function for the case when both the dividend and
the divisor are BigInt values.
Depends on D174585
Assignee | ||
Comment 91•2 months ago
|
||
Implement the last missing PlainTime.prototype methods now that RoundDuration
is available (Part 73).
Adds specialised versions of "BalanceDuration", "TotalDurationNanoseconds", and
"DurationSign" which can be implemented more efficiently when compared to the
already existing versions.
Depends on D174586
Assignee | ||
Comment 92•2 months ago
|
||
And the corresponding "CalendarDateAdd" abstract operation.
Temporal.Calendar.prototype.dateAdd
depends on BalanceDuration from part 72.
Depends on D174587
Assignee | ||
Comment 93•2 months ago
|
||
Can be implemented after adding CalendarDateAdd in part 75.
Depends on D174589
Assignee | ||
Comment 94•2 months ago
|
||
Another set of functions which depend on CalendarDateAdd from part 75.
And again extra complexity to use BigInts when we have to ensure no loss of
precision occurs.
Depends on D174590
Assignee | ||
Comment 95•2 months ago
|
||
More functionality unblocked now that CalendarDateAdd is available.
Depends on D174591
Assignee | ||
Comment 96•2 months ago
|
||
Also required CalendarDateAdd so it can be implemented.
Depends on D174592
Assignee | ||
Comment 97•2 months ago
|
||
We can now implement these functions after part 78 added "GetInstantFor".
Depends on D174593
Assignee | ||
Comment 98•2 months ago
|
||
Depends on D174594
Assignee | ||
Comment 99•2 months ago
|
||
Depends on D174595
Assignee | ||
Comment 100•2 months ago
|
||
Depends on D174596
Assignee | ||
Comment 101•2 months ago
|
||
Depends on D174597
Assignee | ||
Comment 102•2 months ago
|
||
All four functions call "ToTemporalZonedDateTime", which in turn calls the three
previously not implemented abstract operations:
- InterpretISODateTimeOffset
- ParseTemporalZonedDateTimeString
- ToTemporalOffset
Temporal.ZonedDateTime.prototype.equals
additionally calls the previously not
implemented "TimeZoneEquals" abstract operation.
Also add another "PrepareTemporalFields" implementation. This one supports the
requiredFields
parameter from the spec.
The AppendSorted
function has optimisations for appending a single or two
items. When the spec is more stable, we probably want to remove the version
which takes an initializer_list
and directly call the optimised versions.
Depends on D174598
Assignee | ||
Comment 103•2 months ago
|
||
Depends on D174599
Assignee | ||
Comment 104•2 months ago
|
||
As usual some extra code to make sure everything is computed with exact precision.
Depends on D174600
Assignee | ||
Comment 105•2 months ago
|
||
Temporal.Duration.compare
requires to support the following abstract operations:
- ToRelativeTemporalObject
- ParseTemporalRelativeToString
- CalculateOffsetShift
- UnbalanceDurationRelative
- MoveRelativeDate
- DaysUntil
- MoveRelativeDate
Depends on D174601
Assignee | ||
Comment 106•2 months ago
|
||
Temporal.Duration.prototype.{add,subtract}
calls "AddDurationToOrSubtractDurationFromDuration",
which in turn needs a few other previously unimplemented abstract operations:
AddDurationToOrSubtractDurationFromDuration:
- AddDuration
- DifferenceZonedDateTime
- DifferenceISODateTime
- NanosecondsToDays
- AddZonedDateTime
- DifferenceZonedDateTime
The two additional "BalanceDuration" functions are again needed to ensure the
precise result is correctly computed.
Depends on D174602
Assignee | ||
Comment 107•2 months ago
|
||
Adds another round of specialised version of previously implemented
functions.
And implements the "RoundDuration" abstract operation. The spec text for
"RoundDuration" is relatively large and we have to implement basically
everything at least two times to have a fast path for the common case and
additionally a slow path which uses BigInts when the numbers can't be stored
in double
without loss of precision. Both factors lead to having to write
quite a bit of code for this feature.
Depends on D174603
Assignee | ||
Comment 108•2 months ago
|
||
Part 90 added support for "RoundDuration", so now we can finally implement the
last missing Temporal.PlainDateTime
functions.
Depends on D174604
Assignee | ||
Comment 109•2 months ago
|
||
Part 90 added support for "RoundDuration", so now we can finally implement the
last missing Temporal.PlainDate
functions.
Depends on D174605
Assignee | ||
Comment 110•2 months ago
|
||
Part 90 added support for "RoundDuration", so now we can finally implement the
last missing Temporal.ZonedDateTime
functions.
Depends on D174606
Assignee | ||
Comment 111•2 months ago
|
||
Part 90 added support for "RoundDuration", so now we can finally implement the
last missing Temporal.PlainYearMonth
functions.
Depends on D174607
Assignee | ||
Comment 112•2 months ago
|
||
The is the last function which was still missing from the main Temporal spec.
Depends on D174608
Assignee | ||
Comment 113•2 months ago
|
||
TemporalFields.cpp contains a comment how to optimise
PlainDate.prototype.toPlainMonthDay
. Implement the proposed optimisation to
show more concretely what needs to be done when optimising Temporal operations.
Depends on D174609
Assignee | ||
Comment 114•2 months ago
|
||
Update the IcuMemoryUsage Java-script to support Temporal.TimeZone
.
Depends on D174616
Assignee | ||
Comment 115•2 months ago
|
||
Enables the test262 Temporal tests. Test262 needs to be reimported for this to
have an effect.
Depends on D174617
Assignee | ||
Comment 116•2 months ago
|
||
Temporal isn't enabled by default, so we need a separate taskcluster
configuration when we want to test it on the try-servers.
The new "temporaldebug" configuration is based on the existing "plaindebug"
configuration, but additionally sets the "--with-temporal-api" configure
argument.
Depends on D174618
Assignee | ||
Comment 117•2 months ago
|
||
That's it for now. It looks like the "Phabricator Revisions" tab is limited to display no more than 100 entries, so stopping at part 99 seems like a good choice. :-)
Description
•