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.

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

Remove duplicate productions. #370

Closed
wants to merge 1 commit into from
Closed

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Feb 5, 2016

Specifically, in 7.1.3.1 "ToNumber Applied to the String Type",
remove the productions for:
DecimalDigits
DecimalDigit
ExponentPart
ExponentIndicator
SignedInteger
which duplicate productions (modulo the number of colons)
in 11.8.3 "Numeric Literals".

(Note that 7.1.3.1's Syntax already uses nonterminals defined
in 11.8.3 (e.g. BinaryIntegerLiteral), and already has a
sentence referring readers there for the definitions thereof.)

With this change, each nonterminal should have a single defining
production within the body of the spec (i.e., ignoring Annex B),
which should reduce link-target suprises in the rendered spec.

Specifically, in 7.1.3.1 "ToNumber Applied to the String Type",
remove the productions for:
    DecimalDigits
    DecimalDigit
    ExponentPart
    ExponentIndicator
    SignedInteger
which duplicate productions (modulo the number of colons)
in 11.8.3 "Numeric Literals".

(Note that 7.1.3.1's Syntax already uses nonterminals defined
in 11.8.3 (e.g. BinaryIntegerLiteral), and already has a
sentence referring readers there for the definitions thereof.)

With this change, each nonterminal should have a single defining
production within the body of the spec (i.e., ignoring Annex B),
which should reduce link-target suprises in the rendered spec.
@bterlson
Copy link
Member

bterlson commented Feb 6, 2016

I still need to implement a primary attribute in emu due to the usage of real productions in clause 5 (eg. WhileStatement).

I had assumed that this existed for some reason as the duplication goes back at least to ES5. But the productions are equivalent other than the colons. I wonder what @allenwb thinks. Are we missing something?

@jmdyck
Copy link
Collaborator Author

jmdyck commented Feb 6, 2016

I had assumed that this existed for some reason as the duplication goes back at least to ES5.

In fact it goes all the way back to ES1. I suggested removing the duplicates back in 2012 (https://bugs.ecmascript.org/show_bug.cgi?id=632). In ES6 rev 28 (2014-10-14), Allen removed (from "ToNumber Applied to the String Type") the MV rules for the Hex* and Decimal* nonterminals, and also removed the productions for Hex*, but left those for Decimal*, thus addressing much but not quite all of bug 632. The remainder got deferred to ES7, hence this PR.

I'm pretty sure there's no normative effect, it's just an editorial choice.

@bterlson
Copy link
Member

bterlson commented Feb 8, 2016

@jmdyck wow awesome, thanks for the history! I also can't discern any normative effect here so will pull this in.

(Also, so happy to see PRs from you again 👍)

@bterlson bterlson closed this in 573e382 Feb 8, 2016
@jmdyck jmdyck deleted the dedupe_prodns branch February 9, 2016 01:43
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Aug 27, 2019
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Sep 17, 2019
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Dec 13, 2019
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Jan 7, 2020
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Jan 25, 2020
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Feb 5, 2020
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Feb 5, 2020
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Feb 9, 2020
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Feb 14, 2020
... that I should have removed back in 573e382 (PR tc39#370).
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Feb 20, 2020
... that I should have removed back in 573e382 (PR tc39#370).
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Feb 21, 2020
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Mar 18, 2020
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.

2 participants