[strong] Deprecate for-in

[strong] Deprecate for-in

R=marja@chromium.org
BUG=

Committed: https://crrev.com/7d089a59293cf8dd6b8ef579bba3a63350da90b2
Cr-Commit-Position: refs/heads/master@{#26751}

[marja, 2015-02-19 12:34:23]

https://codereview.chromium.org/939063002/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/939063002/diff/1/src/parser.cc#newcode2884
src/parser.cc:2884: bool Parser::CheckInOrOf(bool accept_OF,
... could you move CheckInOrOf to ParserBase, seems pretty unnecessary to have
it twice.

https://codereview.chromium.org/939063002/diff/1/src/preparser.cc
File src/preparser.cc (right):

https://codereview.chromium.org/939063002/diff/1/src/preparser.cc#newcode752
src/preparser.cc:752: if (*ok) {
Nit: I'd write this the same style as Parser, so,

if (!*ok) early return_;
go_on();

... just to keep the logic as unified as possible. It's just a tiny thing but
Parser / PreParser are full of these unnecessary "let's inverse the condition
and write the blocks in a different order" things which accumulate and make the
eventual unification more tedious.

https://codereview.chromium.org/939063002/diff/1/test/mjsunit/strong/for-in.js
File test/mjsunit/strong/for-in.js (right):

https://codereview.chromium.org/939063002/diff/1/test/mjsunit/strong/for-in.j...
test/mjsunit/strong/for-in.js:12: assertThrows("'use strong'; for (var x = 0 in
[]);", SyntaxError);
Can we has tests that for of still works?

(In general, how are we going to test that the stuff we want to keep working
stays working? Should we have a list of tests which should also work w/ strong
mode and then run them w/ strong mode, so that we don't need to duplicate them?)

[rossberg, 2015-02-19 13:23:05]

https://codereview.chromium.org/939063002/diff/1/src/parser.cc
File src/parser.cc (right):

https://codereview.chromium.org/939063002/diff/1/src/parser.cc#newcode2884
src/parser.cc:2884: bool Parser::CheckInOrOf(bool accept_OF,
On 2015/02/19 12:34:22, marja wrote:
> ... could you move CheckInOrOf to ParserBase, seems pretty unnecessary to have
> it twice.

Done.

https://codereview.chromium.org/939063002/diff/1/src/preparser.cc
File src/preparser.cc (right):

https://codereview.chromium.org/939063002/diff/1/src/preparser.cc#newcode752
src/preparser.cc:752: if (*ok) {
On 2015/02/19 12:34:22, marja wrote:
> Nit: I'd write this the same style as Parser, so,
> 
> if (!*ok) early return_;
> go_on();
> 
> ... just to keep the logic as unified as possible. It's just a tiny thing but
> Parser / PreParser are full of these unnecessary "let's inverse the condition
> and write the blocks in a different order" things which accumulate and make
the
> eventual unification more tedious.

Done.

https://codereview.chromium.org/939063002/diff/1/test/mjsunit/strong/for-in.js
File test/mjsunit/strong/for-in.js (right):

https://codereview.chromium.org/939063002/diff/1/test/mjsunit/strong/for-in.j...
test/mjsunit/strong/for-in.js:12: assertThrows("'use strong'; for (var x = 0 in
[]);", SyntaxError);
On 2015/02/19 12:34:22, marja wrote:
> Can we has tests that for of still works?

Done. Also removed the nonsensical var tests, since that's illegal anyway.

> (In general, how are we going to test that the stuff we want to keep working
> stays working? Should we have a list of tests which should also work w/ strong
> mode and then run them w/ strong mode, so that we don't need to duplicate
them?)

Good question. Most existing tests will probably not work, for 'var' alone.
Maybe it's good enough to have a number of dedicated strong mode test programs.
If something really gets ruled out accidentally then I suppose we gonna find out
relatively quickly...

[marja, 2015-02-19 13:28:58]

lgtm

[rossberg, 2015-02-19 13:29:10]

The CQ bit was checked by rossberg@chromium.org

[commit-bot: I haz the power, 2015-02-19 13:30:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/939063002/20001

[commit-bot: I haz the power, 2015-02-19 13:50:37]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-02-19 13:50:55]

Patchset 2 (id:??) landed as
https://crrev.com/7d089a59293cf8dd6b8ef579bba3a63350da90b2
Cr-Commit-Position: refs/heads/master@{#26751}

[arv (Not doing code reviews), 2015-02-19 14:21:15]

arv@chromium.org changed reviewers:
+ arv@chromium.org

[arv (Not doing code reviews), 2015-02-19 14:21:15]

LGTM

[marja, 2015-02-19 16:15:42]

On 2015/02/19 13:23:05, rossberg wrote:
> Good question. Most existing tests will probably not work, for 'var' alone.
> Maybe it's good enough to have a number of dedicated strong mode test
programs.
> If something really gets ruled out accidentally then I suppose we gonna find
out
> relatively quickly...

... how would we find it out though if we only run our small and few dedicated
test progs w/ strong mode, and no other code?

[rossberg, 2015-02-19 16:19:41]

On 2015/02/19 16:15:42, marja wrote:
> On 2015/02/19 13:23:05, rossberg wrote:
> > Good question. Most existing tests will probably not work, for 'var' alone.
> > Maybe it's good enough to have a number of dedicated strong mode test
> programs.
> > If something really gets ruled out accidentally then I suppose we gonna find
> out
> > relatively quickly...
> 
> ... how would we find it out though if we only run our small and few dedicated
> test progs w/ strong mode, and no other code?

Yeah, I think once we have a critical mass of features, we should port some
interesting programs to strong mode as tests. We could also try to run (the
strongified version of) Traceur as a test. Arv, is Traceur pure ES6?

[arv (Not doing code reviews), 2015-02-19 16:26:17]

On 2015/02/19 16:19:41, rossberg wrote:
> On 2015/02/19 16:15:42, marja wrote:
> > On 2015/02/19 13:23:05, rossberg wrote:
> > > Good question. Most existing tests will probably not work, for 'var'
alone.
> > > Maybe it's good enough to have a number of dedicated strong mode test
> > programs.
> > > If something really gets ruled out accidentally then I suppose we gonna
find
> > out
> > > relatively quickly...
> > 
> > ... how would we find it out though if we only run our small and few
dedicated
> > test progs w/ strong mode, and no other code?
> 
> Yeah, I think once we have a critical mass of features, we should port some
> interesting programs to strong mode as tests. We could also try to run (the
> strongified version of) Traceur as a test. Arv, is Traceur pure ES6?

Yes. Right now we compile it to ES5 but there is an option to compile it to ES6.
We have granular flags for everything and I was planning to start using io.js
and keep things io.js supports (let, generators, for-of etc).

[rossberg, 2015-02-19 16:32:59]

On 2015/02/19 16:26:17, arv wrote:
> On 2015/02/19 16:19:41, rossberg wrote:
> > On 2015/02/19 16:15:42, marja wrote:
> > > On 2015/02/19 13:23:05, rossberg wrote:
> > > > Good question. Most existing tests will probably not work, for 'var'
> alone.
> > > > Maybe it's good enough to have a number of dedicated strong mode test
> > > programs.
> > > > If something really gets ruled out accidentally then I suppose we gonna
> find
> > > out
> > > > relatively quickly...
> > > 
> > > ... how would we find it out though if we only run our small and few
> dedicated
> > > test progs w/ strong mode, and no other code?
> > 
> > Yeah, I think once we have a critical mass of features, we should port some
> > interesting programs to strong mode as tests. We could also try to run (the
> > strongified version of) Traceur as a test. Arv, is Traceur pure ES6?
> 
> Yes. Right now we compile it to ES5 but there is an option to compile it to
ES6.
> We have granular flags for everything and I was planning to start using io.js
> and keep things io.js supports (let, generators, for-of etc).

Okay, what does "compile to ES6" mean in this case? If the Traceur sources are
pure ES6, could an ES6-ready VM perhaps even run Traceur itself without
bootstrapping through self compilation first?

[arv (Not doing code reviews), 2015-02-19 17:01:03]

On 2015/02/19 16:32:59, rossberg wrote:
> On 2015/02/19 16:26:17, arv wrote:
> > On 2015/02/19 16:19:41, rossberg wrote:
> > > On 2015/02/19 16:15:42, marja wrote:
> > > > On 2015/02/19 13:23:05, rossberg wrote:
> > > > > Good question. Most existing tests will probably not work, for 'var'
> > alone.
> > > > > Maybe it's good enough to have a number of dedicated strong mode test
> > > > programs.
> > > > > If something really gets ruled out accidentally then I suppose we
gonna
> > find
> > > > out
> > > > > relatively quickly...
> > > > 
> > > > ... how would we find it out though if we only run our small and few
> > dedicated
> > > > test progs w/ strong mode, and no other code?
> > > 
> > > Yeah, I think once we have a critical mass of features, we should port
some
> > > interesting programs to strong mode as tests. We could also try to run
(the
> > > strongified version of) Traceur as a test. Arv, is Traceur pure ES6?
> > 
> > Yes. Right now we compile it to ES5 but there is an option to compile it to
> ES6.
> > We have granular flags for everything and I was planning to start using
io.js
> > and keep things io.js supports (let, generators, for-of etc).
> 
> Okay, what does "compile to ES6" mean in this case? If the Traceur sources are
> pure ES6, could an ES6-ready VM perhaps even run Traceur itself without
> bootstrapping through self compilation first?

Yes. If an engine supports all of ES6 no compilation is needed. (I know we have
cases where derived classes do not call super() for performance reasons.)