778919 - remove CheckStrictParameters

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Luke Wagner [:luke]]

CheckStrictParameters is kindof an oddball:
 - it iterates over all bindings (not just parameters, why this is ok takes some hand-waving)
 - it builds a hashset containing all formals, to look for duplicates (even though we already have tc->decls)
 - it checks for the bad names (eval, keywords), which we usually do when defining bindings and already do for destructuring formals

This weirdness gets in the way of bug 775323.

This trio of patches removes it by putting the relevant checks into DefineArg and BindDestructuringArg, making them more symetrics with the rest of the parser's handling of strict-mode checks.

Comment 1

[Luke Wagner [:luke]]

Attachment: move DefineArg and BindDestructuringArg to be next to functionArguments

Simple syntax movement + a comment.

Comment 2

[Luke Wagner [:luke]]

Attachment: do CheckStrictBindings like everywhere else

Comment 3

[Luke Wagner [:luke]]

Attachment: rm CheckStrictParameters

Do the dup-checking at the one use site.  Strict mode makes this an error, so the bool to check whether we've already warned was useless.

Comment 4

[Eddy Bruel [:ejpbruel]]

Comment on attachment 647287 [details] [diff] [review]
do CheckStrictBindings like everywhere else

Review of attachment 647287 [details] [diff] [review]:
-----------------------------------------------------------------

I assume your comment that:
- it checks for the bad names (eval, keywords), which we usually do when defining bindings and already do for destructuring formals"

was wrong, because you had to add a call to CheckStrictBinding in BindDestructuringArg?

Other than that, looks to me like this patch does the right thing.

Comment 5

[Luke Wagner [:luke]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7f3055bc1c4c
https://hg.mozilla.org/integration/mozilla-inbound/rev/e60512aa511f
https://hg.mozilla.org/integration/mozilla-inbound/rev/ee8a5b7e0d31

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7f3055bc1c4c
https://hg.mozilla.org/mozilla-central/rev/e60512aa511f
https://hg.mozilla.org/mozilla-central/rev/ee8a5b7e0d31