(early draft) Strict mode - throw exception on assignment to read only property.

Draft of throwing exception in strict mode when assigning to read only property.

All StoreICs and KeyedStoreICs propagate strict mode flag (previous change did it only for globals). The flag then flows into IC miss code and into runtime.

There are plenty of todos so I am mostly looking for quick feedback on the direction:

- Handle strict mode in SetElement (this needs to be done regardless but I believe it can be done safely after all below is handled, in another CL)

- Revert Runtime_SetProperty back to 3-4 arguments, merging the strict flag with PropertyAttributes (optional but I'd find it being cleaner - I used adding the extra argument as means to explicitly track down all places where strict flag needs to go. If I modified the enum directly they'd be hard to find). Ultimately the Strict would be similar to ABSENT flag in PropertyAttributes. Never stored but passed around.

- figure out global variables and constants. I am unclear about the overall design on const in V8 (it is not part of standard and V8 implements const differently than jsc/firefox). Lots of TODOs there.

- Handle all special cases of StoreIC and KeyedStoreIC to correctly propagate strict mode (or disable the particular specialization in strict mode)

BUG=
TEST=

[Martin Maly, 2011-02-24 06:33:34]

early draft...

http://codereview.chromium.org/6576024/diff/1/src/arm/ic-arm.cc
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/6576024/diff/1/src/arm/ic-arm.cc#newcode1418
src/arm/ic-arm.cc:1418: 
Once I merge strict flag into PropertyAttributes this should go back to 3 for
non-strict and 4 for strict.

http://codereview.chromium.org/6576024/diff/1/src/debug.cc
File src/debug.cc (right):

http://codereview.chromium.org/6576024/diff/1/src/debug.cc#newcode841
src/debug.cc:841: false);
Debugger code, I assume we don't need strict mode semantics here.

http://codereview.chromium.org/6576024/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):

http://codereview.chromium.org/6576024/diff/1/src/ia32/full-codegen-ia32.cc#n...
src/ia32/full-codegen-ia32.cc:2114: __ CallRuntime(Runtime::kStoreContextSlot,
4);
function non_strict() {
  with(o) { // o has read-only "foo"
    (function()
      "use strict";
      foo = "value" // must throw.
    })();
  }
}

http://codereview.chromium.org/6576024/diff/1/src/ic.h
File src/ic.h (left):

http://codereview.chromium.org/6576024/diff/1/src/ic.h#oldcode406
src/ic.h:406: 
I'd have to clone this enum for KeyedStoreIC also, and KeyedStoreIC then passes
it to StoreIC in some cases so I figured I'd use StrictModeFlag directly and
delete this enum altogether.

http://codereview.chromium.org/6576024/diff/1/src/runtime.cc
File src/runtime.cc (right):

http://codereview.chromium.org/6576024/diff/1/src/runtime.cc#newcode3805
src/runtime.cc:3805: // TODO(mmaly): SetElement strict mode.
The big todo. SetElement currently does no strict mode-ness.

http://codereview.chromium.org/6576024/diff/1/src/runtime.h
File src/runtime.h (right):

http://codereview.chromium.org/6576024/diff/1/src/runtime.h#newcode244
src/runtime.h:244: F(SetProperty, -1 /* 4 or 5 */, 1) \
Will go back to 3-4 arguments.

http://codereview.chromium.org/6576024/diff/1/src/stub-cache.cc
File src/stub-cache.cc (right):

http://codereview.chromium.org/6576024/diff/1/src/stub-cache.cc#newcode1428
src/stub-cache.cc:1428: MaybeObject* result = recv->SetPropertyWithInterceptor(
This needs to be figured out whether we need to pass strict mode flag here.
Probably yes.

http://codereview.chromium.org/6576024/diff/1/src/stub-cache.h
File src/stub-cache.h (right):

http://codereview.chromium.org/6576024/diff/1/src/stub-cache.h#newcode185
src/stub-cache.h:185: Code::ExtraICState extra_ic_state);
I am leaning toward changing all of these to StrictModeFlag so the purpose is
clearer. Passing ExtraICState seems too cryptic.

http://codereview.chromium.org/6576024/diff/1/test/es5conform/es5conform.status
File test/es5conform/es5conform.status (right):

