Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(180)

Unified Diff: src/scopes.cc

Issue 1140633003: Reapply "Resolve references to "this" the same way as normal variables"" (Closed) Base URL: https://chromium.googlesource.com/v8/v8@master
Patch Set: ScopeInfo records slot of "this" binding; formatting Created 5 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/scopes.h ('k') | src/variables.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index b3c163ce503a70a980b72882aca86b3f24566a18..60c14dbdc8d752af1cb209897c97eb7d944f553e 100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -165,7 +165,6 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
scope_calls_eval_ = false;
scope_uses_arguments_ = false;
scope_uses_super_property_ = false;
- scope_uses_this_ = false;
asm_module_ = false;
asm_function_ = outer_scope != NULL && outer_scope->asm_module_;
// Inherit the language mode from the parent scope.
@@ -173,7 +172,6 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
outer_scope_calls_sloppy_eval_ = false;
inner_scope_calls_eval_ = false;
inner_scope_uses_arguments_ = false;
- inner_scope_uses_this_ = false;
inner_scope_uses_super_property_ = false;
force_eager_compilation_ = false;
force_context_allocation_ = (outer_scope != NULL && !is_function_scope())
@@ -309,22 +307,16 @@ void Scope::Initialize() {
scope_inside_with_ = is_with_scope();
}
- // 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
+ // Declare convenience variables and the receiver.
if (is_declaration_scope()) {
DCHECK(!subclass_constructor || is_function_scope());
- 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 (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;
+ }
if (subclass_constructor) {
new_target_ =
@@ -333,9 +325,6 @@ 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()) {
@@ -380,7 +369,6 @@ Scope* Scope::FinalizeBlockScope() {
// Propagate usage flags to outer scope.
if (uses_arguments()) outer_scope_->RecordArgumentsUsage();
if (uses_super_property()) outer_scope_->RecordSuperPropertyUsage();
- if (uses_this()) outer_scope_->RecordThisUsage();
if (scope_calls_eval_) outer_scope_->RecordEvalCall();
return NULL;
@@ -421,9 +409,16 @@ Variable* Scope::LookupLocal(const AstRawString* name) {
maybe_assigned_flag = kMaybeAssigned;
}
- // TODO(marja, rossberg): Declare variables of the right Kind.
- Variable* var = variables_.Declare(this, name, mode, Variable::NORMAL,
- init_flag, maybe_assigned_flag);
+ Variable::Kind kind = Variable::NORMAL;
+ if (location == Variable::CONTEXT &&
+ index == scope_info_->ReceiverContextSlotIndex()) {
+ kind = Variable::THIS;
+ }
+ // TODO(marja, rossberg): Correctly declare FUNCTION, CLASS, NEW_TARGET, and
+ // ARGUMENTS bindings as their corresponding Variable::Kind.
+
+ Variable* var = variables_.Declare(this, name, mode, kind, init_flag,
+ maybe_assigned_flag);
var->AllocateTo(location, index);
return var;
}
@@ -931,13 +926,11 @@ void Scope::Print(int n) {
if (scope_uses_arguments_) Indent(n1, "// scope uses 'arguments'\n");
if (scope_uses_super_property_)
Indent(n1, "// scope uses 'super' property\n");
- if (scope_uses_this_) Indent(n1, "// scope uses 'this'\n");
if (inner_scope_uses_arguments_) {
Indent(n1, "// inner scope uses 'arguments'\n");
}
if (inner_scope_uses_super_property_)
Indent(n1, "// inner scope uses 'super' property\n");
- if (inner_scope_uses_this_) Indent(n1, "// inner scope uses 'this'\n");
if (outer_scope_calls_sloppy_eval_) {
Indent(n1, "// outer scope calls 'eval' in sloppy context\n");
}
@@ -1050,7 +1043,11 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy,
DCHECK(is_script_scope());
}
- if (is_with_scope()) {
+ // "this" can't be shadowed by "eval"-introduced bindings or by "with" scopes.
+ // TODO(wingo): There are other variables in this category; add them.
+ bool name_can_be_shadowed = var == nullptr || !var->is_this();
+
+ if (is_with_scope() && name_can_be_shadowed) {
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
@@ -1061,7 +1058,7 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy,
if (var != NULL && proxy->is_assigned()) var->set_maybe_assigned();
*binding_kind = DYNAMIC_LOOKUP;
return NULL;
- } else if (calls_sloppy_eval()) {
+ } else if (calls_sloppy_eval() && name_can_be_shadowed) {
// A variable binding may have been found in an outer scope, but the current
// scope makes a sloppy 'eval' call, so the found variable may not be
// the correct one (the 'eval' may introduce a binding with the same name).
@@ -1294,9 +1291,6 @@ void Scope::PropagateScopeInfo(bool outer_scope_calls_sloppy_eval ) {
inner->inner_scope_uses_super_property_) {
inner_scope_uses_super_property_ = true;
}
- if (inner->scope_uses_this_ || inner->inner_scope_uses_this_) {
- inner_scope_uses_this_ = true;
- }
}
if (inner->force_eager_compilation_) {
force_eager_compilation_ = true;
@@ -1417,24 +1411,40 @@ void Scope::AllocateParameterLocals(Isolate* isolate) {
// Force context allocation of the parameter.
var->ForceContextAllocation();
}
+ AllocateParameter(var, i);
+ }
+}
- 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::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);
}
}
}
}
+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()) ||
@@ -1504,6 +1514,7 @@ 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'
« no previous file with comments | « src/scopes.h ('k') | src/variables.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698