This repository has been archived by the owner on Sep 27, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 48
Master #80
Open
ericlippert
wants to merge
50
commits into
facebookarchive:master
Choose a base branch
from
ericlippert:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Master #80
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
A backslash was somehow used where a tilde was expected in two places, and an extra backslash crept into an arithmetic expression.
When the coalescing operator (??) was added to section 9, it was not added to the copy of the grammar in section 22.
The spec had namespace names and qualified names defined in the lexical grammar, but no way to get from "token" to "qualified name". The simplest solution is to make "\" a token and make the namespace name and qualified name be part of the syntactic grammar, not the lexical grammar. I note that this is how the Hack and PHP tokenizers actually work, so the spec is brought into line with the implementation.
The lexical grammar did not include the "..." token used in variadic formal parameter lists.
* Hack reserves keywords "parent" and "self"; this was not documented in the specification. (Interestingly, Hack does not reserve "null", "true", "false" or "this", which we might want to look into.) * The explanation of the ${ } interpolation in string literals had a link to a now-deleted section of the specification, which meant that the feature was not explained. Since this feature is very restricted compared to its equivalent in PHP -- the PHP version of it cannot even be *lexed* without direction from the parser! -- I've added some verbiage explaining the difference and giving examples. * The deleted feature mentioned in the point above is the Variable-Name creation operator, but does not give the syntax which has been removed. The ${$y} syntax is at present unsupported in Hack. The $$y syntax is supported incorrectly now; Alex has a diff outstanding to make it an error.
Link to enum-declarations was wrong
Looks like a previous search-and-replace operation was incomplete. The require-once directive should be listed as a directive, not an expression.
The ${} interpolation is invalid in strict Hack, so I've removed it from the specification entirely. (Except to be called out as unsupported in section 23.)
Fixed missing hyphens in return-type
Fix broken cross references
The grammar suggests that the "namespace foo { }" form contains a block of statements, but in fact it contains a block of declarations. I've also eliminated the problematic use of "scope" to mean "definition extent". We already define "scope" as the region of program text in which a given name is visible, so we should not use "scope" to mean the region of program text which belongs syntactically to a namespace definition.
Link to cast expressions is incorrect
Chapter 10 lists two production for anonymous-function-return; chapter 22 only lists one.
We can re-use the return-type production in anonymous type returns.
The grammar implied that enumerated type bodies were of the form x = 1 ; y = 2 that is, semicolon separated; in fact the items are semicolon terminated: x = 1; y = 2;
The specification stated that a method is a modifier followed by a function, but that's not right; that would imply that "public <<foo>> function bar()..." is legal. The <<foo>> needs to come before the public. I've fixed up the grammar to ensure that the attribute comes before the modifier.
The specification nowhere noted that the first statement inside the block of a switch must be a case or default.
The require-extends-clause and require-implements-clause productions were missing the required semicolons.
Hack, oddly enough, allows ": void" annotations on constructors and destructors, but does not require them. The annotation must be exactly ": void"; it cannot be an alias for void, for example.
In Hack it is legal for a non-empty parameter list to end with a comma. For example "function foo(int $x, int $y,) : void {}"
The text originally said that a catch clause parameter is a parameter list with exactly one element. But this is wrong; a parameter list with exactly one element can (1) have an optional attribute, (2) have an optional default value, and (3) have an optional trailing comma. None of those are legal in Hack. It is far simpler to say that a catch clause must have a type and a variable, end of story.
The productions for anonymous function and lambda parameter declarations did not support either "..." or trailing comma syntaxes. I've fixed up the grammar to support both, as we do for named function / method parameter lists.
The "anonymous-function-body" production is nowhere used in the anonymous function production, but rather only in the lambda expression production. This seems bizarre and confusing. I've renamed anonymous-function-body to the more understandable "lambda-body".
The spec had numerous typos where generic type parameter lists eg,"<+T, -U>" were specified when generic type argument lists eg "<int, string>" were intended.
Argument lists, like parameter lists, may end in a trailing comma if there is at least one item.
A generic type argument list may optionally end in a comma.
A type specifier list -- used in tuple type and anonymous function type productions -- may end in a comma.
The right hand side of a scope resolution qualifier may be the keyword "class"; the grammar had "class" in italics, indicating that it was a non-terminal, when a terminal was intended.
The grammar did not indicate that the scope resolution operator could have a variable name on the right. (Though the supporting text indicated that it could be used to name a static property.)
Hack permits the class type designator of an object creation ("new") expression to be a scope-qualified name ("$foo::$bar"). I also tidied up the normative text somewhat to be a bit more clear and less redundant.
Hack allows the class designator to be of the form $x->$y, and a few other forms. I've updated the grammar and the supporting text.
A member selection operator (-> or ?->) may have a variable on the right, eg, $x->$y, in strict mode. I also fixed some mis-edits in the grammar summary; it looks like at one time there was going to be a "designator" production as with "new".
Namespace use declarations may (1) specify the "kind", so that they can alias const or function members, and (2) have a "group" syntactic sugar to cut down on irritating redundancy.
The field specifier in a shape may be of the form "foo::bar".
The definition of a shape type may use the :: operator.
A generic type parameter declaration list may be comma terminated: class List<T,U,> ...
I could not make head nor tail of the constraints on type aliases; as far as I can tell, the thing to the right can be any type, and I don't know why there is all this verbiage -- some of it ungrammatical and likely due to mis-edits -- about qualified names and what the name means. Also, a type alias may declare a generic type parameter list. I'm not yet quite sure of what the semantics are. Once I figure that out I'll come back and add some text describing it.
A type alias (type, newtype) declaration may have an <<attribute>> on it, just like a class.
An inclusion directive takes as its operand an expression. The grammar calls out that it could be an expression or a parenthesized expression, but a parenthesized expression already is an expression. We can simply say that the directive takes an expression, end of story.
The use-clause variable list may have a trailing comma
Field initializers can have a trailing comma
The argument of a classname type can be generic
The lexical grammar as given in the spec required that the body of a HEREDOC/NOWDOC string literal contain at least one newline, but that is not required: <<<FOO FOO is a legal string, but the grammar required that it be at least <<<FOO FOO I've fixed up the grammar so that the body is not required.
* An enum declaration can have an attribute specification * Fixed the formatting of the "defined elsewhere" list in the section on enums.
The spec only allowed for four things in a cast operator, but in fact Hack allows eight keywords and any simple name. The grammar now matches the implementation. However, doing so introduces an ambiguity; (x)-$y could be a subtraction or a cast. The spec now describes a heuristic that an implementation can use to tell whether or not (x) is to be parsed as a cast or a parenthesized expression. There is still more work to do here, not done in this diff: (1) We need to describe the semantics of all possible casts. (2) We should consider whether the cast type can be more than just a simple name. Can it be an array type, a shape type, a generic type, a nullable type? Can it be a qualified name? And so on.
The grammar listing has a byref-assignment-expression production which is nowhere mentioned elsewhere in the grammar or in the specification. I assume this is a mis-edit; when the feature was removed from the spec it was not removed from the grammar chapter.
Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired. Before we can review or merge your code, we need you to email [email protected] with your details so we can update your status. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
HEREDOC / NOWDOC strings can be entirely empty; the spec said the body had to have at least a newline.
An enum declaration may begin with an attribute specification.
A cast type may be a simple name. This introduces an ambiguity in the grammar; I describe a heuristic that an implementation can use to disambiguate casts from parenthesized expressions.
Remove the unused "byref assignment" production from the grammar.