Clean up and update const / var

This CL simplifies var / const by ensuring the behavior is consistent in itself, and with regular JS semantics; between regular var/const and eval-ed var/const.

Legacy const is changed so that a declaration declares a configurable, but non-writable, slot, and the initializer reconfigures it (when possible) to non-configurable non-writable. This avoids the need for "the hole" as marker value in JSContextExtensionObjects and GlobalObjects. Undefined is used instead.

BUG=
R=rossberg@chromium.org

Committed: https://code.google.com/p/v8/source/detail?r=22379

[Toon Verwaest, 2014-07-10 14:12:47]

PTAL

[rossberg, 2014-07-11 12:54:49]

https://codereview.chromium.org/379893002/diff/120001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/379893002/diff/120001/src/objects.cc#newcode2354
src/objects.cc:2354: FieldDescriptor d(handle(descriptors->GetKey(modify_index),
isolate),
Hm, how exactly is this related to the rest of the CL?

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2146
src/runtime.cc:2146: return *value;
Maybe have a comment saying what this value is used for downstream, because it's
not easy to see.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2247
src/runtime.cc:2247: // For now, we determine this by checking if the current
value is the hole.
Update comment?

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2267
src/runtime.cc:2267: RUNTIME_FUNCTION(Runtime_DeclareContextSlot) {
Wasn't the plan to rename this to DeclareLookupSlot?

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2271
src/runtime.cc:2271: // Declarations are always made in a function or native
context. In the case
Can also be a global context.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2282
src/runtime.cc:2282: bool is_var = *initial_value == NULL;
Maybe add a TODO to clean this up also.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2343
src/runtime.cc:2343: RUNTIME_FUNCTION(Runtime_InitializeConstContextSlot) {
InitializeLegacyConstLookupSlot?

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2374
src/runtime.cc:2374: // Strict mode handling not needed (const is disallowed in
strict mode).
s/const/legacy const/

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-eval...
File test/mjsunit/const-eval-init.js (right):

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-eval...
test/mjsunit/const-eval-init.js:39: // Initialization turns into assignment to
global 'x'.
Adjust comment (also below)

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-rede...
File test/mjsunit/const-redecl.js (right):

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-rede...
test/mjsunit/const-redecl.js:217: const j = 2;  // This is what makes the
function above throw, because the
Adjust comment

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-rede...
test/mjsunit/const-redecl.js:227: // argument is that "const k;" is equivalent
to "const k = undefined;".
Comment doesn't make sense anymore.

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/regress/re...
File test/mjsunit/regress/regress-global-freeze-const.js (right):

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/regress/re...
test/mjsunit/regress/regress-global-freeze-const.js:7: eval('"use strict"; x =
20;');
Removing the const makes the name of this test obscure. Instead, maybe assert
that the original eval throws now?

[Toon Verwaest, 2014-07-14 07:39:56]

Addressed comments, PTAL again

https://codereview.chromium.org/379893002/diff/120001/src/objects.cc
File src/objects.cc (right):

https://codereview.chromium.org/379893002/diff/120001/src/objects.cc#newcode2354
src/objects.cc:2354: FieldDescriptor d(handle(descriptors->GetKey(modify_index),
isolate),
On 2014/07/11 12:54:48, rossberg wrote:
> Hm, how exactly is this related to the rest of the CL?

Not at all. It's not part of this CL, and an improved version already landed.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2146
src/runtime.cc:2146: return *value;
Not sure what to say here. It's the result of the declaration, which is quite
obvious from the fact that it's the result of the declaration function :)

On 2014/07/11 12:54:48, rossberg wrote:
> Maybe have a comment saying what this value is used for downstream, because
it's
> not easy to see.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2247
src/runtime.cc:2247: // For now, we determine this by checking if the current
value is the hole.
On 2014/07/11 12:54:49, rossberg wrote:
> Update comment?

Done.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2267
src/runtime.cc:2267: RUNTIME_FUNCTION(Runtime_DeclareContextSlot) {
On 2014/07/11 12:54:49, rossberg wrote:
> Wasn't the plan to rename this to DeclareLookupSlot?

Done.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2271
src/runtime.cc:2271: // Declarations are always made in a function or native
context. In the case
On 2014/07/11 12:54:49, rossberg wrote:
> Can also be a global context.

Done.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2282
src/runtime.cc:2282: bool is_var = *initial_value == NULL;
On 2014/07/11 12:54:49, rossberg wrote:
> Maybe add a TODO to clean this up also.

Done.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2343
src/runtime.cc:2343: RUNTIME_FUNCTION(Runtime_InitializeConstContextSlot) {
On 2014/07/11 12:54:49, rossberg wrote:
> InitializeLegacyConstLookupSlot?

Done.

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2374
src/runtime.cc:2374: // Strict mode handling not needed (const is disallowed in
strict mode).
On 2014/07/11 12:54:48, rossberg wrote:
> s/const/legacy const/

Done.

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-eval...
File test/mjsunit/const-eval-init.js (right):

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-eval...
test/mjsunit/const-eval-init.js:39: // Initialization turns into assignment to
global 'x'.
On 2014/07/11 12:54:49, rossberg wrote:
> Adjust comment (also below)

Done.

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-rede...
File test/mjsunit/const-redecl.js (right):

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-rede...
test/mjsunit/const-redecl.js:217: const j = 2;  // This is what makes the
function above throw, because the
On 2014/07/11 12:54:49, rossberg wrote:
> Adjust comment

Done.

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/const-rede...
test/mjsunit/const-redecl.js:227: // argument is that "const k;" is equivalent
to "const k = undefined;".
On 2014/07/11 12:54:49, rossberg wrote:
> Comment doesn't make sense anymore.

Done.

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/regress/re...
File test/mjsunit/regress/regress-global-freeze-const.js (right):

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/regress/re...
test/mjsunit/regress/regress-global-freeze-const.js:7: eval('"use strict"; x =
20;');
Changed the name of the file to indicate its meaning.
I had adjusted the code to ensure it keeps testing what it was supposed to.

On 2014/07/11 12:54:49, rossberg wrote:
> Removing the const makes the name of this test obscure. Instead, maybe assert
> that the original eval throws now?

[rossberg, 2014-07-14 12:04:22]

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc
File src/runtime.cc (right):

https://codereview.chromium.org/379893002/diff/120001/src/runtime.cc#newcode2146
src/runtime.cc:2146: return *value;
On 2014/07/14 07:39:55, Toon Verwaest wrote:
> Not sure what to say here. It's the result of the declaration, which is quite
> obvious from the fact that it's the result of the declaration function :)

But declarations don't have a result (other than undefined), that's why I'm
asking. :)

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/regress/re...
File test/mjsunit/regress/regress-global-freeze-const.js (right):

https://codereview.chromium.org/379893002/diff/120001/test/mjsunit/regress/re...
test/mjsunit/regress/regress-global-freeze-const.js:7: eval('"use strict"; x =
20;');
On 2014/07/14 07:39:56, Toon Verwaest wrote:
> Changed the name of the file to indicate its meaning.
> I had adjusted the code to ensure it keeps testing what it was supposed to.

Hm, I think it really was supposed to test const's behaviour. We changed that
behaviour now, so I think it's more adequate to adjust the test expectations
rather than to test something different. Or have both.

[Toon Verwaest, 2014-07-14 12:16:02]

Returning undefined now.

[rossberg, 2014-07-14 12:23:57]

lgtm

[Toon Verwaest, 2014-07-14 14:01:28]

Committed patchset #3 manually as r22379 (tree was closed).