http://codereview.chromium.org/6576024/diff/1/test/es5conform/es5conform.stat...
test/es5conform/es5conform.status:266: # in strict mode (Global.Infinity)
Global.Infinity is currently not read-only in V8. This differs from jsc/mozilla
but making it r/o interfers with current implementation of global const/var
declaration and causes several sputnik test failures. They may be wrong tests,
haven't analyzed that yet.
I am building the change making Infinity (and others) read/only as a separate
CL.

[Mark Miller, 2011-02-24 06:58:12]

> - figure out global variables and constants. I am unclear 
> about the overall design on const in V8 (it is not part of 
> standard and V8 implements const differently than 
> jsc/firefox). Lots of TODOs there. 

JSC Seems to follow the recommendations at
http://wiki.ecmascript.org/doku.php?id=conventions:no_non_standard_strict_decls
of rejecting non-std declarations including const and nested named function
declarations in strict code:

    > function foo(){ const x = 1; }
    > function foo(){"use strict"; const x = 1; }
    SyntaxError: Parse error

    > function foo(){ {function bar(){}} }
    > function foo(){"use strict"; {function bar(){}} }
    SyntaxError: Parse error

"let"s seem to be rejected unconditionally:

    > function foo(){ let x = 1; }
    SyntaxError: Parse error
    > function foo(){"use strict"; let x = 1; }
    SyntaxError: Parse error

By following this recommendation, we enable Harmony to give these constructs a
decent standard semantics, because the current cacophony of divergent legacy de
facto semantics for these constructs only need be supported in non-strict code.
We avoid creating a legacy of strict code that locks us into continuing these
incoherent semantics.

And following these recommendations is compatible with JSC.

[Lasse Reichstein, 2011-02-24 12:37:54]

I think fixing elements in a separate CL is a good idea.

As for const, I'm not too sure what our implementation is either. It *should* be
a read-only variable, except that it can be overwritten by the first execution
of the assignment in the declaration. I think we have a runtime call for that
assignment, so all normal stores should just fail. I think we store "the hole"
in the variable until the first initialization.

If possible, we should not disable any optimizations in strict mode. We do want
people to use strict mode for their code (if nothing else, to encourage them to
avoid using with and leaky eval).

I've only given this a quick look, but it seems fine so far.

http://codereview.chromium.org/6576024/diff/1/src/arm/ic-arm.cc
File src/arm/ic-arm.cc (right):

http://codereview.chromium.org/6576024/diff/1/src/arm/ic-arm.cc#newcode1418
src/arm/ic-arm.cc:1418: 
While I appreciate saving a parameter, beging strict seems a property of the
assigning code, not the property.
At least, call the parameter something else, like assignmentFlags (which
includes PropertyAttributes and strict-mode flag).

http://codereview.chromium.org/6576024/diff/1/src/debug.cc
File src/debug.cc (right):

http://codereview.chromium.org/6576024/diff/1/src/debug.cc#newcode841
src/debug.cc:841: false);
I agree.

http://codereview.chromium.org/6576024/diff/1/src/stub-cache.cc
File src/stub-cache.cc (right):

http://codereview.chromium.org/6576024/diff/1/src/stub-cache.cc#newcode1428
src/stub-cache.cc:1428: MaybeObject* result = recv->SetPropertyWithInterceptor(
I'm guessing "yes" too.

http://codereview.chromium.org/6576024/diff/1/src/stub-cache.h
File src/stub-cache.h (right):

http://codereview.chromium.org/6576024/diff/1/src/stub-cache.h#newcode185
src/stub-cache.h:185: Code::ExtraICState extra_ic_state);
Agree. If the extra IC state is just the one bit, just use the original bit for
it.

http://codereview.chromium.org/6576024/diff/1/test/es5conform/es5conform.status
File test/es5conform/es5conform.status (right):

http://codereview.chromium.org/6576024/diff/1/test/es5conform/es5conform.stat...
test/es5conform/es5conform.status:266: # in strict mode (Global.Infinity)
Good choice. If the Sputnik tests fail, just disable them. I welcome not being
able to overwrite (at least some) basic language features.