Commit cbc6adc8 authored by adamk's avatar adamk Committed by Commit bot

[cleanup] Remove dead code from DeclareLookupSlot and rename it

Runtime_DeclareLookupSlot is used when generating code for var and function declarations
originating in an eval. Over time, it's accumulated quite a bit of cruft, which this CL removes:

  - With legacy const gone, lookup slots never have any property attributes.
  - There was a bit signaling that the variable was from an eval, but that was redundant since
    DeclareLookupSlot is only used for eval.
  - Some Proxy-related code didn't make sense here.

Its name was also not terribly clear: while "LookupSlot" is used in several places, this
particular function is only used for declaring variables and functions inside sloppy eval.
Renamed (and split into two) to make this clear for future archeologists.

Also added various DCHECKs to check the assumptions being made.

Review-Url: https://codereview.chromium.org/2061173002
Cr-Commit-Position: refs/heads/master@{#37111}
parent 4257fdea
......@@ -40,7 +40,6 @@ Variable::Variable(Scope* scope, const AstRawString* name, VariableMode mode,
index_(-1),
initializer_position_(RelocInfo::kNoPosition),
local_if_not_shadowed_(NULL),
is_from_eval_(false),
force_context_allocation_(false),
is_used_(false),
initialization_flag_(initialization_flag),
......
......@@ -119,21 +119,8 @@ class Variable: public ZoneObject {
index_ = index;
}
void SetFromEval() { is_from_eval_ = true; }
static int CompareIndex(Variable* const* v, Variable* const* w);
PropertyAttributes DeclarationPropertyAttributes() const {
int property_attributes = NONE;
if (IsImmutableVariableMode(mode_)) {
property_attributes |= READ_ONLY;
}
if (is_from_eval_) {
property_attributes |= EVAL_DECLARED;
}
return static_cast<PropertyAttributes>(property_attributes);
}
private:
Scope* scope_;
const AstRawString* name_;
......@@ -149,9 +136,6 @@ class Variable: public ZoneObject {
// binding scope (exclusive).
Variable* local_if_not_shadowed_;
// True if this variable is introduced by a sloppy eval
bool is_from_eval_;
// Usage info.
bool force_context_allocation_; // set by variable resolver
bool is_used_;
......
......@@ -1134,12 +1134,8 @@ void AstGraphBuilder::VisitVariableDeclaration(VariableDeclaration* decl) {
case VariableLocation::LOOKUP: {
DCHECK(!hole_init);
Node* name = jsgraph()->Constant(variable->name());
Node* value = jsgraph()->ZeroConstant(); // Indicates no initial value.
Node* attr =
jsgraph()->Constant(variable->DeclarationPropertyAttributes());
const Operator* op =
javascript()->CallRuntime(Runtime::kDeclareLookupSlot);
Node* store = NewNode(op, name, value, attr);
const Operator* op = javascript()->CallRuntime(Runtime::kDeclareEvalVar);
Node* store = NewNode(op, name);
PrepareFrameState(store, decl->proxy()->id());
break;
}
......@@ -1178,11 +1174,9 @@ void AstGraphBuilder::VisitFunctionDeclaration(FunctionDeclaration* decl) {
VisitForValue(decl->fun());
Node* value = environment()->Pop();
Node* name = jsgraph()->Constant(variable->name());
Node* attr =
jsgraph()->Constant(variable->DeclarationPropertyAttributes());
const Operator* op =
javascript()->CallRuntime(Runtime::kDeclareLookupSlot);
Node* store = NewNode(op, name, value, attr);
javascript()->CallRuntime(Runtime::kDeclareEvalFunction);
Node* store = NewNode(op, name, value);
PrepareFrameState(store, decl->proxy()->id());
break;
}
......
......@@ -801,14 +801,11 @@ void FullCodeGenerator::VisitVariableDeclaration(
case VariableLocation::LOOKUP: {
Comment cmnt(masm_, "[ VariableDeclaration");
__ mov(r2, Operand(variable->name()));
// Declaration nodes are always introduced in one of four modes.
DCHECK(IsDeclaredVariableMode(mode));
DCHECK_EQ(VAR, mode);
DCHECK(!hole_init);
__ mov(r0, Operand(Smi::FromInt(0))); // Indicates no initial value.
__ Push(r2, r0);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot);
__ mov(r2, Operand(variable->name()));
__ Push(r2);
__ CallRuntime(Runtime::kDeclareEvalVar);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......@@ -865,8 +862,7 @@ void FullCodeGenerator::VisitFunctionDeclaration(
PushOperand(r2);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
PushOperand(Smi::FromInt(variable->DeclarationPropertyAttributes()));
CallRuntimeWithOperands(Runtime::kDeclareLookupSlot);
CallRuntimeWithOperands(Runtime::kDeclareEvalFunction);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......
......@@ -798,14 +798,11 @@ void FullCodeGenerator::VisitVariableDeclaration(
case VariableLocation::LOOKUP: {
Comment cmnt(masm_, "[ VariableDeclaration");
__ Mov(x2, Operand(variable->name()));
// Declaration nodes are always introduced in one of four modes.
DCHECK(IsDeclaredVariableMode(mode));
DCHECK_EQ(VAR, mode);
DCHECK(!hole_init);
// Pushing 0 (xzr) indicates no initial value.
__ Push(x2, xzr);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot);
__ Mov(x2, Operand(variable->name()));
__ Push(x2);
__ CallRuntime(Runtime::kDeclareEvalVar);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......@@ -862,8 +859,7 @@ void FullCodeGenerator::VisitFunctionDeclaration(
PushOperand(x2);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
PushOperand(Smi::FromInt(variable->DeclarationPropertyAttributes()));
CallRuntimeWithOperands(Runtime::kDeclareLookupSlot);
CallRuntimeWithOperands(Runtime::kDeclareEvalFunction);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......
......@@ -748,14 +748,10 @@ void FullCodeGenerator::VisitVariableDeclaration(
case VariableLocation::LOOKUP: {
Comment cmnt(masm_, "[ VariableDeclaration");
__ push(Immediate(variable->name()));
// VariableDeclaration nodes are always introduced in one of four modes.
DCHECK(IsDeclaredVariableMode(mode));
DCHECK_EQ(VAR, mode);
DCHECK(!hole_init);
__ push(Immediate(Smi::FromInt(0))); // Indicates no initial value.
__ push(
Immediate(Smi::FromInt(variable->DeclarationPropertyAttributes())));
__ CallRuntime(Runtime::kDeclareLookupSlot);
__ push(Immediate(variable->name()));
__ CallRuntime(Runtime::kDeclareEvalVar);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......@@ -808,8 +804,7 @@ void FullCodeGenerator::VisitFunctionDeclaration(
Comment cmnt(masm_, "[ FunctionDeclaration");
PushOperand(variable->name());
VisitForStackValue(declaration->fun());
PushOperand(Smi::FromInt(variable->DeclarationPropertyAttributes()));
CallRuntimeWithOperands(Runtime::kDeclareLookupSlot);
CallRuntimeWithOperands(Runtime::kDeclareEvalFunction);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......
......@@ -798,15 +798,11 @@ void FullCodeGenerator::VisitVariableDeclaration(
case VariableLocation::LOOKUP: {
Comment cmnt(masm_, "[ VariableDeclaration");
__ li(a2, Operand(variable->name()));
// Declaration nodes are always introduced in one of four modes.
DCHECK(IsDeclaredVariableMode(mode));
DCHECK_EQ(VAR, mode);
DCHECK(!hole_init);
DCHECK(Smi::FromInt(0) == 0);
__ mov(a0, zero_reg); // Smi::FromInt(0) indicates no initial value.
__ Push(a2, a0);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot);
__ li(a2, Operand(variable->name()));
__ Push(a2);
__ CallRuntime(Runtime::kDeclareEvalVar);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......@@ -863,8 +859,7 @@ void FullCodeGenerator::VisitFunctionDeclaration(
PushOperand(a2);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
PushOperand(Smi::FromInt(variable->DeclarationPropertyAttributes()));
CallRuntimeWithOperands(Runtime::kDeclareLookupSlot);
CallRuntimeWithOperands(Runtime::kDeclareEvalFunction);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......
......@@ -797,15 +797,11 @@ void FullCodeGenerator::VisitVariableDeclaration(
case VariableLocation::LOOKUP: {
Comment cmnt(masm_, "[ VariableDeclaration");
__ li(a2, Operand(variable->name()));
// Declaration nodes are always introduced in one of four modes.
DCHECK(IsDeclaredVariableMode(mode));
DCHECK_EQ(VAR, mode);
DCHECK(!hole_init);
DCHECK(Smi::FromInt(0) == 0);
__ mov(a0, zero_reg); // Smi::FromInt(0) indicates no initial value.
__ Push(a2, a0);
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot);
__ li(a2, Operand(variable->name()));
__ Push(a2);
__ CallRuntime(Runtime::kDeclareEvalVar);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......@@ -862,8 +858,7 @@ void FullCodeGenerator::VisitFunctionDeclaration(
PushOperand(a2);
// Push initial value for function declaration.
VisitForStackValue(declaration->fun());
PushOperand(Smi::FromInt(variable->DeclarationPropertyAttributes()));
CallRuntimeWithOperands(Runtime::kDeclareLookupSlot);
CallRuntimeWithOperands(Runtime::kDeclareEvalFunction);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......
......@@ -760,13 +760,10 @@ void FullCodeGenerator::VisitVariableDeclaration(
case VariableLocation::LOOKUP: {
Comment cmnt(masm_, "[ VariableDeclaration");
__ Push(variable->name());
// Declaration nodes are always introduced in one of four modes.
DCHECK(IsDeclaredVariableMode(mode));
DCHECK_EQ(VAR, mode);
DCHECK(!hole_init);
__ Push(Smi::FromInt(0)); // Indicates no initial value.
__ Push(Smi::FromInt(variable->DeclarationPropertyAttributes()));
__ CallRuntime(Runtime::kDeclareLookupSlot);
__ Push(variable->name());
__ CallRuntime(Runtime::kDeclareEvalVar);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......@@ -820,8 +817,7 @@ void FullCodeGenerator::VisitFunctionDeclaration(
Comment cmnt(masm_, "[ FunctionDeclaration");
PushOperand(variable->name());
VisitForStackValue(declaration->fun());
PushOperand(Smi::FromInt(variable->DeclarationPropertyAttributes()));
CallRuntimeWithOperands(Runtime::kDeclareLookupSlot);
CallRuntimeWithOperands(Runtime::kDeclareEvalFunction);
PrepareForBailoutForId(proxy->id(), BailoutState::NO_REGISTERS);
break;
}
......
......@@ -757,22 +757,15 @@ void BytecodeGenerator::VisitVariableDeclaration(VariableDeclaration* decl) {
}
break;
case VariableLocation::LOOKUP: {
DCHECK(IsDeclaredVariableMode(mode));
DCHECK_EQ(VAR, mode);
DCHECK(!hole_init);
register_allocator()->PrepareForConsecutiveAllocations(3);
Register name = register_allocator()->NextConsecutiveRegister();
Register init_value = register_allocator()->NextConsecutiveRegister();
Register attributes = register_allocator()->NextConsecutiveRegister();
Register name = register_allocator()->NewRegister();
builder()->LoadLiteral(variable->name()).StoreAccumulatorInRegister(name);
builder()
->LoadLiteral(Smi::FromInt(0))
.StoreAccumulatorInRegister(init_value);
builder()
->LoadLiteral(Smi::FromInt(variable->DeclarationPropertyAttributes()))
.StoreAccumulatorInRegister(attributes)
.CallRuntime(Runtime::kDeclareLookupSlot, name, 3);
->LoadLiteral(variable->name())
.StoreAccumulatorInRegister(name)
.CallRuntime(Runtime::kDeclareEvalVar, name, 1);
break;
}
}
......@@ -808,18 +801,14 @@ void BytecodeGenerator::VisitFunctionDeclaration(FunctionDeclaration* decl) {
break;
}
case VariableLocation::LOOKUP: {
register_allocator()->PrepareForConsecutiveAllocations(3);
register_allocator()->PrepareForConsecutiveAllocations(2);
Register name = register_allocator()->NextConsecutiveRegister();
Register literal = register_allocator()->NextConsecutiveRegister();
Register attributes = register_allocator()->NextConsecutiveRegister();
builder()->LoadLiteral(variable->name()).StoreAccumulatorInRegister(name);
VisitForAccumulatorValue(decl->fun());
builder()
->StoreAccumulatorInRegister(literal)
.LoadLiteral(Smi::FromInt(variable->DeclarationPropertyAttributes()))
.StoreAccumulatorInRegister(attributes)
.CallRuntime(Runtime::kDeclareLookupSlot, name, 3);
builder()->StoreAccumulatorInRegister(literal).CallRuntime(
Runtime::kDeclareEvalFunction, name, 2);
}
}
}
......
......@@ -2036,13 +2036,12 @@ Variable* Parser::Declare(Declaration* declaration,
// In a var binding in a sloppy direct eval, pollute the enclosing scope
// with this new binding by doing the following:
// The proxy is bound to a lookup variable to force a dynamic declaration
// using the DeclareLookupSlot runtime function.
// using the DeclareEvalVar or DeclareEvalFunction runtime functions.
Variable::Kind kind = Variable::NORMAL;
// TODO(sigurds) figure out if kNotAssigned is OK here
var = new (zone()) Variable(declaration_scope, name, mode, kind,
declaration->initialization(), kNotAssigned);
var->AllocateTo(VariableLocation::LOOKUP, -1);
var->SetFromEval();
resolve = true;
}
......@@ -2062,7 +2061,7 @@ Variable* Parser::Declare(Declaration* declaration,
// same variable if it is declared several times. This is not a
// semantic issue as long as we keep the source order, but it may be
// a performance issue since it may lead to repeated
// RuntimeHidden_DeclareLookupSlot calls.
// DeclareEvalVar or DeclareEvalFunction calls.
declaration_scope->AddDeclaration(declaration);
// If requested and we have a local variable, bind the proxy to the variable
......
......@@ -28,11 +28,6 @@ enum PropertyAttributes {
// ABSENT can never be stored in or returned from a descriptor's attributes
// bitfield. It is only used as a return value meaning the attributes of
// a non-existent property.
// When creating a property, EVAL_DECLARED used to indicate that the property
// came from a sloppy-mode direct eval, and certain checks need to be done.
// Cannot be stored in or returned from a descriptor's attributes bitfield.
EVAL_DECLARED = 128
};
......
......@@ -188,74 +188,60 @@ RUNTIME_FUNCTION(Runtime_InitializeConstGlobal) {
return *value;
}
namespace {
Object* DeclareLookupSlot(Isolate* isolate, Handle<String> name,
Handle<Object> initial_value,
PropertyAttributes attr) {
// Declarations are always made in a function, eval or script context, or
// a declaration block scope.
// In the case of eval code, the context passed is the context of the caller,
// which may be some nested context and not the declaration context.
Object* DeclareEvalHelper(Isolate* isolate, Handle<String> name,
Handle<Object> value) {
// Declarations are always made in a function, native, or script context, or
// a declaration block scope. Since this is called from eval, the context
// passed is the context of the caller, which may be some nested context and
// not the declaration context.
Handle<Context> context_arg(isolate->context(), isolate);
Handle<Context> context(context_arg->declaration_context(), isolate);
// TODO(verwaest): Unify the encoding indicating "var" with DeclareGlobals.
bool is_var = *initial_value == NULL;
bool is_function = initial_value->IsJSFunction();
DCHECK_EQ(1, BoolToInt(is_var) + BoolToInt(is_function));
DCHECK(context->IsFunctionContext() || context->IsNativeContext() ||
context->IsScriptContext() ||
(context->IsBlockContext() && context->has_extension()));
bool is_function = value->IsJSFunction();
bool is_var = !is_function;
DCHECK(!is_var || value->IsUndefined(isolate));
int index;
PropertyAttributes attributes;
BindingFlags binding_flags;
if ((attr & EVAL_DECLARED) != 0) {
// Check for a conflict with a lexically scoped variable
context_arg->Lookup(name, LEXICAL_TEST, &index, &attributes,
&binding_flags);
if (attributes != ABSENT && binding_flags == BINDING_CHECK_INITIALIZED) {
return ThrowRedeclarationError(isolate, name);
}
attr = static_cast<PropertyAttributes>(attr & ~EVAL_DECLARED);
// Check for a conflict with a lexically scoped variable
context_arg->Lookup(name, LEXICAL_TEST, &index, &attributes, &binding_flags);
if (attributes != ABSENT && binding_flags == BINDING_CHECK_INITIALIZED) {
return ThrowRedeclarationError(isolate, name);
}
Handle<Object> holder = context->Lookup(name, DONT_FOLLOW_CHAINS, &index,
&attributes, &binding_flags);
if (holder.is_null()) {
// In case of JSProxy, an exception might have been thrown.
if (isolate->has_pending_exception()) return isolate->heap()->exception();
}
DCHECK(!isolate->has_pending_exception());
Handle<JSObject> object;
Handle<Object> value =
is_function ? initial_value
: Handle<Object>::cast(isolate->factory()->undefined_value());
// TODO(verwaest): This case should probably not be covered by this function,
// but by DeclareGlobals instead.
if (attributes != ABSENT && holder->IsJSGlobalObject()) {
return DeclareGlobals(isolate, Handle<JSGlobalObject>::cast(holder), name,
value, attr, is_var, is_function);
value, NONE, is_var, is_function);
}
if (context_arg->extension()->IsJSGlobalObject()) {
Handle<JSGlobalObject> global(
JSGlobalObject::cast(context_arg->extension()), isolate);
return DeclareGlobals(isolate, global, name, value, attr, is_var,
return DeclareGlobals(isolate, global, name, value, NONE, is_var,
is_function);
} else if (context->IsScriptContext()) {
DCHECK(context->global_object()->IsJSGlobalObject());
Handle<JSGlobalObject> global(
JSGlobalObject::cast(context->global_object()), isolate);
return DeclareGlobals(isolate, global, name, value, attr, is_var,
return DeclareGlobals(isolate, global, name, value, NONE, is_var,
is_function);
}
if (attributes != ABSENT) {
// The name was declared before; check for conflicting re-declarations.
if ((attributes & READ_ONLY) != 0) {
return ThrowRedeclarationError(isolate, name);
}
DCHECK_EQ(NONE, attributes);
// Skip var re-declarations.
if (is_var) return isolate->heap()->undefined_value();
......@@ -263,7 +249,7 @@ Object* DeclareLookupSlot(Isolate* isolate, Handle<String> name,
DCHECK(is_function);
if (index != Context::kNotFound) {
DCHECK(holder.is_identical_to(context));
context->set(index, *initial_value);
context->set(index, *value);
return isolate->heap()->undefined_value();
}
......@@ -292,26 +278,28 @@ Object* DeclareLookupSlot(Isolate* isolate, Handle<String> name,
}
RETURN_FAILURE_ON_EXCEPTION(isolate, JSObject::SetOwnPropertyIgnoreAttributes(
object, name, value, attr));
object, name, value, NONE));
return isolate->heap()->undefined_value();
}
} // namespace
RUNTIME_FUNCTION(Runtime_DeclareLookupSlot) {
RUNTIME_FUNCTION(Runtime_DeclareEvalFunction) {
HandleScope scope(isolate);
DCHECK_EQ(3, args.length());
DCHECK_EQ(2, args.length());
CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
CONVERT_ARG_HANDLE_CHECKED(Object, initial_value, 1);
CONVERT_ARG_HANDLE_CHECKED(Smi, property_attributes, 2);
PropertyAttributes attributes =
static_cast<PropertyAttributes>(property_attributes->value());
return DeclareLookupSlot(isolate, name, initial_value, attributes);
CONVERT_ARG_HANDLE_CHECKED(Object, value, 1);
return DeclareEvalHelper(isolate, name, value);
}
RUNTIME_FUNCTION(Runtime_DeclareEvalVar) {
HandleScope scope(isolate);
DCHECK_EQ(1, args.length());
CONVERT_ARG_HANDLE_CHECKED(String, name, 0);
return DeclareEvalHelper(isolate, name,
isolate->factory()->undefined_value());
}
namespace {
......
......@@ -473,30 +473,31 @@ namespace internal {
F(RegExpExecReThrow, 4, 1) \
F(IsRegExp, 1, 1)
#define FOR_EACH_INTRINSIC_SCOPES(F) \
F(ThrowConstAssignError, 0, 1) \
F(DeclareGlobals, 2, 1) \
F(InitializeVarGlobal, 3, 1) \
F(InitializeConstGlobal, 2, 1) \
F(DeclareLookupSlot, 3, 1) \
F(NewSloppyArguments_Generic, 1, 1) \
F(NewStrictArguments, 1, 1) \
F(NewRestParameter, 1, 1) \
F(NewSloppyArguments, 3, 1) \
F(NewClosure, 1, 1) \
F(NewClosure_Tenured, 1, 1) \
F(NewScriptContext, 2, 1) \
F(NewFunctionContext, 1, 1) \
F(PushWithContext, 2, 1) \
F(PushCatchContext, 3, 1) \
F(PushBlockContext, 2, 1) \
F(IsJSModule, 1, 1) \
F(PushModuleContext, 2, 1) \
F(DeclareModules, 1, 1) \
F(DeleteLookupSlot, 1, 1) \
F(LoadLookupSlot, 1, 1) \
F(LoadLookupSlotInsideTypeof, 1, 1) \
F(StoreLookupSlot_Sloppy, 2, 1) \
#define FOR_EACH_INTRINSIC_SCOPES(F) \
F(ThrowConstAssignError, 0, 1) \
F(DeclareGlobals, 2, 1) \
F(InitializeVarGlobal, 3, 1) \
F(InitializeConstGlobal, 2, 1) \
F(DeclareEvalFunction, 2, 1) \
F(DeclareEvalVar, 1, 1) \
F(NewSloppyArguments_Generic, 1, 1) \
F(NewStrictArguments, 1, 1) \
F(NewRestParameter, 1, 1) \
F(NewSloppyArguments, 3, 1) \
F(NewClosure, 1, 1) \
F(NewClosure_Tenured, 1, 1) \
F(NewScriptContext, 2, 1) \
F(NewFunctionContext, 1, 1) \
F(PushWithContext, 2, 1) \
F(PushCatchContext, 3, 1) \
F(PushBlockContext, 2, 1) \
F(IsJSModule, 1, 1) \
F(PushModuleContext, 2, 1) \
F(DeclareModules, 1, 1) \
F(DeleteLookupSlot, 1, 1) \
F(LoadLookupSlot, 1, 1) \
F(LoadLookupSlotInsideTypeof, 1, 1) \
F(StoreLookupSlot_Sloppy, 2, 1) \
F(StoreLookupSlot_Strict, 2, 1)
#define FOR_EACH_INTRINSIC_SIMD(F) \
......
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