Skip to content

Editorial: Add "!" before infallible calls#660

Closed
gibson042 wants to merge 7 commits intotc39:masterfrom
gibson042:2022-03-infallible-calls
Closed

Editorial: Add "!" before infallible calls#660
gibson042 wants to merge 7 commits intotc39:masterfrom
gibson042:2022-03-infallible-calls

Conversation

@gibson042
Copy link
Member

This cleans up some calls that are not already addressed by my other open PRs (#602, #610, #625, #630).

@gibson042 gibson042 requested a review from ryzokuken March 17, 2022 21:41
1. Assert: Type(_tag_) is String.
1. Assert: Type(_options_) is Object.
1. If IsStructurallyValidLanguageTag(_tag_) is *false*, throw a *RangeError* exception.
1. If ! IsStructurallyValidLanguageTag(_tag_) is *false*, throw a *RangeError* exception.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

given 262's completion reform, where most AOs don't need to return completions, would it make more sense to update 402 to match rather than adding a bunch of prefixes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, quite possibly. Do you have a sense of what that would like like here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’d expect a separate PR that updates every AO to use the new signature, and ecmarkup will enforce that only those that return completion records are used with the ? or ! prefix - and predicates all don’t.

It’s probably a good amount of work, but it’s also probably a good idea to do before 2022, so the two 2022’s can match.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're hoping for the upcoming TC39 plenary to approve the 2022 spec, and my other open PRs have more material impact. I'd therefore like ECMA-402 completion reform to follow those, but only one is considered necessary for the 2022 edition so there probably won't be time to make the cutoff.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that makes sense, but i'm not actually sure the 402 spec is valid anymore since the implicit completion rules for AOs in the 262 spec it relies on no longer exist.

in other words, i'm not sure the 402 2022 spec can be cut until the completion reform is done :-/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that the use of completion records in 262 was technically "invalid" for years, I don't think completion reform needs to block cutting the 2022 edition unless the current state is actually unclear to human readers.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How was it invalid?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Completion records were described as only containing ECMAScript language values, but held many other types of values.

(Also the "Any reference to a Completion Record value that is in a context that does not explicitly require a complete Completion Record value is equivalent to an explicit reference to the [[Value]] field of the Completion Record value unless the Completion Record is an abrupt completion." wasn't really precise enough to be meaningful.)

@Ms2ger
Copy link
Contributor

Ms2ger commented Mar 23, 2022

Can you review the ArrayCreate calls as well (unless you're doing those elsewhere)?

@gibson042
Copy link
Member Author

gibson042 commented Mar 23, 2022

@Ms2ger Done, along with some other broad patterns (although I'd like to stop here for the other PRs to settle before addressing the rest in a followup).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants