Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Lack of [[Source]] in FormatNumericRangeToParts if returned by FormatApproximately #99

Closed
FrankYFTang opened this issue Jun 21, 2022 · 4 comments
Closed

Lack of [[Source]] in FormatNumericRangeToParts if returned by FormatApproximately #99

FrankYFTang opened this issue Jun 21, 2022 · 4 comments
Assignees
@sffc
Labels
bug Something isn't working

Comments

@FrankYFTang
Copy link
Contributor

FrankYFTang commented Jun 21, 2022 •
edited
Loading

In FormatNumericRangeToParts
https://tc39.es/proposal-intl-numberformat-v3/out/numberformat/diff.html#sec-formatnumericrangetoparts

1. Let parts be ? PartitionNumberRangePattern (numberFormat, x, y).
...
4. For each part in parts, do
  d. Perform ! CreateDataPropertyOrThrow(O, "source", part.[[Source]]).

but in PartitionNumberRangePattern
https://tc39.es/proposal-intl-numberformat-v3/out/numberformat/diff.html#sec-partitionnumberrangepattern

8 If xResult is equal to yResult, return FormatApproximately (numberFormat, xResult).

and
in FormatApproximately
https://tc39.es/proposal-intl-numberformat-v3/out/numberformat/diff.html#sec-formatapproximately

3. Insert a new Record { [[Type]]: "approximatelySign", [[Value]]: approximatelySign } at index i in result.

this mean step 4-d in PartitionNumberRangePattern may get a part without [[Source]].

I think this is a spec bug and we should change step 3 of FormatApproximately to

3. Insert a new Record { [[Type]]: "approximatelySign", [[Value]]: approximatelySign, [[Source]]: "shared" } at index i in result.

instead.
@sffc sffc added the bug Something isn't working label Jun 21, 2022
@sffc sffc self-assigned this Jun 21, 2022
@sffc
Copy link
Collaborator

sffc commented Jun 21, 2022

DateTimeFormat adds the source field for equivalent dates:

https://tc39.es/ecma402/#sec-partitiondatetimerangepattern

We should do the same in NumberFormat.

@anba
Copy link

anba commented Jun 22, 2022

Same issue as #63, right?

pull bot pushed a commit to jamlee-t/v8 that referenced this issue Jun 22, 2022
@FrankYFTang
Fix unreachable code under --harmony-intl-number-format-v3
ba6db33 
ICU 71 added new enum value UNUM_APPROXIMATELY_SIGN_FIELD
need to map to "approximatelySign"

We also discover a spec bug in
tc39/proposal-intl-numberformat-v3#99

All the parts of formatRangeToParts should have a source "shared" for
the case that start and end are the same or very close.

Bug: chromium:1336865
Change-Id: I89142479989d3d2017d8cb89194db737710c38ed
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3717278
Reviewed-by: Shu-yu Guo <[email protected]>
Commit-Queue: Frank Tang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#81286}
@FrankYFTang
Copy link
Contributor Author

Same issue as #63, right?

yes.

@FrankYFTang
Copy link
Contributor Author

this is a dup of #63

@sffc sffc closed this as not planned Won't fix, can't repro, duplicate, stale Jun 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sffc sffc

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@sffc @anba @FrankYFTang