Closed nekevss closed 1 month ago
Marking this as ready for review. There still may be a couple fixes needed, but I think I'd prefer to move forward with this for now, and do fixes in the follow up.
Overall, this is only -6 test262 tests vs. the current version. The primary cause is most likely due to the constraining needed around month
and day
. (e.g., { year: 2021, month: 99999, day: 500 }
)
I'm currently thinking of two approaches: switch MonthCode
to Month
, or CalendarFields
will need to take the overflow argument and handle the constrain behavior when constructed (not the biggest fan of the latter).
EDIT: This actually needs a little more work I think
This is more or less ready for review. Would rather move forward with this as is and address the broken tests in the future.
Below are the diff results of running the current version in Boa vs. this PR in Boa.
< Fetching latest test262 commits...
< Test262 switching to commit dde3050bdbfb8f425084077b6293563932d57ebc...
< Reset test262 to commit: dde3050bdbfb8f425084077b6293563932d57ebc...
< HEAD is now at dde3050bdb Fix test regression in asyncHelpers.js (#4197)
8c4
< Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal:
---
> Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/:
1570,1572c1566
< `/home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/from/one-of-era-erayear-undefined.js`: Passed
< `/home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/from/one-of-era-erayear-undefined.js` (strict mode): starting
< `/home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/from/one-of-era-erayear-undefined.js` (strict): Passed
---
> `/home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/from/one-of-era-erayear-undefined.js`: Failed
1724c1718
< Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/from results: total: 56, passed: 42, ignored: 0, failed: 14 , conformance: 75.00%
---
> Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/from results: total: 56, passed: 41, ignored: 0, failed: 15 , conformance: 73.21%
2096,2098c2090
< `/home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/prototype/with/overflow-undefined.js`: Passed
< `/home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/prototype/with/overflow-undefined.js` (strict mode): starting
< `/home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/prototype/with/overflow-undefined.js` (strict): Passed
---
> `/home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/prototype/with/overflow-undefined.js`: Failed
2132c2124
< Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/prototype/with results: total: 18, passed: 15, ignored: 0, failed: 3 , conformance: 83.33%
---
> Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/prototype/with results: total: 18, passed: 14, ignored: 0, failed: 4 , conformance: 77.78%
3438c3430
< Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/prototype results: total: 460, passed: 281, ignored: 0, failed: 179 , conformance: 61.09%
---
> Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate/prototype results: total: 460, passed: 280, ignored: 0, failed: 180 , conformance: 60.87%
3512c3504
< Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate results: total: 573, passed: 370, ignored: 0, failed: 203 , conformance: 64.57%
---
> Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/PlainDate results: total: 573, passed: 368, ignored: 0, failed: 205 , conformance: 64.22%
11590c11582
< Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal results: total: 3855, passed: 1568, ignored: 0, failed: 2287 , conformance: 40.67%
---
> Suite /home/nekevss/Projects/boa/test262/test/built-ins/Temporal/ results: total: 3855, passed: 1566, ignored: 0, failed: 2289 , conformance: 40.62%
11595c11587
< Passed tests: 1568
---
> Passed tests: 1566
11597,11598c11589,11590
< Failed tests: 2287 (0 panics)
< Conformance: 40.67%
---
> Failed tests: 2289 (0 panics)
> Conformance: 40.62%
This PR refactors
TemporalFields
intoCalendarFields
. I'm leaving it as a draft for now as I want to run plug this into Boa and run some tests beforehand.General Context:
This builds on the work from #89 and #92.
With the partials implemented for the
with
methods. The values that would be provided can now be accounted for. Primarily, the time fields really didn't need to exist in theTemporalFields
representation anymore (along with the TimeZone and offset which will probably be living on thePartialZonedDateTime
). Furthermore, the partials were now handling theundefined
case of the fields, so the calendar specific fields no longer needed to be anOption
. In general, I also just think that this is a better native representation of the fields values in comparison to the previous version.Furthermore, this PR also implements better handling for Era and Month Codes according to the Intl era and monthCode proposal.
There are a couple points where feedback/bike-shedding would definitely be appreciated. I think the method names in
calendar_types.rs
are getting a bit unyielding. I am also a bit worried about introducing theCalendarMethods
trait, but I do think it makes the ergonomics around passingDate
,DateTime
, and, eventually,ZonedDateTime
as a fallback convenient.cc: @sffc