From 5cab6be83a44567a3ee5747d728a399025d38411 Mon Sep 17 00:00:00 2001 From: machenbach Date: Thu, 7 May 2015 00:20:43 -0700 Subject: [PATCH] Revert of Resolve references to "this" the same way as normal variables (patchset #2 id:20001 of https://codereview.chromium.org/1130733003/) Reason for revert: [Sheriff] Breaks jetstream benchmark with errors like this: >>> Running suite: JetStream/bigfib.cpp >>> Stdout (#1): undefined:93: ReferenceError: this is not defined this['Module'] = Module; ^ ReferenceError: this is not defined at eval (eval at __run (runner.js:13:3), :93:3) at eval (native) at __run (runner.js:13:3) at Object.runSimpleBenchmark (runner.js:44:31) at runner.js:97:13 Original issue's description: > Resolve references to "this" the same way as normal variables > > Make the parser handle references to "this" as unresolved variables, so the > same logic as for the rest of function parameters is used for the receiver. > Minor additions to the code generation handle copying the receiver to the > context, along with the rest of the function parameters. > > Based on work by Adrian Perez de Castro . > > BUG=v8:2700 > LOG=N > > Committed: https://crrev.com/06a792b7cc2db33ffce7244c044a9c05afbb6116 > Cr-Commit-Position: refs/heads/master@{#28263} TBR=rossberg@chromium.org,arv@chromium.org,wingo@igalia.com NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=v8:2700 Review URL: https://codereview.chromium.org/1129723003 Cr-Commit-Position: refs/heads/master@{#28283} --- src/arm/full-codegen-arm.cc | 12 ++--- src/arm/lithium-codegen-arm.cc | 7 ++- src/arm64/full-codegen-arm64.cc | 12 ++--- src/arm64/lithium-codegen-arm64.cc | 7 ++- src/compiler/ast-graph-builder.cc | 55 ++++--------------- src/compiler/ast-graph-builder.h | 2 +- src/contexts.cc | 9 +--- src/heap/heap.h | 1 - src/ia32/full-codegen-ia32.cc | 9 ++-- src/ia32/lithium-codegen-ia32.cc | 9 ++-- src/mips/full-codegen-mips.cc | 12 ++--- src/mips/lithium-codegen-mips.cc | 9 ++-- src/mips64/full-codegen-mips64.cc | 12 ++--- src/mips64/lithium-codegen-mips64.cc | 9 ++-- src/parser.cc | 23 ++++---- src/runtime/runtime-debug.cc | 21 +++----- src/runtime/runtime-scopes.cc | 20 ++----- src/scopeinfo.cc | 3 +- src/scopes.cc | 66 ++++++++++------------- src/scopes.h | 23 ++------ src/variables.cc | 4 +- src/x64/full-codegen-x64.cc | 12 ++--- src/x64/lithium-codegen-x64.cc | 9 ++-- test/cctest/test-serialize.cc | 2 +- test/mjsunit/debug-scopes.js | 4 -- test/mjsunit/harmony/arrow-functions-this.js | 81 ---------------------------- 26 files changed, 120 insertions(+), 313 deletions(-) delete mode 100644 test/mjsunit/harmony/arrow-functions-this.js diff --git a/src/arm/full-codegen-arm.cc b/src/arm/full-codegen-arm.cc index 35d6917..6f43d17 100644 --- a/src/arm/full-codegen-arm.cc +++ b/src/arm/full-codegen-arm.cc @@ -127,7 +127,7 @@ void FullCodeGenerator::Generate() { // global proxy when called as functions (without an explicit receiver // object). if (is_sloppy(info->language_mode()) && !info->is_native() && - info->MayUseThis() && info->scope()->has_this_declaration()) { + info->MayUseThis()) { Label ok; int receiver_offset = info->scope()->num_parameters() * kPointerSize; __ ldr(r2, MemOperand(sp, receiver_offset)); @@ -216,9 +216,8 @@ void FullCodeGenerator::Generate() { __ str(r0, MemOperand(fp, StandardFrameConstants::kContextOffset)); // Copy any necessary parameters into the context. int num_parameters = info->scope()->num_parameters(); - int first_parameter = info->scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; @@ -3067,9 +3066,8 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { __ ldr(r4, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset)); // r3: the receiver of the enclosing function. - Variable* this_var = scope()->LookupThis(); - DCHECK_NOT_NULL(this_var); - __ ldr(r3, VarOperand(this_var, r3)); + int receiver_offset = 2 + info_->scope()->num_parameters(); + __ ldr(r3, MemOperand(fp, receiver_offset * kPointerSize)); // r2: language mode. __ mov(r2, Operand(Smi::FromInt(language_mode()))); diff --git a/src/arm/lithium-codegen-arm.cc b/src/arm/lithium-codegen-arm.cc index 407bb91..e91fddf 100644 --- a/src/arm/lithium-codegen-arm.cc +++ b/src/arm/lithium-codegen-arm.cc @@ -121,7 +121,7 @@ bool LCodeGen::GeneratePrologue() { // global proxy when called as functions (without an explicit receiver // object). if (is_sloppy(info_->language_mode()) && info()->MayUseThis() && - !info_->is_native() && info_->scope()->has_this_declaration()) { + !info_->is_native()) { Label ok; int receiver_offset = info_->scope()->num_parameters() * kPointerSize; __ ldr(r2, MemOperand(sp, receiver_offset)); @@ -197,9 +197,8 @@ bool LCodeGen::GeneratePrologue() { __ str(r0, MemOperand(fp, StandardFrameConstants::kContextOffset)); // Copy any necessary parameters into the context. int num_parameters = scope()->num_parameters(); - int first_parameter = scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; diff --git a/src/arm64/full-codegen-arm64.cc b/src/arm64/full-codegen-arm64.cc index ab21d02..aee9ddf 100644 --- a/src/arm64/full-codegen-arm64.cc +++ b/src/arm64/full-codegen-arm64.cc @@ -125,7 +125,7 @@ void FullCodeGenerator::Generate() { // global proxy when called as functions (without an explicit receiver // object). if (is_sloppy(info->language_mode()) && !info->is_native() && - info->MayUseThis() && info->scope()->has_this_declaration()) { + info->MayUseThis()) { Label ok; int receiver_offset = info->scope()->num_parameters() * kXRegSize; __ Peek(x10, receiver_offset); @@ -217,9 +217,8 @@ void FullCodeGenerator::Generate() { __ Str(x0, MemOperand(fp, StandardFrameConstants::kContextOffset)); // Copy any necessary parameters into the context. int num_parameters = info->scope()->num_parameters(); - int first_parameter = info->scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; @@ -2752,9 +2751,8 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { __ Ldr(x10, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset)); // Prepare to push the receiver of the enclosing function. - Variable* this_var = scope()->LookupThis(); - DCHECK_NOT_NULL(this_var); - __ Ldr(x11, VarOperand(this_var, x11)); + int receiver_offset = 2 + info_->scope()->num_parameters(); + __ Ldr(x11, MemOperand(fp, receiver_offset * kPointerSize)); // Prepare to push the language mode. __ Mov(x12, Smi::FromInt(language_mode())); diff --git a/src/arm64/lithium-codegen-arm64.cc b/src/arm64/lithium-codegen-arm64.cc index ea259d5..c2a2ff3 100644 --- a/src/arm64/lithium-codegen-arm64.cc +++ b/src/arm64/lithium-codegen-arm64.cc @@ -669,7 +669,7 @@ bool LCodeGen::GeneratePrologue() { // global proxy when called as functions (without an explicit receiver // object). if (is_sloppy(info_->language_mode()) && info()->MayUseThis() && - !info()->is_native() && info()->scope()->has_this_declaration()) { + !info_->is_native()) { Label ok; int receiver_offset = info_->scope()->num_parameters() * kXRegSize; __ Peek(x10, receiver_offset); @@ -728,9 +728,8 @@ bool LCodeGen::GeneratePrologue() { __ Str(x0, MemOperand(fp, StandardFrameConstants::kContextOffset)); // Copy any necessary parameters into the context. int num_parameters = scope()->num_parameters(); - int first_parameter = scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { Register value = x0; Register scratch = x3; diff --git a/src/compiler/ast-graph-builder.cc b/src/compiler/ast-graph-builder.cc index 26983d6..7125f0b 100644 --- a/src/compiler/ast-graph-builder.cc +++ b/src/compiler/ast-graph-builder.cc @@ -470,21 +470,15 @@ bool AstGraphBuilder::CreateGraph(bool constant_context, bool stack_check) { // Build receiver check for sloppy mode if necessary. // TODO(mstarzinger/verwaest): Should this be moved back into the CallIC? - Node* patched_receiver = nullptr; - if (scope->has_this_declaration()) { - Node* original_receiver = NewNode(common()->Parameter(0), graph()->start()); - patched_receiver = BuildPatchReceiverToGlobalProxy(original_receiver); - if (scope->receiver()->IsStackAllocated()) { - env.Bind(scope->receiver(), patched_receiver); - } - } + Node* original_receiver = env.Lookup(scope->receiver()); + Node* patched_receiver = BuildPatchReceiverToGlobalProxy(original_receiver); + env.Bind(scope->receiver(), patched_receiver); // Build function context only if there are context allocated variables. int heap_slots = info()->num_heap_slots() - Context::MIN_CONTEXT_SLOTS; if (heap_slots > 0) { // Push a new inner context scope for the function. - Node* inner_context = - BuildLocalFunctionContext(function_context_.get(), patched_receiver); + Node* inner_context = BuildLocalFunctionContext(function_context_.get()); ContextScope top_context(this, scope, inner_context); CreateGraphBody(stack_check); } else { @@ -2173,23 +2167,7 @@ void AstGraphBuilder::VisitCall(Call* expr) { // Create node to ask for help resolving potential eval call. This will // provide a fully resolved callee and the corresponding receiver. Node* function = GetFunctionClosure(); - // TODO(wingo): ResolvePossibleDirectEval doesn't really need a receiver, - // now that eval scopes don't have "this" declarations. Remove this hack - // once ResolvePossibleDirectEval changes. - Node* receiver; - { - Variable* variable = info()->scope()->LookupThis(); - if (variable->IsStackAllocated()) { - receiver = environment()->Lookup(variable); - } else { - DCHECK(variable->IsContextSlot()); - int depth = current_scope()->ContextChainLength(variable->scope()); - bool immutable = variable->maybe_assigned() == kNotAssigned; - const Operator* op = - javascript()->LoadContext(depth, variable->index(), immutable); - receiver = NewNode(op, current_context()); - } - } + Node* receiver = environment()->Lookup(info()->scope()->receiver()); Node* language = jsgraph()->Constant(language_mode()); Node* position = jsgraph()->Constant(info()->scope()->start_position()); const Operator* op = @@ -2678,36 +2656,25 @@ Node* AstGraphBuilder::BuildPatchReceiverToGlobalProxy(Node* receiver) { } -Node* AstGraphBuilder::BuildLocalFunctionContext(Node* context, - Node* patched_receiver) { - Scope* scope = info()->scope(); +Node* AstGraphBuilder::BuildLocalFunctionContext(Node* context) { Node* closure = GetFunctionClosure(); // Allocate a new local context. Node* local_context = - scope->is_script_scope() - ? BuildLocalScriptContext(scope) + info()->scope()->is_script_scope() + ? BuildLocalScriptContext(info()->scope()) : NewNode(javascript()->CreateFunctionContext(), closure); - if (scope->has_this_declaration() && scope->receiver()->IsContextSlot()) { - DCHECK_NOT_NULL(patched_receiver); - // Context variable (at bottom of the context chain). - Variable* variable = scope->receiver(); - DCHECK_EQ(0, scope->ContextChainLength(variable->scope())); - const Operator* op = javascript()->StoreContext(0, variable->index()); - NewNode(op, local_context, patched_receiver); - } - // Copy parameters into context if necessary. - int num_parameters = scope->num_parameters(); + int num_parameters = info()->scope()->num_parameters(); for (int i = 0; i < num_parameters; i++) { - Variable* variable = scope->parameter(i); + Variable* variable = info()->scope()->parameter(i); if (!variable->IsContextSlot()) continue; // Temporary parameter node. The parameter indices are shifted by 1 // (receiver is parameter index -1 but environment index 0). Node* parameter = NewNode(common()->Parameter(i + 1), graph()->start()); // Context variable (at bottom of the context chain). - DCHECK_EQ(0, scope->ContextChainLength(variable->scope())); + DCHECK_EQ(0, info()->scope()->ContextChainLength(variable->scope())); const Operator* op = javascript()->StoreContext(0, variable->index()); NewNode(op, local_context, parameter); } diff --git a/src/compiler/ast-graph-builder.h b/src/compiler/ast-graph-builder.h index f9d4472..74728dd 100644 --- a/src/compiler/ast-graph-builder.h +++ b/src/compiler/ast-graph-builder.h @@ -246,7 +246,7 @@ class AstGraphBuilder : public AstVisitor { Node* BuildPatchReceiverToGlobalProxy(Node* receiver); // Builders to create local function, script and block contexts. - Node* BuildLocalFunctionContext(Node* context, Node* patched_receiver); + Node* BuildLocalFunctionContext(Node* context); Node* BuildLocalScriptContext(Scope* scope); Node* BuildLocalBlockContext(Scope* scope); diff --git a/src/contexts.cc b/src/contexts.cc index 12eb871..a6a61df 100644 --- a/src/contexts.cc +++ b/src/contexts.cc @@ -258,13 +258,8 @@ Handle Context::Lookup(Handle name, object->IsJSContextExtensionObject()) { maybe = JSReceiver::GetOwnPropertyAttributes(object, name); } else if (context->IsWithContext()) { - // A with context will never bind "this". - if (name->Equals(*isolate->factory()->this_string())) { - maybe = Just(ABSENT); - } else { - LookupIterator it(object, name); - maybe = UnscopableLookup(&it); - } + LookupIterator it(object, name); + maybe = UnscopableLookup(&it); } else { maybe = JSReceiver::GetPropertyAttributes(object, name); } diff --git a/src/heap/heap.h b/src/heap/heap.h index 09585ab..c4c8068 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -231,7 +231,6 @@ namespace internal { V(source_string, "source") \ V(source_url_string, "source_url") \ V(source_mapping_url_string, "source_mapping_url") \ - V(this_string, "this") \ V(global_string, "global") \ V(ignore_case_string, "ignoreCase") \ V(multiline_string, "multiline") \ diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc index 234e3b3..6ef215e 100644 --- a/src/ia32/full-codegen-ia32.cc +++ b/src/ia32/full-codegen-ia32.cc @@ -210,9 +210,8 @@ void FullCodeGenerator::Generate() { // Copy parameters into context if necessary. int num_parameters = info->scope()->num_parameters(); - int first_parameter = info->scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; @@ -2960,9 +2959,7 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { // Push the enclosing function. __ push(Operand(ebp, JavaScriptFrameConstants::kFunctionOffset)); // Push the receiver of the enclosing function. - Variable* this_var = scope()->LookupThis(); - DCHECK_NOT_NULL(this_var); - __ push(VarOperand(this_var, ecx)); + __ push(Operand(ebp, (2 + info_->scope()->num_parameters()) * kPointerSize)); // Push the language mode. __ push(Immediate(Smi::FromInt(language_mode()))); diff --git a/src/ia32/lithium-codegen-ia32.cc b/src/ia32/lithium-codegen-ia32.cc index addce60..3f1bab7 100644 --- a/src/ia32/lithium-codegen-ia32.cc +++ b/src/ia32/lithium-codegen-ia32.cc @@ -141,8 +141,8 @@ bool LCodeGen::GeneratePrologue() { // Sloppy mode functions and builtins need to replace the receiver with the // global proxy when called as functions (without an explicit receiver // object). - if (is_sloppy(info()->language_mode()) && info()->MayUseThis() && - !info()->is_native() && info()->scope()->has_this_declaration()) { + if (is_sloppy(info_->language_mode()) && info()->MayUseThis() && + !info_->is_native()) { Label ok; // +1 for return address. int receiver_offset = (scope()->num_parameters() + 1) * kPointerSize; @@ -275,9 +275,8 @@ bool LCodeGen::GeneratePrologue() { // Copy parameters into context if necessary. int num_parameters = scope()->num_parameters(); - int first_parameter = scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; diff --git a/src/mips/full-codegen-mips.cc b/src/mips/full-codegen-mips.cc index 69d0011..5ac885a 100644 --- a/src/mips/full-codegen-mips.cc +++ b/src/mips/full-codegen-mips.cc @@ -135,7 +135,7 @@ void FullCodeGenerator::Generate() { // global proxy when called as functions (without an explicit receiver // object). if (is_sloppy(info->language_mode()) && !info->is_native() && - info->MayUseThis() && info->scope()->has_this_declaration()) { + info->MayUseThis()) { Label ok; int receiver_offset = info->scope()->num_parameters() * kPointerSize; __ lw(at, MemOperand(sp, receiver_offset)); @@ -225,9 +225,8 @@ void FullCodeGenerator::Generate() { __ sw(v0, MemOperand(fp, StandardFrameConstants::kContextOffset)); // Copy any necessary parameters into the context. int num_parameters = info->scope()->num_parameters(); - int first_parameter = info->scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; @@ -3038,9 +3037,8 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { __ lw(t2, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset)); // t1: the receiver of the enclosing function. - Variable* this_var = scope()->LookupThis(); - DCHECK_NOT_NULL(this_var); - __ lw(t1, VarOperand(this_var, t1)); + int receiver_offset = 2 + info_->scope()->num_parameters(); + __ lw(t1, MemOperand(fp, receiver_offset * kPointerSize)); // t0: the language mode. __ li(t0, Operand(Smi::FromInt(language_mode()))); diff --git a/src/mips/lithium-codegen-mips.cc b/src/mips/lithium-codegen-mips.cc index 9c66316..b173e30 100644 --- a/src/mips/lithium-codegen-mips.cc +++ b/src/mips/lithium-codegen-mips.cc @@ -143,8 +143,8 @@ bool LCodeGen::GeneratePrologue() { // Sloppy mode functions and builtins need to replace the receiver with the // global proxy when called as functions (without an explicit receiver // object). - if (is_sloppy(info()->language_mode()) && info()->MayUseThis() && - !info()->is_native() && info()->scope()->has_this_declaration()) { + if (is_sloppy(info_->language_mode()) && info()->MayUseThis() && + !info_->is_native()) { Label ok; int receiver_offset = info_->scope()->num_parameters() * kPointerSize; __ LoadRoot(at, Heap::kUndefinedValueRootIndex); @@ -217,9 +217,8 @@ bool LCodeGen::GeneratePrologue() { __ sw(v0, MemOperand(fp, StandardFrameConstants::kContextOffset)); // Copy any necessary parameters into the context. int num_parameters = scope()->num_parameters(); - int first_parameter = scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; diff --git a/src/mips64/full-codegen-mips64.cc b/src/mips64/full-codegen-mips64.cc index aa39c42..d2eb9dc 100644 --- a/src/mips64/full-codegen-mips64.cc +++ b/src/mips64/full-codegen-mips64.cc @@ -135,7 +135,7 @@ void FullCodeGenerator::Generate() { // global proxy when called as functions (without an explicit receiver // object). if (is_sloppy(info->language_mode()) && !info->is_native() && - info->MayUseThis() && info->scope()->has_this_declaration()) { + info->MayUseThis()) { Label ok; int receiver_offset = info->scope()->num_parameters() * kPointerSize; __ ld(at, MemOperand(sp, receiver_offset)); @@ -222,9 +222,8 @@ void FullCodeGenerator::Generate() { __ sd(v0, MemOperand(fp, StandardFrameConstants::kContextOffset)); // Copy any necessary parameters into the context. int num_parameters = info->scope()->num_parameters(); - int first_parameter = info->scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; @@ -3040,9 +3039,8 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { __ ld(a6, MemOperand(fp, JavaScriptFrameConstants::kFunctionOffset)); // a5: the receiver of the enclosing function. - Variable* this_var = scope()->LookupThis(); - DCHECK_NOT_NULL(this_var); - __ ld(a5, VarOperand(this_var, a5)); + int receiver_offset = 2 + info_->scope()->num_parameters(); + __ ld(a5, MemOperand(fp, receiver_offset * kPointerSize)); // a4: the language mode. __ li(a4, Operand(Smi::FromInt(language_mode()))); diff --git a/src/mips64/lithium-codegen-mips64.cc b/src/mips64/lithium-codegen-mips64.cc index e8f7234..223370f 100644 --- a/src/mips64/lithium-codegen-mips64.cc +++ b/src/mips64/lithium-codegen-mips64.cc @@ -118,8 +118,8 @@ bool LCodeGen::GeneratePrologue() { // Sloppy mode functions and builtins need to replace the receiver with the // global proxy when called as functions (without an explicit receiver // object). - if (is_sloppy(info()->language_mode()) && info()->MayUseThis() && - !info()->is_native() && info()->scope()->has_this_declaration()) { + if (is_sloppy(info_->language_mode()) && info()->MayUseThis() && + !info_->is_native()) { Label ok; int receiver_offset = info_->scope()->num_parameters() * kPointerSize; __ LoadRoot(at, Heap::kUndefinedValueRootIndex); @@ -192,9 +192,8 @@ bool LCodeGen::GeneratePrologue() { __ sd(v0, MemOperand(fp, StandardFrameConstants::kContextOffset)); // Copy any necessary parameters into the context. int num_parameters = scope()->num_parameters(); - int first_parameter = scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; diff --git a/src/parser.cc b/src/parser.cc index 5e3db12..3b537d2 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -740,9 +740,7 @@ const AstRawString* ParserTraits::GetNextSymbol(Scanner* scanner) { Expression* ParserTraits::ThisExpression(Scope* scope, AstNodeFactory* factory, int pos) { - return scope->NewUnresolved(factory, - parser_->ast_value_factory()->this_string(), - Variable::THIS, pos, pos + 4); + return factory->NewVariableProxy(scope->receiver(), pos); } Expression* ParserTraits::SuperReference(Scope* scope, AstNodeFactory* factory, @@ -790,8 +788,7 @@ Expression* ParserTraits::ExpressionFromIdentifier(const AstRawString* name, Scope* scope, AstNodeFactory* factory) { if (parser_->fni_ != NULL) parser_->fni_->PushVariableName(name); - return scope->NewUnresolved(factory, name, Variable::NORMAL, start_position, - end_position); + return scope->NewUnresolved(factory, name, start_position, end_position); } @@ -970,8 +967,6 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) { FunctionLiteral* result = NULL; { - // TODO(wingo): Add an outer GLOBAL_SCOPE corresponding to the native - // context, which will have the "this" binding for script scopes. Scope* scope = NewScope(scope_, SCRIPT_SCOPE); info->set_script_scope(scope); if (!info->context().is_null() && !info->context()->IsNativeContext()) { @@ -1924,9 +1919,9 @@ VariableProxy* Parser::NewUnresolved(const AstRawString* name, // scope. // Let/const variables in harmony mode are always added to the immediately // enclosing scope. - return DeclarationScope(mode)->NewUnresolved( - factory(), name, Variable::NORMAL, scanner()->location().beg_pos, - scanner()->location().end_pos); + return DeclarationScope(mode)->NewUnresolved(factory(), name, + scanner()->location().beg_pos, + scanner()->location().end_pos); } @@ -3590,8 +3585,8 @@ Statement* Parser::ParseForStatement(ZoneList* labels, Expression* enumerable = ParseExpression(true, CHECK_OK); Expect(Token::RPAREN, CHECK_OK); - VariableProxy* each = scope_->NewUnresolved( - factory(), name, Variable::NORMAL, each_beg_pos, each_end_pos); + VariableProxy* each = + scope_->NewUnresolved(factory(), name, each_beg_pos, each_end_pos); Statement* body = ParseSubStatement(NULL, CHECK_OK); InitializeForEachStatement(loop, each, enumerable, body); Block* result = @@ -3672,8 +3667,8 @@ Statement* Parser::ParseForStatement(ZoneList* labels, scope_ = for_scope; Expect(Token::RPAREN, CHECK_OK); - VariableProxy* each = scope_->NewUnresolved( - factory(), name, Variable::NORMAL, each_beg_pos, each_end_pos); + VariableProxy* each = + scope_->NewUnresolved(factory(), name, each_beg_pos, each_end_pos); Statement* body = ParseSubStatement(NULL, CHECK_OK); Block* body_block = factory()->NewBlock(NULL, 3, false, RelocInfo::kNoPosition); diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc index 591c865..239f2b2 100644 --- a/src/runtime/runtime-debug.cc +++ b/src/runtime/runtime-debug.cc @@ -1298,21 +1298,16 @@ class ScopeIterator { context_ = Handle(); return; } - if (scope_type == ScopeTypeScript) { - seen_script_scope_ = true; - if (context_->IsScriptContext()) { + if (scope_type == ScopeTypeScript) seen_script_scope_ = true; + if (nested_scope_chain_.is_empty()) { + if (scope_type == ScopeTypeScript) { + if (context_->IsScriptContext()) { + context_ = Handle(context_->previous(), isolate_); + } + CHECK(context_->IsNativeContext()); + } else { context_ = Handle(context_->previous(), isolate_); } - if (!nested_scope_chain_.is_empty()) { - DCHECK_EQ(nested_scope_chain_.last()->scope_type(), SCRIPT_SCOPE); - nested_scope_chain_.RemoveLast(); - DCHECK(nested_scope_chain_.is_empty()); - } - CHECK(context_->IsNativeContext()); - return; - } - if (nested_scope_chain_.is_empty()) { - context_ = Handle(context_->previous(), isolate_); } else { if (nested_scope_chain_.last()->HasContext()) { DCHECK(context_->previous() != NULL); diff --git a/src/runtime/runtime-scopes.cc b/src/runtime/runtime-scopes.cc index 1de2c4c..7a523a7 100644 --- a/src/runtime/runtime-scopes.cc +++ b/src/runtime/runtime-scopes.cc @@ -250,12 +250,6 @@ RUNTIME_FUNCTION(Runtime_DeclareLookupSlot) { JSGlobalObject::cast(context_arg->extension()), isolate); return DeclareGlobals(isolate, global, name, value, attr, is_var, is_const, is_function); - } else if (context->IsScriptContext()) { - DCHECK(context->global_object()->IsJSGlobalObject()); - Handle global( - JSGlobalObject::cast(context->global_object()), isolate); - return DeclareGlobals(isolate, global, name, value, attr, is_var, is_const, - is_function); } if (attributes != ABSENT) { @@ -331,12 +325,8 @@ RUNTIME_FUNCTION(Runtime_InitializeLegacyConstLookupSlot) { // meanwhile. If so, re-introduce the variable in the context extension. if (attributes == ABSENT) { Handle declaration_context(context_arg->declaration_context()); - if (declaration_context->IsScriptContext()) { - holder = handle(declaration_context->global_object(), isolate); - } else { - DCHECK(declaration_context->has_extension()); - holder = handle(declaration_context->extension(), isolate); - } + DCHECK(declaration_context->has_extension()); + holder = handle(declaration_context->extension(), isolate); CHECK(holder->IsJSObject()); } else { // For JSContextExtensionObjects, the initializer can be run multiple times @@ -640,12 +630,8 @@ RUNTIME_FUNCTION(Runtime_NewScriptContext) { FindNameClash(scope_info, global_object, script_context_table); if (isolate->has_pending_exception()) return name_clash_result; - // Script contexts have a canonical empty function as their closure, not the - // anonymous closure containing the global code. See - // FullCodeGenerator::PushFunctionArgumentForContextAllocation. - Handle closure(native_context->closure()); Handle result = - isolate->factory()->NewScriptContext(closure, scope_info); + isolate->factory()->NewScriptContext(function, scope_info); DCHECK(function->context() == isolate->context()); DCHECK(function->context()->global_object() == result->global_object()); diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc index 4b079b9..0127449 100644 --- a/src/scopeinfo.cc +++ b/src/scopeinfo.cc @@ -317,8 +317,7 @@ bool ScopeInfo::LocalIsSynthetic(int var) { // with user declarations, the current temporaries like .generator_object and // .result start with a dot, so we can use that as a flag. It's a hack! Handle name(LocalName(var)); - return (name->length() > 0 && name->Get(0) == '.') || - name->Equals(*GetIsolate()->factory()->this_string()); + return name->length() > 0 && name->Get(0) == '.'; } diff --git a/src/scopes.cc b/src/scopes.cc index a5af8c0..542a3d3 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -305,16 +305,22 @@ void Scope::Initialize() { scope_inside_with_ = is_with_scope(); } - // Declare convenience variables and the receiver. + // Declare convenience variables. + // Declare and allocate receiver (even for the script scope, and even + // if naccesses_ == 0). + // NOTE: When loading parameters in the script scope, we must take + // care not to access them as properties of the global object, but + // instead load them directly from the stack. Currently, the only + // such parameter is 'this' which is passed on the stack when + // invoking scripts if (is_declaration_scope()) { DCHECK(!subclass_constructor || is_function_scope()); - if (has_this_declaration()) { - Variable* var = variables_.Declare( - this, ast_value_factory_->this_string(), - subclass_constructor ? CONST : VAR, Variable::THIS, - subclass_constructor ? kNeedsInitialization : kCreatedInitialized); - receiver_ = var; - } + Variable* var = variables_.Declare( + this, ast_value_factory_->this_string(), + subclass_constructor ? CONST : VAR, Variable::THIS, + subclass_constructor ? kNeedsInitialization : kCreatedInitialized); + var->AllocateTo(Variable::PARAMETER, -1); + receiver_ = var; if (subclass_constructor) { new_target_ = @@ -323,6 +329,9 @@ void Scope::Initialize() { new_target_->AllocateTo(Variable::PARAMETER, -2); new_target_->set_is_used(); } + } else { + DCHECK(outer_scope() != NULL); + receiver_ = outer_scope()->receiver(); } if (is_function_scope() && !is_arrow_scope()) { @@ -1028,7 +1037,7 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy, DCHECK(is_script_scope()); } - if (is_with_scope() && (var == nullptr || !var->is_this())) { + if (is_with_scope()) { DCHECK(!already_resolved()); // The current scope is a with scope, so the variable binding can not be // statically resolved. However, note that it was necessary to do a lookup @@ -1388,40 +1397,24 @@ void Scope::AllocateParameterLocals(Isolate* isolate) { // Force context allocation of the parameter. var->ForceContextAllocation(); } - AllocateParameter(var, i); - } -} - -void Scope::AllocateParameter(Variable* var, int index) { - if (MustAllocate(var)) { - if (MustAllocateInContext(var)) { - DCHECK(var->IsUnallocated() || var->IsContextSlot()); - if (var->IsUnallocated()) { - AllocateHeapSlot(var); - } - } else { - DCHECK(var->IsUnallocated() || var->IsParameter()); - if (var->IsUnallocated()) { - var->AllocateTo(Variable::PARAMETER, index); + if (MustAllocate(var)) { + if (MustAllocateInContext(var)) { + DCHECK(var->IsUnallocated() || var->IsContextSlot()); + if (var->IsUnallocated()) { + AllocateHeapSlot(var); + } + } else { + DCHECK(var->IsUnallocated() || var->IsParameter()); + if (var->IsUnallocated()) { + var->AllocateTo(Variable::PARAMETER, i); + } } } } } -void Scope::AllocateReceiver() { - DCHECK_NOT_NULL(receiver()); - DCHECK_EQ(receiver()->scope(), this); - - if (has_forced_context_allocation()) { - // Force context allocation of the receiver. - receiver()->ForceContextAllocation(); - } - AllocateParameter(receiver(), -1); -} - - void Scope::AllocateNonParameterLocal(Isolate* isolate, Variable* var) { DCHECK(var->scope() == this); DCHECK(!var->IsVariable(isolate->factory()->dot_result_string()) || @@ -1491,7 +1484,6 @@ void Scope::AllocateVariablesRecursively(Isolate* isolate) { // Allocate variables for this scope. // Parameters must be allocated first, if any. if (is_function_scope()) AllocateParameterLocals(isolate); - if (has_this_declaration()) AllocateReceiver(); AllocateNonParameterLocals(isolate); // Force allocation of a context for this scope if necessary. For a 'with' diff --git a/src/scopes.h b/src/scopes.h index 7a013e9..b8025cc 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -144,15 +144,14 @@ class Scope: public ZoneObject { // Create a new unresolved variable. VariableProxy* NewUnresolved(AstNodeFactory* factory, const AstRawString* name, - Variable::Kind kind = Variable::NORMAL, int start_position = RelocInfo::kNoPosition, int end_position = RelocInfo::kNoPosition) { // Note that we must not share the unresolved variables with // the same name because they may be removed selectively via // RemoveUnresolved(). DCHECK(!already_resolved()); - VariableProxy* proxy = - factory->NewVariableProxy(name, kind, start_position, end_position); + VariableProxy* proxy = factory->NewVariableProxy( + name, Variable::NORMAL, start_position, end_position); unresolved_.Add(proxy, zone_); return proxy; } @@ -333,21 +332,7 @@ class Scope: public ZoneObject { LanguageMode language_mode() const { return language_mode_; } // The variable corresponding to the 'this' value. - Variable* receiver() { - DCHECK(has_this_declaration()); - DCHECK_NOT_NULL(receiver_); - return receiver_; - } - - Variable* LookupThis() { return Lookup(ast_value_factory_->this_string()); } - - // TODO(wingo): Add a GLOBAL_SCOPE scope type which will lexically allocate - // "this" (and no other variable) on the native context. Script scopes then - // will not have a "this" declaration. - bool has_this_declaration() const { - return (is_function_scope() && !is_arrow_scope()) || is_module_scope() || - is_script_scope(); - } + Variable* receiver() { return receiver_; } // The variable corresponding to the 'new.target' value. Variable* new_target_var() { return new_target_; } @@ -701,8 +686,6 @@ class Scope: public ZoneObject { void AllocateNonParameterLocal(Isolate* isolate, Variable* var); void AllocateNonParameterLocals(Isolate* isolate); void AllocateVariablesRecursively(Isolate* isolate); - void AllocateParameter(Variable* var, int index); - void AllocateReceiver(); void AllocateModules(); // Resolve and fill in the allocation information for all variables diff --git a/src/variables.cc b/src/variables.cc index 0c94d75..c0b8bd7 100644 --- a/src/variables.cc +++ b/src/variables.cc @@ -59,8 +59,8 @@ bool Variable::IsGlobalObjectProperty() const { // Temporaries are never global, they must always be allocated in the // activation frame. return (IsDynamicVariableMode(mode_) || - (IsDeclaredVariableMode(mode_) && !IsLexicalVariableMode(mode_))) && - scope_ != NULL && scope_->is_script_scope() && !is_this(); + (IsDeclaredVariableMode(mode_) && !IsLexicalVariableMode(mode_))) + && scope_ != NULL && scope_->is_script_scope(); } diff --git a/src/x64/full-codegen-x64.cc b/src/x64/full-codegen-x64.cc index b09d29b..cdd5707 100644 --- a/src/x64/full-codegen-x64.cc +++ b/src/x64/full-codegen-x64.cc @@ -115,7 +115,7 @@ void FullCodeGenerator::Generate() { // global proxy when called as functions (without an explicit receiver // object). if (is_sloppy(info->language_mode()) && !info->is_native() && - info->MayUseThis() && info->scope()->has_this_declaration()) { + info->MayUseThis()) { Label ok; // +1 for return address. StackArgumentsAccessor args(rsp, info->scope()->num_parameters()); @@ -209,9 +209,8 @@ void FullCodeGenerator::Generate() { // Copy any necessary parameters into the context. int num_parameters = info->scope()->num_parameters(); - int first_parameter = info->scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; @@ -2961,9 +2960,8 @@ void FullCodeGenerator::EmitResolvePossiblyDirectEval(int arg_count) { __ Push(Operand(rbp, JavaScriptFrameConstants::kFunctionOffset)); // Push the receiver of the enclosing function and do runtime call. - Variable* this_var = scope()->LookupThis(); - DCHECK_NOT_NULL(this_var); - __ Push(VarOperand(this_var, rcx)); + StackArgumentsAccessor args(rbp, info_->scope()->num_parameters()); + __ Push(args.GetReceiverOperand()); // Push the language mode. __ Push(Smi::FromInt(language_mode())); diff --git a/src/x64/lithium-codegen-x64.cc b/src/x64/lithium-codegen-x64.cc index 80b3267..eb8274b 100644 --- a/src/x64/lithium-codegen-x64.cc +++ b/src/x64/lithium-codegen-x64.cc @@ -129,8 +129,8 @@ bool LCodeGen::GeneratePrologue() { // Sloppy mode functions need to replace the receiver with the global proxy // when called as functions (without an explicit receiver object). - if (is_sloppy(info()->language_mode()) && info()->MayUseThis() && - !info()->is_native() && info()->scope()->has_this_declaration()) { + if (is_sloppy(info_->language_mode()) && info()->MayUseThis() && + !info_->is_native()) { Label ok; StackArgumentsAccessor args(rsp, scope()->num_parameters()); __ movp(rcx, args.GetReceiverOperand()); @@ -213,9 +213,8 @@ bool LCodeGen::GeneratePrologue() { // Copy any necessary parameters into the context. int num_parameters = scope()->num_parameters(); - int first_parameter = scope()->has_this_declaration() ? -1 : 0; - for (int i = first_parameter; i < num_parameters; i++) { - Variable* var = (i == -1) ? scope()->receiver() : scope()->parameter(i); + for (int i = 0; i < num_parameters; i++) { + Variable* var = scope()->parameter(i); if (var->IsContextSlot()) { int parameter_offset = StandardFrameConstants::kCallerSPOffset + (num_parameters - 1 - i) * kPointerSize; diff --git a/test/cctest/test-serialize.cc b/test/cctest/test-serialize.cc index 72eb194..1ed43ac 100644 --- a/test/cctest/test-serialize.cc +++ b/test/cctest/test-serialize.cc @@ -628,7 +628,7 @@ UNINITIALIZED_DEPENDENT_TEST(CustomContextDeserialization, root = deserializer.DeserializePartial(isolate, global_proxy, &outdated_contexts).ToHandleChecked(); - CHECK_EQ(3, outdated_contexts->length()); + CHECK_EQ(2, outdated_contexts->length()); CHECK(root->IsContext()); Handle context = Handle::cast(root); CHECK(context->global_proxy() == *global_proxy); diff --git a/test/mjsunit/debug-scopes.js b/test/mjsunit/debug-scopes.js index 1e1e22a..ad137de 100644 --- a/test/mjsunit/debug-scopes.js +++ b/test/mjsunit/debug-scopes.js @@ -167,10 +167,6 @@ function CheckScopeContent(content, number, exec_state) { if (!scope.scopeObject().property('arguments').isUndefined()) { scope_size--; } - // Ditto for 'this'. - if (!scope.scopeObject().property('this').isUndefined()) { - scope_size--; - } // Skip property with empty name. if (!scope.scopeObject().property('').isUndefined()) { scope_size--; diff --git a/test/mjsunit/harmony/arrow-functions-this.js b/test/mjsunit/harmony/arrow-functions-this.js deleted file mode 100644 index 2c2ad07..0000000 --- a/test/mjsunit/harmony/arrow-functions-this.js +++ /dev/null @@ -1,81 +0,0 @@ -// Copyright 2015 the V8 project authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -// Flags: --harmony-arrow-functions - -var object = {}; -var global = this; -var call = Function.call.bind(Function.call); - -var globalSloppyArrow = () => this; -var globalStrictArrow = () => { "use strict"; return this; }; -var globalSloppyArrowEval = (s) => eval(s); -var globalStrictArrowEval = (s) => { "use strict"; return eval(s); }; - -var sloppyFunctionArrow = function() { - return (() => this)(); -}; -var strictFunctionArrow = function() { - "use strict"; - return (() => this)(); -}; -var sloppyFunctionEvalArrow = function() { - return eval("(() => this)()"); -}; -var strictFunctionEvalArrow = function() { - "use strict"; - return eval("(() => this)()"); -}; -var sloppyFunctionArrowEval = function(s) { - return (() => eval(s))(); -}; -var strictFunctionArrowEval = function(s) { - "use strict"; - return (() => eval(s))(); -}; - -var withObject = { 'this': object } -var arrowInsideWith, arrowInsideWithEval; -with (withObject) { - arrowInsideWith = () => this; - arrowInsideWithEval = (s) => eval(s); -} - -assertEquals(global, call(globalSloppyArrow, object)); -assertEquals(global, call(globalStrictArrow, object)); -assertEquals(global, call(globalSloppyArrowEval, object, "this")); -assertEquals(global, call(globalStrictArrowEval, object, "this")); -assertEquals(global, call(globalSloppyArrowEval, object, "(() => this)()")); -assertEquals(global, call(globalStrictArrowEval, object, "(() => this)()")); - -assertEquals(object, call(sloppyFunctionArrow, object)); -assertEquals(global, call(sloppyFunctionArrow, undefined)); -assertEquals(object, call(strictFunctionArrow, object)); -assertEquals(undefined, call(strictFunctionArrow, undefined)); - -assertEquals(object, call(sloppyFunctionEvalArrow, object)); -assertEquals(global, call(sloppyFunctionEvalArrow, undefined)); -assertEquals(object, call(strictFunctionEvalArrow, object)); -assertEquals(undefined, call(strictFunctionEvalArrow, undefined)); - -assertEquals(object, call(sloppyFunctionArrowEval, object, "this")); -assertEquals(global, call(sloppyFunctionArrowEval, undefined, "this")); -assertEquals(object, call(strictFunctionArrowEval, object, "this")); -assertEquals(undefined, call(strictFunctionArrowEval, undefined, "this")); - -assertEquals(object, - call(sloppyFunctionArrowEval, object, "(() => this)()")); -assertEquals(global, - call(sloppyFunctionArrowEval, undefined, "(() => this)()")); -assertEquals(object, - call(strictFunctionArrowEval, object, "(() => this)()")); -assertEquals(undefined, - call(strictFunctionArrowEval, undefined, "(() => this)()")); - -assertEquals(global, call(arrowInsideWith, undefined)); -assertEquals(global, call(arrowInsideWith, object)); -assertEquals(global, call(arrowInsideWithEval, undefined, "this")); -assertEquals(global, call(arrowInsideWithEval, object, "this")); -assertEquals(global, call(arrowInsideWithEval, undefined, "(() => this)()")); -assertEquals(global, call(arrowInsideWithEval, object, "(() => this)()")); -- 2.7.4