Commit 2f820780 authored by Maya Lekova's avatar Maya Lekova Committed by Commit Bot

Revert "[parser] Fix caching dynamic vars on wrong scope"

This reverts commit 304e97d3.

Reason for revert: Last roll is failing - https://ci.chromium.org/p/chromium/builders/try/linux-rel/282356

Original change's description:
> [parser] Fix caching dynamic vars on wrong scope
> 
> When looking up a variable in a deserialized WITH scope, we were
> unconditionally passing in the cache scope to the lookup, even if the
> with was inside the cache scope. This would lead to and outer scope of
> the with holding the generated dynamic variable. If the cache scope was
> the SCRIPT scope, the dynamic variable would be interpreted as a global
> object property.
> 
> Now, we only store the WITH scope dynamic variables in the cache scope
> if it is an inner scope of the WITH scope, same as we do for 'normal'
> scope lookups.
> 
> Fixed: chromium:1041210
> Change-Id: I4e8eb25bbb8ea58311355d13a9c7c97bf2fa3ec7
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1997135
> Reviewed-by: Toon Verwaest <verwaest@chromium.org>
> Commit-Queue: Toon Verwaest <verwaest@chromium.org>
> Auto-Submit: Leszek Swirski <leszeks@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#65732}

TBR=leszeks@chromium.org,verwaest@chromium.org

Change-Id: I7b6d77d03b603152a9a47541db466934f46b1176
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2000140Reviewed-by: 's avatarMaya Lekova <mslekova@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65747}
parent 75eab984
......@@ -7,7 +7,6 @@
#include <set>
#include "src/ast/ast.h"
#include "src/base/logging.h"
#include "src/base/optional.h"
#include "src/builtins/accessors.h"
#include "src/common/message-template.h"
......@@ -1900,9 +1899,8 @@ template <Scope::ScopeLookupMode mode>
Variable* Scope::Lookup(VariableProxy* proxy, Scope* scope,
Scope* outer_scope_end, Scope* cache_scope,
bool force_context_allocation) {
// If we have already passed the cache scope in earlier recursions, we should
// first quickly check if the current scope uses the cache scope before
// continuing.
// If we have already passed it in earlier recursions though, so we should
// first quickly check if it is an outer scope before continuing.
if (mode == kDeserializedScope &&
scope->deserialized_scope_uses_external_cache()) {
Variable* var = cache_scope->variables_.Lookup(proxy->raw_name());
......@@ -1920,17 +1918,13 @@ Variable* Scope::Lookup(VariableProxy* proxy, Scope* scope,
// the scopes in which it's evaluating.
if (mode == kDeserializedScope &&
V8_UNLIKELY(scope->is_debug_evaluate_scope_)) {
DCHECK(scope->deserialized_scope_uses_external_cache() ||
scope == cache_scope);
return cache_scope->NonLocal(proxy->raw_name(), VariableMode::kDynamic);
}
// Try to find the variable in this scope.
Variable* var;
Scope* current_cache_scope = nullptr;
if (mode == kParsedScope) {
var = scope->LookupLocal(proxy->raw_name());
DCHECK_NULL(cache_scope);
} else {
DCHECK_EQ(mode, kDeserializedScope);
bool external_cache = scope->deserialized_scope_uses_external_cache();
......@@ -1942,8 +1936,8 @@ Variable* Scope::Lookup(VariableProxy* proxy, Scope* scope,
Variable* var = scope->variables_.Lookup(proxy->raw_name());
if (var != nullptr) return var;
}
current_cache_scope = external_cache ? cache_scope : scope;
var = scope->LookupInScopeInfo(proxy->raw_name(), current_cache_scope);
var = scope->LookupInScopeInfo(proxy->raw_name(),
external_cache ? cache_scope : scope);
}
// We found a variable and we are done. (Even if there is an 'eval' in this
......@@ -1972,14 +1966,14 @@ Variable* Scope::Lookup(VariableProxy* proxy, Scope* scope,
DCHECK(!scope->is_script_scope());
if (V8_UNLIKELY(scope->is_with_scope())) {
return LookupWith(proxy, scope, outer_scope_end, current_cache_scope,
return LookupWith(proxy, scope, outer_scope_end, cache_scope,
force_context_allocation);
}
if (V8_UNLIKELY(
scope->is_declaration_scope() &&
scope->AsDeclarationScope()->sloppy_eval_can_extend_vars())) {
return LookupSloppyEval(proxy, scope, outer_scope_end,
current_cache_scope, force_context_allocation);
return LookupSloppyEval(proxy, scope, outer_scope_end, cache_scope,
force_context_allocation);
}
force_context_allocation |= scope->is_function_scope();
......
// Copyright 2020 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: --stress-lazy-source-positions
with ({}) {
(() => eval())();
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment