Waldemar's stage 3 review

[waldemarhorwat]

Here's my stage 3 review. I found typos and simple issues which should be trivially fixable.

Some typos spawning infinite loops; ClassContents should be the other nonterminal in these:

ClassContents :: ClassUnion

1. Return MayContainStrings of the ClassContents.

ClassContents :: ClassIntersection

1. Return MayContainStrings of the ClassContents.

ClassContents :: ClassSubtraction

1. Return MayContainStrings of the ClassContents.

The "or" at the end makes this sentence ambiguous:

A Pattern is either a BMP pattern or a Unicode pattern depending upon whether or not its associated flags contain a u or a v.

This sentence is confusing and can be interpreted that AllCharacters is the set of all characters even if neither u nor v is set. But in that case AllCharacters should be the set of all 16-bit code points. The deleted old wording explained this better. This should either reference the unusual meaning of "character" in the sense of pattern semantics here or mention the BMP case directly:

AllCharacters is the mathematical set of “all characters” according to the regular expression flags. If UnicodeSets is true and IgnoreCase is true, then AllCharacters is the set of all Unicode code points c that do not have a Simple Case Folding mapping (that is, scf(c)=c). Otherwise, AllCharacters is the set of all characters.

MatchSequence(m1, m2) references a free variable direction which is not defined. Please add and pass direction as a third parameter.

MaybeSimpleCaseFolding algorithm: I don't like S starting as a copy of A. This causes problems for folks reading the spec because it's hard to reason about what the Remove on step 3.c.i will do without intimate knowledge of scf's implementation which is not provided in the ECMAScript spec — for example, if you don't know how scf is implemented, you can't rule out step 3.c.i removing something previously added by step 3.c.ii, which would make this algorithm dependent on the iteration order of a mathematical set. To avoid these issues, the algorithm should start with S empty and step 3.c should just add t to S if it's not already there.

[mathiasbynens]

Thanks for the thorough review, Waldemar! We’ll work on addressing these.

[mathiasbynens]

Some typos spawning infinite loops; ClassContents should be the other nonterminal in these:

ClassContents :: ClassUnion

1. Return MayContainStrings of the ClassContents.

ClassContents :: ClassIntersection

1. Return MayContainStrings of the ClassContents.

ClassContents :: ClassSubtraction

1. Return MayContainStrings of the ClassContents.

Fixed in tc39/ecma262@d0fd9b7.

The "or" at the end makes this sentence ambiguous:

A Pattern is either a BMP pattern or a Unicode pattern depending upon whether or not its associated flags contain a u or a v.

PR: mathiasbynens/ecma262#7 by @markusicu

This sentence is confusing and can be interpreted that AllCharacters is the set of all characters even if neither u nor v is set. But in that case AllCharacters should be the set of all 16-bit code points. The deleted old wording explained this better. This should either reference the unusual meaning of "character" in the sense of pattern semantics here or mention the BMP case directly:

AllCharacters is the mathematical set of “all characters” according to the regular expression flags. If UnicodeSets is true and IgnoreCase is true, then AllCharacters is the set of all Unicode code points c that do not have a Simple Case Folding mapping (that is, scf(c)=c). Otherwise, AllCharacters is the set of all characters.

PR: mathiasbynens/ecma262#5

MatchSequence(m1, m2) references a free variable direction which is not defined. Please add and pass direction as a third parameter.

PR: mathiasbynens/ecma262#7 by @markusicu

MaybeSimpleCaseFolding algorithm: I don't like S starting as a copy of A. This causes problems for folks reading the spec because it's hard to reason about what the Remove on step 3.c.i will do without intimate knowledge of scf's implementation which is not provided in the ECMAScript spec — for example, if you don't know how scf is implemented, you can't rule out step 3.c.i removing something previously added by step 3.c.ii, which would make this algorithm dependent on the iteration order of a mathematical set. To avoid these issues, the algorithm should start with S empty and step 3.c should just add t to S if it's not already there.

Patch: mathiasbynens/ecma262#6

[markusicu]

I have addressed the remaining two items in mathiasbynens/ecma262#7, please review those changes.

The "or" at the end makes this sentence ambiguous:

A Pattern is either a BMP pattern or a Unicode pattern depending upon whether or not its associated flags contain a u or a v.

and

MatchSequence(m1, m2) references a free variable direction which is not defined. Please add and pass direction as a third parameter.

[waldemarhorwat]

Looks good.

[mathiasbynens]

Thanks for your review, and for also reviewing our patches addressing your review feedback, Waldemar!