[parser] Make LookupRecursive less recursive

Recursion is really only useful for sloppy eval and with scopes, which are
uncommon.

Change-Id: I2560b600cab9b00a82d5837a3daa28c8d38c2959
Reviewed-on: https://chromium-review.googlesource.com/c/1322451
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Toon Verwaest <verwaest@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57316}
This commit is contained in:
Toon Verwaest 2018-11-07 09:54:03 +01:00 committed by Commit Bot
parent 4001f86afa
commit 5e6f91e7c6
4 changed files with 137 additions and 87 deletions

View File

@ -32,8 +32,7 @@ void Scope::ResolveScopesThenForEachVariable(DeclarationScope* max_outer_scope,
next = proxy->next_unresolved();
DCHECK(!proxy->is_resolved());
Variable* var =
lookup->LookupRecursive(info, proxy, max_outer_scope->outer_scope());
Variable* var = Lookup(info, proxy, lookup, max_outer_scope->outer_scope());
if (var == nullptr) {
variable_proxy_stackvisitor(proxy);
} else if (var != Scope::kDummyPreParserVariable &&

View File

@ -1775,100 +1775,137 @@ Variable* Scope::NonLocal(const AstRawString* name, VariableMode mode) {
return var;
}
Variable* Scope::LookupRecursive(ParseInfo* info, VariableProxy* proxy,
Scope* outer_scope_end) {
DCHECK_NE(outer_scope_end, this);
// Short-cut: whenever we find a debug-evaluate scope, just look everything up
// dynamically. Debug-evaluate doesn't properly create scope info for the
// lookups it does. It may not have a valid 'this' declaration, and anything
// accessed through debug-evaluate might invalidly resolve to stack-allocated
// variables.
// TODO(yangguo): Remove once debug-evaluate creates proper ScopeInfo for the
// scopes in which it's evaluating.
if (is_debug_evaluate_scope_)
return NonLocal(proxy->raw_name(), VariableMode::kDynamic);
// Try to find the variable in this scope.
Variable* var = LookupLocal(proxy->raw_name());
// We found a variable and we are done. (Even if there is an 'eval' in this
// scope which introduces the same variable again, the resulting variable
// remains the same.)
if (var != nullptr) return var;
if (outer_scope_ == outer_scope_end) {
// We may just be trying to find all free variables. In that case, don't
// declare them in the outer scope.
if (!is_script_scope()) return nullptr;
if (proxy->is_private_name()) {
info->pending_error_handler()->ReportMessageAt(
proxy->position(), proxy->position() + 1,
MessageTemplate::kInvalidPrivateFieldAccess, proxy->raw_name(),
kSyntaxError);
return nullptr;
// static
Variable* Scope::Lookup(ParseInfo* info, VariableProxy* proxy, Scope* scope,
Scope* outer_scope_end, bool force_context_allocation) {
while (true) {
DCHECK_NE(outer_scope_end, scope);
// Short-cut: whenever we find a debug-evaluate scope, just look everything
// up dynamically. Debug-evaluate doesn't properly create scope info for the
// lookups it does. It may not have a valid 'this' declaration, and anything
// accessed through debug-evaluate might invalidly resolve to
// stack-allocated variables.
// TODO(yangguo): Remove once debug-evaluate creates proper ScopeInfo for
// the scopes in which it's evaluating.
if (V8_UNLIKELY(scope->is_debug_evaluate_scope_)) {
return scope->NonLocal(proxy->raw_name(), VariableMode::kDynamic);
}
// No binding has been found. Declare a variable on the global object.
return AsDeclarationScope()->DeclareDynamicGlobal(proxy->raw_name(),
NORMAL_VARIABLE);
// Try to find the variable in this scope.
Variable* var = scope->LookupLocal(proxy->raw_name());
// We found a variable and we are done. (Even if there is an 'eval' in this
// scope which introduces the same variable again, the resulting variable
// remains the same.)
if (var != nullptr) {
if (force_context_allocation && !var->is_dynamic()) {
var->ForceContextAllocation();
}
return var;
}
if (scope->outer_scope_ == outer_scope_end) break;
DCHECK(!scope->is_script_scope());
if (V8_UNLIKELY(scope->is_with_scope())) {
return LookupWith(info, proxy, scope, outer_scope_end,
force_context_allocation);
}
if (V8_UNLIKELY(scope->is_declaration_scope() &&
scope->AsDeclarationScope()->calls_sloppy_eval())) {
return LookupSloppyEval(info, proxy, scope, outer_scope_end,
force_context_allocation);
}
force_context_allocation |= scope->is_function_scope();
scope = scope->outer_scope_;
}
DCHECK(!is_script_scope());
// We may just be trying to find all free variables. In that case, don't
// declare them in the outer scope.
// TODO(marja): Separate Lookup for preparsed scopes better.
if (!scope->is_script_scope()) return nullptr;
var = outer_scope_->LookupRecursive(info, proxy, outer_scope_end);
// The variable could not be resolved statically.
if (var == nullptr) return var;
// TODO(marja): Separate LookupRecursive for preparsed scopes better.
if (var == kDummyPreParserVariable || var == kDummyPreParserLexicalVariable) {
DCHECK(GetDeclarationScope()->is_being_lazily_parsed());
return var;
if (V8_UNLIKELY(proxy->is_private_name())) {
info->pending_error_handler()->ReportMessageAt(
proxy->position(), proxy->position() + 1,
MessageTemplate::kInvalidPrivateFieldAccess, proxy->raw_name(),
kSyntaxError);
return nullptr;
}
if (is_function_scope() && !var->is_dynamic()) {
var->ForceContextAllocation();
// No binding has been found. Declare a variable on the global object.
return scope->AsDeclarationScope()->DeclareDynamicGlobal(proxy->raw_name(),
NORMAL_VARIABLE);
}
namespace {
bool CanBeShadowed(Scope* scope, Variable* var) {
if (var == nullptr) return false;
// TODO(marja): Separate Lookup for preparsed scopes better.
if (var == Scope::kDummyPreParserVariable ||
var == Scope::kDummyPreParserLexicalVariable) {
DCHECK(scope->GetDeclarationScope()->is_being_lazily_parsed());
return false;
}
// "this" can't be shadowed by "eval"-introduced bindings or by "with"
// scopes.
// "this" can't be shadowed by "eval"-introduced bindings or by "with" scopes.
// TODO(wingo): There are other variables in this category; add them.
if (var->is_this()) return var;
return !var->is_this();
}
}; // namespace
if (is_with_scope()) {
// 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
// in the outer scope anyway, because if a binding exists in an outer
// scope, the associated variable has to be marked as potentially being
// accessed from inside of an inner with scope (the property may not be in
// the 'with' object).
if (!var->is_dynamic() && var->IsUnallocated()) {
DCHECK(!already_resolved_);
var->set_is_used();
var->ForceContextAllocation();
if (proxy->is_assigned()) var->set_maybe_assigned();
}
return NonLocal(proxy->raw_name(), VariableMode::kDynamic);
Variable* Scope::LookupWith(ParseInfo* info, VariableProxy* proxy, Scope* scope,
Scope* outer_scope_end,
bool force_context_allocation) {
DCHECK(scope->is_with_scope());
Variable* var = Lookup(info, proxy, scope->outer_scope_, outer_scope_end,
force_context_allocation);
if (!CanBeShadowed(scope, var)) return var;
// 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
// in the outer scope anyway, because if a binding exists in an outer
// scope, the associated variable has to be marked as potentially being
// accessed from inside of an inner with scope (the property may not be in
// the 'with' object).
if (!var->is_dynamic() && var->IsUnallocated()) {
DCHECK(!scope->already_resolved_);
var->set_is_used();
var->ForceContextAllocation();
if (proxy->is_assigned()) var->set_maybe_assigned();
}
return scope->NonLocal(proxy->raw_name(), VariableMode::kDynamic);
}
Variable* Scope::LookupSloppyEval(ParseInfo* info, VariableProxy* proxy,
Scope* scope, Scope* outer_scope_end,
bool force_context_allocation) {
DCHECK(scope->is_declaration_scope() &&
scope->AsDeclarationScope()->calls_sloppy_eval());
Variable* var = Lookup(info, proxy, scope->outer_scope_, outer_scope_end,
force_context_allocation);
if (!CanBeShadowed(scope, var)) return var;
// 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). In
// that case, change the lookup result to reflect this situation. Only
// scopes that can host var bindings (declaration scopes) need be considered
// here (this excludes block and catch scopes), and variable lookups at
// script scope are always dynamic.
if (var->IsGlobalObjectProperty()) {
return scope->NonLocal(proxy->raw_name(), VariableMode::kDynamicGlobal);
}
if (is_declaration_scope() && AsDeclarationScope()->calls_sloppy_eval()) {
// 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). In
// that case, change the lookup result to reflect this situation. Only
// scopes that can host var bindings (declaration scopes) need be considered
// here (this excludes block and catch scopes), and variable lookups at
// script scope are always dynamic.
if (var->IsGlobalObjectProperty()) {
return NonLocal(proxy->raw_name(), VariableMode::kDynamicGlobal);
}
if (var->is_dynamic()) return var;
if (var->is_dynamic()) return var;
Variable* invalidated = var;
var = NonLocal(proxy->raw_name(), VariableMode::kDynamicLocal);
var->set_local_if_not_shadowed(invalidated);
}
Variable* invalidated = var;
var = scope->NonLocal(proxy->raw_name(), VariableMode::kDynamicLocal);
var->set_local_if_not_shadowed(invalidated);
return var;
}
@ -1876,7 +1913,7 @@ Variable* Scope::LookupRecursive(ParseInfo* info, VariableProxy* proxy,
bool Scope::ResolveVariable(ParseInfo* info, VariableProxy* proxy) {
DCHECK(info->script_scope()->is_script_scope());
DCHECK(!proxy->is_resolved());
Variable* var = LookupRecursive(info, proxy, nullptr);
Variable* var = Lookup(info, proxy, this, nullptr);
if (var == nullptr) {
DCHECK(proxy->is_private_name());
return false;
@ -1986,7 +2023,7 @@ bool Scope::ResolveVariablesRecursively(ParseInfo* info) {
if (is_declaration_scope() && AsDeclarationScope()->was_lazily_parsed()) {
DCHECK_EQ(variables_.occupancy(), 0);
for (VariableProxy* proxy : unresolved_list_) {
Variable* var = outer_scope()->LookupRecursive(info, proxy, nullptr);
Variable* var = Lookup(info, proxy, outer_scope(), nullptr);
if (var == nullptr) {
DCHECK(proxy->is_private_name());
return false;

View File

@ -592,8 +592,15 @@ class V8_EXPORT_PRIVATE Scope : public NON_EXPORTED_BASE(ZoneObject) {
// scope, and stopping when reaching the outer_scope_end scope. If the code is
// executed because of a call to 'eval', the context parameter should be set
// to the calling context of 'eval'.
Variable* LookupRecursive(ParseInfo* info, VariableProxy* proxy,
Scope* outer_scope_end);
static Variable* Lookup(ParseInfo* info, VariableProxy* proxy, Scope* scope,
Scope* outer_scope_end,
bool force_context_allocation = false);
static Variable* LookupWith(ParseInfo* info, VariableProxy* proxy,
Scope* scope, Scope* outer_scope_end,
bool force_context_allocation);
static Variable* LookupSloppyEval(ParseInfo* info, VariableProxy* proxy,
Scope* scope, Scope* outer_scope_end,
bool force_context_allocation);
void ResolveTo(ParseInfo* info, VariableProxy* proxy, Variable* var);
V8_WARN_UNUSED_RESULT bool ResolveVariable(ParseInfo* info,
VariableProxy* proxy);

View File

@ -0,0 +1,7 @@
// Copyright 2018 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.
function f() { with ({}) { return (()=>this)() } }
var o = {}
assertEquals(o, f.call(o))