From 826f8da55fb868a365d047a4a653eb8ff2bfc14e Mon Sep 17 00:00:00 2001 From: rossberg Date: Fri, 7 Aug 2015 04:38:20 -0700 Subject: [PATCH] [es6] Use strict arguments objects for destructured parameters Plus some renaming for consistency. R=adamk@chromium.org BUG=v8:811 LOG=N Review URL: https://codereview.chromium.org/1278783002 Cr-Commit-Position: refs/heads/master@{#30064} --- src/compiler.cc | 4 ++-- src/compiler.h | 2 +- src/factory.cc | 3 +-- src/full-codegen/arm/full-codegen-arm.cc | 2 +- src/full-codegen/arm64/full-codegen-arm64.cc | 2 +- src/full-codegen/full-codegen.h | 2 +- src/full-codegen/ia32/full-codegen-ia32.cc | 2 +- src/full-codegen/mips/full-codegen-mips.cc | 2 +- src/full-codegen/mips64/full-codegen-mips64.cc | 2 +- src/full-codegen/ppc/full-codegen-ppc.cc | 2 +- src/full-codegen/x64/full-codegen-x64.cc | 2 +- src/full-codegen/x87/full-codegen-x87.cc | 2 +- src/objects-inl.h | 8 ++++---- src/objects.h | 12 ++++++------ src/parser.cc | 1 + src/runtime/runtime-scopes.cc | 4 ++-- src/scopeinfo.cc | 12 ++++++------ src/scopes.cc | 9 +++++++-- src/scopes.h | 8 ++++---- test/mjsunit/harmony/destructuring.js | 21 +++++++++++++++++++++ 20 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/compiler.cc b/src/compiler.cc index ccf4171..ede006c 100644 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -222,8 +222,8 @@ void CompilationInfo::EnsureFeedbackVector() { } -bool CompilationInfo::is_simple_parameter_list() { - return scope()->is_simple_parameter_list(); +bool CompilationInfo::has_simple_parameters() { + return scope()->has_simple_parameters(); } diff --git a/src/compiler.h b/src/compiler.h index df81a4f..78ae812 100644 --- a/src/compiler.h +++ b/src/compiler.h @@ -400,7 +400,7 @@ class CompilationInfo { void PrintAstForTesting(); #endif - bool is_simple_parameter_list(); + bool has_simple_parameters(); Handle GenerateCodeStub(); diff --git a/src/factory.cc b/src/factory.cc index 8944905..05cb259 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -2348,10 +2348,9 @@ Handle Factory::NewDebugInfo(Handle shared) { Handle Factory::NewArgumentsObject(Handle callee, int length) { bool strict_mode_callee = is_strict(callee->shared()->language_mode()) || - !callee->is_simple_parameter_list(); + !callee->has_simple_parameters(); Handle map = strict_mode_callee ? isolate()->strict_arguments_map() : isolate()->sloppy_arguments_map(); - AllocationSiteUsageContext context(isolate(), Handle(), false); DCHECK(!isolate()->has_pending_exception()); diff --git a/src/full-codegen/arm/full-codegen-arm.cc b/src/full-codegen/arm/full-codegen-arm.cc index 0e6d4c0..7ebf078 100644 --- a/src/full-codegen/arm/full-codegen-arm.cc +++ b/src/full-codegen/arm/full-codegen-arm.cc @@ -318,7 +318,7 @@ void FullCodeGenerator::Generate() { // The stub will rewrite receiver and parameter count if the previous // stack frame was an arguments adapter frame. ArgumentsAccessStub::Type type; - if (is_strict(language_mode()) || !is_simple_parameter_list()) { + if (is_strict(language_mode()) || !has_simple_parameters()) { type = ArgumentsAccessStub::NEW_STRICT; } else if (function()->has_duplicate_parameters()) { type = ArgumentsAccessStub::NEW_SLOPPY_SLOW; diff --git a/src/full-codegen/arm64/full-codegen-arm64.cc b/src/full-codegen/arm64/full-codegen-arm64.cc index 678471d..a90019e 100644 --- a/src/full-codegen/arm64/full-codegen-arm64.cc +++ b/src/full-codegen/arm64/full-codegen-arm64.cc @@ -324,7 +324,7 @@ void FullCodeGenerator::Generate() { // The stub will rewrite receiver and parameter count if the previous // stack frame was an arguments adapter frame. ArgumentsAccessStub::Type type; - if (is_strict(language_mode()) || !is_simple_parameter_list()) { + if (is_strict(language_mode()) || !has_simple_parameters()) { type = ArgumentsAccessStub::NEW_STRICT; } else if (function()->has_duplicate_parameters()) { type = ArgumentsAccessStub::NEW_SLOPPY_SLOW; diff --git a/src/full-codegen/full-codegen.h b/src/full-codegen/full-codegen.h index 6761bda..5406ccd 100644 --- a/src/full-codegen/full-codegen.h +++ b/src/full-codegen/full-codegen.h @@ -700,7 +700,7 @@ class FullCodeGenerator: public AstVisitor { bool is_eval() { return info_->is_eval(); } bool is_native() { return info_->is_native(); } LanguageMode language_mode() { return function()->language_mode(); } - bool is_simple_parameter_list() { return info_->is_simple_parameter_list(); } + bool has_simple_parameters() { return info_->has_simple_parameters(); } FunctionLiteral* function() { return info_->function(); } Scope* scope() { return scope_; } diff --git a/src/full-codegen/ia32/full-codegen-ia32.cc b/src/full-codegen/ia32/full-codegen-ia32.cc index 6c1ac09..d9ca3bd 100644 --- a/src/full-codegen/ia32/full-codegen-ia32.cc +++ b/src/full-codegen/ia32/full-codegen-ia32.cc @@ -320,7 +320,7 @@ void FullCodeGenerator::Generate() { // The stub will rewrite receiver and parameter count if the previous // stack frame was an arguments adapter frame. ArgumentsAccessStub::Type type; - if (is_strict(language_mode()) || !is_simple_parameter_list()) { + if (is_strict(language_mode()) || !has_simple_parameters()) { type = ArgumentsAccessStub::NEW_STRICT; } else if (function()->has_duplicate_parameters()) { type = ArgumentsAccessStub::NEW_SLOPPY_SLOW; diff --git a/src/full-codegen/mips/full-codegen-mips.cc b/src/full-codegen/mips/full-codegen-mips.cc index cef99ab..a63f954c 100644 --- a/src/full-codegen/mips/full-codegen-mips.cc +++ b/src/full-codegen/mips/full-codegen-mips.cc @@ -337,7 +337,7 @@ void FullCodeGenerator::Generate() { // The stub will rewrite receiever and parameter count if the previous // stack frame was an arguments adapter frame. ArgumentsAccessStub::Type type; - if (is_strict(language_mode()) || !is_simple_parameter_list()) { + if (is_strict(language_mode()) || !has_simple_parameters()) { type = ArgumentsAccessStub::NEW_STRICT; } else if (function()->has_duplicate_parameters()) { type = ArgumentsAccessStub::NEW_SLOPPY_SLOW; diff --git a/src/full-codegen/mips64/full-codegen-mips64.cc b/src/full-codegen/mips64/full-codegen-mips64.cc index 8075822..2f3f115 100644 --- a/src/full-codegen/mips64/full-codegen-mips64.cc +++ b/src/full-codegen/mips64/full-codegen-mips64.cc @@ -333,7 +333,7 @@ void FullCodeGenerator::Generate() { // The stub will rewrite receiever and parameter count if the previous // stack frame was an arguments adapter frame. ArgumentsAccessStub::Type type; - if (is_strict(language_mode()) || !is_simple_parameter_list()) { + if (is_strict(language_mode()) || !has_simple_parameters()) { type = ArgumentsAccessStub::NEW_STRICT; } else if (function()->has_duplicate_parameters()) { type = ArgumentsAccessStub::NEW_SLOPPY_SLOW; diff --git a/src/full-codegen/ppc/full-codegen-ppc.cc b/src/full-codegen/ppc/full-codegen-ppc.cc index e20f367..841077c 100644 --- a/src/full-codegen/ppc/full-codegen-ppc.cc +++ b/src/full-codegen/ppc/full-codegen-ppc.cc @@ -330,7 +330,7 @@ void FullCodeGenerator::Generate() { // The stub will rewrite receiver and parameter count if the previous // stack frame was an arguments adapter frame. ArgumentsAccessStub::Type type; - if (is_strict(language_mode()) || !is_simple_parameter_list()) { + if (is_strict(language_mode()) || !has_simple_parameters()) { type = ArgumentsAccessStub::NEW_STRICT; } else if (function()->has_duplicate_parameters()) { type = ArgumentsAccessStub::NEW_SLOPPY_SLOW; diff --git a/src/full-codegen/x64/full-codegen-x64.cc b/src/full-codegen/x64/full-codegen-x64.cc index 65c0456..206048f 100644 --- a/src/full-codegen/x64/full-codegen-x64.cc +++ b/src/full-codegen/x64/full-codegen-x64.cc @@ -319,7 +319,7 @@ void FullCodeGenerator::Generate() { // stack frame was an arguments adapter frame. ArgumentsAccessStub::Type type; - if (is_strict(language_mode()) || !is_simple_parameter_list()) { + if (is_strict(language_mode()) || !has_simple_parameters()) { type = ArgumentsAccessStub::NEW_STRICT; } else if (function()->has_duplicate_parameters()) { type = ArgumentsAccessStub::NEW_SLOPPY_SLOW; diff --git a/src/full-codegen/x87/full-codegen-x87.cc b/src/full-codegen/x87/full-codegen-x87.cc index f1848c9..38702e5 100644 --- a/src/full-codegen/x87/full-codegen-x87.cc +++ b/src/full-codegen/x87/full-codegen-x87.cc @@ -317,7 +317,7 @@ void FullCodeGenerator::Generate() { // The stub will rewrite receiver and parameter count if the previous // stack frame was an arguments adapter frame. ArgumentsAccessStub::Type type; - if (is_strict(language_mode()) || !is_simple_parameter_list()) { + if (is_strict(language_mode()) || !has_simple_parameters()) { type = ArgumentsAccessStub::NEW_STRICT; } else if (function()->has_duplicate_parameters()) { type = ArgumentsAccessStub::NEW_SLOPPY_SLOW; diff --git a/src/objects-inl.h b/src/objects-inl.h index 3a9fe29..d8175c7 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -5378,8 +5378,8 @@ bool SharedFunctionInfo::is_compiled() { } -bool SharedFunctionInfo::is_simple_parameter_list() { - return scope_info()->IsSimpleParameterList(); +bool SharedFunctionInfo::has_simple_parameters() { + return scope_info()->HasSimpleParameters(); } @@ -5685,8 +5685,8 @@ bool JSFunction::is_compiled() { } -bool JSFunction::is_simple_parameter_list() { - return shared()->is_simple_parameter_list(); +bool JSFunction::has_simple_parameters() { + return shared()->has_simple_parameters(); } diff --git a/src/objects.h b/src/objects.h index 257f0e5..8e64fe3 100644 --- a/src/objects.h +++ b/src/objects.h @@ -3886,8 +3886,8 @@ class ScopeInfo : public FixedArray { // Return if this is a nested function within an asm module scope. bool IsAsmFunction() { return AsmFunctionField::decode(Flags()); } - bool IsSimpleParameterList() { - return IsSimpleParameterListField::decode(Flags()); + bool HasSimpleParameters() { + return HasSimpleParametersField::decode(Flags()); } // Return the function_name if present. @@ -4086,10 +4086,10 @@ class ScopeInfo : public FixedArray { class AsmModuleField : public BitField { }; class AsmFunctionField : public BitField {}; - class IsSimpleParameterListField + class HasSimpleParametersField : public BitField {}; class FunctionKindField - : public BitField {}; + : public BitField {}; // BitFields representing the encoded information for context locals in the // ContextLocalInfoEntries part. @@ -6630,7 +6630,7 @@ class SharedFunctionInfo: public HeapObject { // Calculate the number of in-object properties. int CalculateInObjectProperties(); - inline bool is_simple_parameter_list(); + inline bool has_simple_parameters(); // Initialize a SharedFunctionInfo from a parsed function literal. static void InitFromFunctionLiteral(Handle shared_info, @@ -7145,7 +7145,7 @@ class JSFunction: public JSObject { // Returns `false` if formal parameters include rest parameters, optional // parameters, or destructuring parameters. // TODO(caitp): make this a flag set during parsing - inline bool is_simple_parameter_list(); + inline bool has_simple_parameters(); // [next_function_link]: Links functions into various lists, e.g. the list // of optimized functions hanging off the native_context. The CodeFlusher diff --git a/src/parser.cc b/src/parser.cc index 6a0838e..45202cb 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -3926,6 +3926,7 @@ void ParserTraits::ParseArrowFunctionFormalParameterList( *duplicate_loc = classifier.duplicate_formal_parameter_error().location; } } + DCHECK_EQ(parameters->is_simple, parameters->scope->has_simple_parameters()); } diff --git a/src/runtime/runtime-scopes.cc b/src/runtime/runtime-scopes.cc index ff7e783..be13a14 100644 --- a/src/runtime/runtime-scopes.cc +++ b/src/runtime/runtime-scopes.cc @@ -397,7 +397,7 @@ static Handle NewSloppyArguments(Isolate* isolate, Object** parameters, int argument_count) { CHECK(!IsSubclassConstructor(callee->shared()->kind())); - DCHECK(callee->is_simple_parameter_list()); + DCHECK(callee->has_simple_parameters()); Handle result = isolate->factory()->NewArgumentsObject(callee, argument_count); @@ -518,7 +518,7 @@ RUNTIME_FUNCTION(Runtime_NewArguments) { Object** parameters = reinterpret_cast(frame->GetParameterSlot(-1)); return (is_strict(callee->shared()->language_mode()) || - !callee->is_simple_parameter_list()) + !callee->has_simple_parameters()) ? *NewStrictArguments(isolate, callee, parameters, argument_count) : *NewSloppyArguments(isolate, callee, parameters, argument_count); } diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc index e490fd9..e53f36d 100644 --- a/src/scopeinfo.cc +++ b/src/scopeinfo.cc @@ -34,9 +34,6 @@ Handle ScopeInfo::Create(Isolate* isolate, Zone* zone, DCHECK_EQ(scope->ContextLocalCount(), context_local_count); DCHECK_EQ(scope->ContextGlobalCount(), context_global_count); - bool simple_parameter_list = - scope->is_function_scope() ? scope->is_simple_parameter_list() : true; - // Determine use and location of the "this" binding if it is present. VariableAllocationInfo receiver_info; if (scope->has_this_declaration()) { @@ -85,6 +82,9 @@ Handle ScopeInfo::Create(Isolate* isolate, Zone* zone, Factory* factory = isolate->factory(); Handle scope_info = factory->NewScopeInfo(length); + bool has_simple_parameters = + scope->is_function_scope() && scope->has_simple_parameters(); + // Encode the flags. int flags = ScopeTypeField::encode(scope->scope_type()) | CallsEvalField::encode(scope->calls_eval()) | @@ -94,7 +94,7 @@ Handle ScopeInfo::Create(Isolate* isolate, Zone* zone, FunctionVariableMode::encode(function_variable_mode) | AsmModuleField::encode(scope->asm_module()) | AsmFunctionField::encode(scope->asm_function()) | - IsSimpleParameterListField::encode(simple_parameter_list) | + HasSimpleParametersField::encode(has_simple_parameters) | FunctionKindField::encode(scope->function_kind()); scope_info->SetFlags(flags); scope_info->SetParameterCount(parameter_count); @@ -224,7 +224,7 @@ Handle ScopeInfo::CreateGlobalThisBinding(Isolate* isolate) { const int context_local_count = 1; const int context_global_count = 0; const int strong_mode_free_variable_count = 0; - const bool simple_parameter_list = true; + const bool has_simple_parameters = true; const VariableAllocationInfo receiver_info = CONTEXT; const VariableAllocationInfo function_name_info = NONE; const VariableMode function_variable_mode = VAR; @@ -248,7 +248,7 @@ Handle ScopeInfo::CreateGlobalThisBinding(Isolate* isolate) { FunctionVariableField::encode(function_name_info) | FunctionVariableMode::encode(function_variable_mode) | AsmModuleField::encode(false) | AsmFunctionField::encode(false) | - IsSimpleParameterListField::encode(simple_parameter_list) | + HasSimpleParametersField::encode(has_simple_parameters) | FunctionKindField::encode(FunctionKind::kNormalFunction); scope_info->SetFlags(flags); scope_info->SetParameterCount(parameter_count); diff --git a/src/scopes.cc b/src/scopes.cc index 319aca5..8ce28bd 100644 --- a/src/scopes.cc +++ b/src/scopes.cc @@ -180,7 +180,8 @@ void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope, num_heap_slots_ = 0; num_global_slots_ = 0; num_modules_ = 0; - module_var_ = NULL, + module_var_ = NULL; + has_simple_parameters_ = true; rest_parameter_ = NULL; rest_index_ = -1; scope_info_ = scope_info; @@ -468,6 +469,7 @@ Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode mode, Variable* var; if (mode == TEMPORARY) { var = NewTemporary(name); + has_simple_parameters_ = false; } else { var = variables_.Declare(this, name, mode, Variable::NORMAL, kCreatedInitialized); @@ -478,6 +480,7 @@ Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode mode, DCHECK_NULL(rest_parameter_); rest_parameter_ = var; rest_index_ = num_parameters(); + has_simple_parameters_ = false; } params_.Add(var, zone()); return var; @@ -1399,7 +1402,9 @@ void Scope::AllocateParameterLocals(Isolate* isolate) { // In strict mode 'arguments' does not alias formal parameters. // Therefore in strict mode we allocate parameters as if 'arguments' // were not used. - uses_sloppy_arguments = is_sloppy(language_mode()); + // If the parameter list is not simple, arguments isn't sloppy either. + uses_sloppy_arguments = + is_sloppy(language_mode()) && has_simple_parameters(); } if (rest_parameter_ && !MustAllocate(rest_parameter_)) { diff --git a/src/scopes.h b/src/scopes.h index f48b3ae..acfad4c 100644 --- a/src/scopes.h +++ b/src/scopes.h @@ -389,10 +389,9 @@ class Scope: public ZoneObject { return rest_index_ >= 0; } - bool is_simple_parameter_list() const { + bool has_simple_parameters() const { DCHECK(is_function_scope()); - if (rest_index_ >= 0) return false; - return true; + return has_simple_parameters_; } // The local variable 'arguments' if we need to allocate it; NULL otherwise. @@ -632,7 +631,8 @@ class Scope: public ZoneObject { // For module scopes, the host scope's temporary variable binding this module. Variable* module_var_; - // Rest parameter + // Info about the parameter list of a function. + bool has_simple_parameters_; Variable* rest_parameter_; int rest_index_; diff --git a/test/mjsunit/harmony/destructuring.js b/test/mjsunit/harmony/destructuring.js index 0bbaf26..2a525fa 100644 --- a/test/mjsunit/harmony/destructuring.js +++ b/test/mjsunit/harmony/destructuring.js @@ -918,6 +918,27 @@ }()); +(function TestArgumentsForNonSimpleParameters() { + function f1({}, x) { arguments[1] = 0; return x } + assertEquals(6, f1({}, 6)); + function f2({}, x) { x = 2; return arguments[1] } + assertEquals(7, f2({}, 7)); + function f3(x, {}) { arguments[0] = 0; return x } + assertEquals(6, f3(6, {})); + function f4(x, {}) { x = 2; return arguments[0] } + assertEquals(7, f4(7, {})); + function f5(x, ...a) { arguments[0] = 0; return x } + assertEquals(6, f5(6, {})); + function f6(x, ...a) { x = 2; return arguments[0] } + assertEquals(6, f6(6, {})); + function f7({a: x}) { x = 2; return arguments[0].a } + assertEquals(5, f7({a: 5})); + function f8(x, ...a) { a = []; return arguments[1] } + assertEquals(6, f8(5, 6)); + // TODO(caitp, rossberg): add cases for default parameters. +}()); + + (function TestForInOfTDZ() { assertThrows("'use strict'; let x = {}; for (let [x, y] of {x});", ReferenceError); assertThrows("'use strict'; let x = {}; for (let [y, x] of {x});", ReferenceError); -- 2.7.4