Normative: Fix inconsistency in order of observable operations in ...FromFields() for non-ISO calendars #2377
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ptomato
added
spec-text
Codecov Report
@@ Coverage Diff @@
## main #2377 +/- ##
=======================================
Coverage 95.01% 95.01%
=======================================
Files 20 20
Lines 10803 10803
Branches 1925 1925
=======================================
Hits 10264 10264
Misses 503 503
Partials 36 36
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Aditi-1400
approved these changes
Aug 16, 2022
sffc
approved these changes
Aug 16, 2022
gibson042
previously requested changes
Aug 17, 2022
ptomato
force-pushed
the
observable-order-inconsistency
branch
from
60fc723 to
95bfd89
Compare
Ms2ger
approved these changes
Aug 23, 2022
|
I'll mark this as a draft in anticipation of presenting it at the next TC39 meeting. |
ptomato
force-pushed
the
observable-order-inconsistency
branch
from
95bfd89 to
875c5bb
Compare
ptomato
added a commit
to tc39/how-we-work
that referenced
this pull request
Aug 26, 2022
Triggered by the discussion in tc39/proposal-temporal#2377 (comment) Arguments should consistently be processed in order.
ptomato
added a commit
to ptomato/test262
that referenced
this pull request
Sep 14, 2022
This implements the normative change in tc39/proposal-temporal#2377 which reached consensus at the September 2022 TC39 meeting. It changes the order in which observable operations are performed on the values passed to the ___fromFields methods of Calendar.
|
This reached consensus at the September 2022 TC39 meeting. Tests are in tc39/test262#3666 |
Ms2ger
pushed a commit
to tc39/test262
that referenced
this pull request
Sep 15, 2022
This implements the normative change in tc39/proposal-temporal#2377 which reached consensus at the September 2022 TC39 meeting. It changes the order in which observable operations are performed on the values passed to the ___fromFields methods of Calendar.
Ms2ger
force-pushed
the
observable-order-inconsistency
branch
from
875c5bb to
bf1b916
Compare
Previously, the monthDayFromFields() method of non-ISO calendars behaved differently from dateFromFields() and yearMonthFromFields(), as well as monthDayFromFields() of the ISO calendar. The latter checked the overflow option before accessing the fields, and the former checked the overflow option afterwards. This normative change fixes this inconsistency. The general principle is that arguments to a function should be validated in order, so we go with accessing the fields before checking the overflow option. Requires an update to the reference code, which previously consistently checked the overflow option first.
Ms2ger
force-pushed
the
observable-order-inconsistency
branch
from
bf1b916 to
6d66459
Compare
balusch
pushed a commit
to balusch/v8
that referenced
this pull request
Sep 24, 2022
Sync to the spec/calendar.html changes in tc39/proposal-temporal#2377 to change the order of calling ToTemporalOverflow. This cl only cover the ISO8601 part, the the intl part of the PR is not yet implemented and will be handle when e implemenet them. Spec text: https://tc39.es/proposal-temporal/#sec-temporal-isodatefromfields https://tc39.es/proposal-temporal/#sec-temporal-isoyearmonthfromfields https://tc39.es/proposal-temporal/#sec-temporal-isomonthdayfromfields Bug: v8:11544 Change-Id: Ia4386d460dc45b0b377a483c6f4793da4cbd7c20 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3903223 Commit-Queue: Frank Tang <[email protected]> Reviewed-by: Adam Klein <[email protected]> Cr-Commit-Position: refs/heads/main@{#83410}
catamorphism
pushed a commit
to catamorphism/test262
that referenced
this pull request
Oct 4, 2022
This implements the normative change in tc39/proposal-temporal#2377 which reached consensus at the September 2022 TC39 meeting. It changes the order in which observable operations are performed on the values passed to the ___fromFields methods of Calendar.
catamorphism
pushed a commit
to catamorphism/test262
that referenced
this pull request
Oct 5, 2022
This implements the normative change in tc39/proposal-temporal#2377 which reached consensus at the September 2022 TC39 meeting. It changes the order in which observable operations are performed on the values passed to the ___fromFields methods of Calendar.
ptomato
added a commit
to tc39/how-we-work
that referenced
this pull request
Feb 13, 2023
Triggered by the discussion in tc39/proposal-temporal#2377 (comment) Arguments should consistently be processed in order.
ptomato
added a commit
to tc39/how-we-work
that referenced
this pull request
Jun 27, 2024
Triggered by the discussion in tc39/proposal-temporal#2377 (comment) Arguments should consistently be processed in order.
ptomato
added a commit
to tc39/how-we-work
that referenced
this pull request
Jul 12, 2024
Triggered by the discussion in tc39/proposal-temporal#2377 (comment) Arguments should consistently be processed in order.
ptomato
added a commit
to tc39/how-we-work
that referenced
this pull request
Jul 16, 2024
Triggered by the discussion in tc39/proposal-temporal#2377 (comment) Arguments should consistently be processed in order.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, the monthDayFromFields() method of non-ISO calendars behaved
differently from dateFromFields() and yearMonthFromFields(), checking the
overflow option after accessing the fields whereas the other two checked
the overflow option first.
This normative change fixes this inconsistency.
Split out from #2199 which does not need to be normative. Note this will need a rebase when #2199 is merged.