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

Editorial: Fix grammatical term misuse #804

Merged
merged 16 commits into from
Feb 13, 2017
Merged

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Feb 8, 2017

Fix many incorrect or questionable uses of grammatical terms, mostly "production" being used in the sense of "instance of a production".

See https://bugs.ecmascript.org/show_bug.cgi?id=2985, submitted in 2014 against ES6 by @rossberg-chromium.

(There are still a few problematic cases, but this PR is good enough for now.)

jmdyck added 15 commits February 7, 2017 21:50
When a phrase such as
    a |Foo| production
or
    a |Foo| grammar production
or
    a |Foo| syntactic production
is talking about an *instance* of the production,
change it to just
    a |Foo|
Replace 4 occurrences of
    the syntactic production that is being evaluated
with a reference to the appropriate nonterminal.
(Because one doesn't evaluate productions.)
Change occurrences of "this production" to "this |Foo|" where appropriate.
In
    When processing the production <emu-grammar>...
and
    Let _formalParameterList_ be the production <emu-grammar>...

change
    the production
to
    an instance of the production
Change occurrences of
    |ClassElement| is the production <emu-grammar>...
to
    |ClassElement| is <emu-grammar>...

(It should more properly be "is an instance of", but
there are lots of pre-existing examples of the "is" formulation.)
Change a couple occurrences of "production" to "phrase".
Replace occurrences of:
    parsed grammar phrase
    parsed grammar production
    parse result
with
    Parse Node
For phrases such as:
    a production
    a grammar production
    a syntactic grammar production
when they actually refer to an *instance* of the production,
change them to:
    a Parse Node
this production expansion's |Term|        ->  this |Term|
this production's |Term|                  ->  this |Term|
this production's |Atom|                  ->  |Atom|
the expansion of this production's |Atom| ->  |Atom|

to the left of this production expansion's initial left parenthesis ->
to the left of this |Atom|
this production's |Atom|  ->  this |Atom|
number of times the <emu-grammar> production is expanded  ->
number of <emu-grammar> Parse Nodes

number of <emu-grammar> productions ->
number of <emu-grammar> Parse Nodes
the total number of <emu-grammar> Parse Nodes prior to this |Thing| plus
the total number of <emu-grammar> Parse Nodes enclosing this |Thing|

->

the total number of <emu-grammar> Parse Nodes prior to or enclosing this |Thing|
Rename some parameters whose names don't convey
the values that get passed to them:

EvaluateNew:                   _constructProduction_ -> _constructExpr_
IsAnonymousFunctionDefinition: _production_  -> _expr_
IsInTailPosition:              _nonterminal_ -> _call_
HasProductionInTailPosition:   _nonterminal_ -> _call_
(The former wording read like a traversal of the grammar,
not a traversal of a parse tree.)
@bterlson
Copy link
Member

This is awesome. We should probably define a term Parse Node in 5.1.4 or similar and call out that it is an instance of whatever production it parsed and that it has Children.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Feb 12, 2017

We should probably define a term Parse Node in 5.1.4 or similar

Done. (I'm not entirely happy with the wording, but don't see an easy way to improve it.)

@bterlson
Copy link
Member

FWIW, I find this wording impeccable!

@bterlson bterlson merged commit d65c54b into tc39:master Feb 13, 2017
@jmdyck jmdyck deleted the gram_term_misuse branch February 13, 2018 03:24
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