From d941053dbe8433bf149a5db869cd17048486e8d2 Mon Sep 17 00:00:00 2001 From: "kmillikin@chromium.org" Date: Mon, 8 Aug 2011 16:14:46 +0000 Subject: [PATCH] Revert "Revert "Fix a bug in scope analysis."" Reapply r8838 with a fix for the issue of function names. Because function names can be added/changed/removed through the API, remember whether the function is anonymous when initially parsed and use that information when compiling. R=vegorov@chromium.org BUG=1583 TEST=regress-1583 Review URL: http://codereview.chromium.org/7491097 git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@8858 ce2b1a6d-e550-0410-aec6-3dcde31c8c00 --- src/compiler.cc | 1 + src/heap.cc | 37 ++++++++++++------------ src/objects-inl.h | 35 +++++------------------ src/objects.h | 32 ++++++++++++++------- src/parser.cc | 54 +++++++++++++++++++----------------- src/parser.h | 3 +- src/runtime.cc | 40 ++++++++++++++++---------- src/runtime.h | 2 ++ src/v8natives.js | 8 ++++-- test/mjsunit/regress/regress-1583.js | 50 +++++++++++++++++++++++++++++++++ 10 files changed, 161 insertions(+), 101 deletions(-) create mode 100644 test/mjsunit/regress/regress-1583.js diff --git a/src/compiler.cc b/src/compiler.cc index c265b95..97d5ddc 100755 --- a/src/compiler.cc +++ b/src/compiler.cc @@ -736,6 +736,7 @@ void Compiler::SetFunctionInfo(Handle function_info, function_info->set_start_position(lit->start_position()); function_info->set_end_position(lit->end_position()); function_info->set_is_expression(lit->is_expression()); + function_info->set_is_anonymous(lit->name()->length() == 0); function_info->set_is_toplevel(is_toplevel); function_info->set_inferred_name(*lit->inferred_name()); function_info->SetThisPropertyAssignmentsInfo( diff --git a/src/heap.cc b/src/heap.cc index 8c5981b..dbf3b95 100644 --- a/src/heap.cc +++ b/src/heap.cc @@ -2399,40 +2399,41 @@ MaybeObject* Heap::AllocateForeign(Address address, PretenureFlag pretenure) { MaybeObject* Heap::AllocateSharedFunctionInfo(Object* name) { - Object* result; - { MaybeObject* maybe_result = - Allocate(shared_function_info_map(), OLD_POINTER_SPACE); - if (!maybe_result->ToObject(&result)) return maybe_result; - } + SharedFunctionInfo* share; + MaybeObject* maybe = Allocate(shared_function_info_map(), OLD_POINTER_SPACE); + if (!maybe->To(&share)) return maybe; - SharedFunctionInfo* share = SharedFunctionInfo::cast(result); + // Set pointer fields. share->set_name(name); Code* illegal = isolate_->builtins()->builtin(Builtins::kIllegal); share->set_code(illegal); share->set_scope_info(SerializedScopeInfo::Empty()); - Code* construct_stub = isolate_->builtins()->builtin( - Builtins::kJSConstructStubGeneric); + Code* construct_stub = + isolate_->builtins()->builtin(Builtins::kJSConstructStubGeneric); share->set_construct_stub(construct_stub); - share->set_expected_nof_properties(0); - share->set_length(0); - share->set_formal_parameter_count(0); share->set_instance_class_name(Object_symbol()); share->set_function_data(undefined_value()); share->set_script(undefined_value()); - share->set_start_position_and_type(0); share->set_debug_info(undefined_value()); share->set_inferred_name(empty_string()); - share->set_compiler_hints(0); - share->set_deopt_counter(Smi::FromInt(FLAG_deopt_every_n_times)); share->set_initial_map(undefined_value()); - share->set_this_property_assignments_count(0); share->set_this_property_assignments(undefined_value()); - share->set_opt_count(0); + share->set_deopt_counter(Smi::FromInt(FLAG_deopt_every_n_times)); + + // Set integer fields (smi or int, depending on the architecture). + share->set_length(0); + share->set_formal_parameter_count(0); + share->set_expected_nof_properties(0); share->set_num_literals(0); + share->set_start_position_and_type(0); share->set_end_position(0); share->set_function_token_position(0); - share->set_native(false); - return result; + // All compiler hints default to false or 0. + share->set_compiler_hints(0); + share->set_this_property_assignments_count(0); + share->set_opt_count(0); + + return share; } diff --git a/src/objects-inl.h b/src/objects-inl.h index 0e76c9c..70ed47b 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -3534,35 +3534,14 @@ void SharedFunctionInfo::set_optimization_disabled(bool disable) { } -BOOL_ACCESSORS(SharedFunctionInfo, - compiler_hints, - strict_mode, +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, strict_mode, kStrictModeFunction) - - -bool SharedFunctionInfo::native() { - return BooleanBit::get(compiler_hints(), kNative); -} - - -void SharedFunctionInfo::set_native(bool value) { - set_compiler_hints(BooleanBit::set(compiler_hints(), - kNative, - value)); -} - - -bool SharedFunctionInfo::bound() { - return BooleanBit::get(compiler_hints(), kBoundFunction); -} - - -void SharedFunctionInfo::set_bound(bool value) { - set_compiler_hints(BooleanBit::set(compiler_hints(), - kBoundFunction, - value)); -} - +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, native, kNative) +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, + name_should_print_as_anonymous, + kNameShouldPrintAsAnonymous) +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, bound, kBoundFunction) +BOOL_ACCESSORS(SharedFunctionInfo, compiler_hints, is_anonymous, kIsAnonymous) ACCESSORS(CodeCache, default_cache, FixedArray, kDefaultCacheOffset) ACCESSORS(CodeCache, normal_type_cache, Object, kNormalTypeCacheOffset) diff --git a/src/objects.h b/src/objects.h index 1bcb627..8f9f18c 100644 --- a/src/objects.h +++ b/src/objects.h @@ -4666,12 +4666,10 @@ class SharedFunctionInfo: public HeapObject { inline void set_end_position(int end_position); // Is this function a function expression in the source code. - inline bool is_expression(); - inline void set_is_expression(bool value); + DECL_BOOLEAN_ACCESSORS(is_expression) // Is this function a top-level function (scripts, evals). - inline bool is_toplevel(); - inline void set_is_toplevel(bool value); + DECL_BOOLEAN_ACCESSORS(is_toplevel) // Bit field containing various information collected by the compiler to // drive optimization. @@ -4727,13 +4725,21 @@ class SharedFunctionInfo: public HeapObject { // These needs special threatment in .call and .apply since // null passed as the receiver should not be translated to the // global object. - inline bool native(); - inline void set_native(bool value); + DECL_BOOLEAN_ACCESSORS(native) + + // Indicates that the function was created by the Function function. + // Though it's anonymous, toString should treat it as if it had the name + // "anonymous". We don't set the name itself so that the system does not + // see a binding for it. + DECL_BOOLEAN_ACCESSORS(name_should_print_as_anonymous) // Indicates whether the function is a bound function created using // the bind function. - inline bool bound(); - inline void set_bound(bool value); + DECL_BOOLEAN_ACCESSORS(bound) + + // Indicates that the function is anonymous (the name field can be set + // through the API, which does not change this flag). + DECL_BOOLEAN_ACCESSORS(is_anonymous) // Indicates whether or not the code in the shared function support // deoptimization. @@ -4915,7 +4921,6 @@ class SharedFunctionInfo: public HeapObject { // Bit positions in compiler_hints. static const int kCodeAgeSize = 3; static const int kCodeAgeMask = (1 << kCodeAgeSize) - 1; - static const int kBoundFunction = 9; enum CompilerHints { kHasOnlySimpleThisPropertyAssignments, @@ -4926,7 +4931,11 @@ class SharedFunctionInfo: public HeapObject { kStrictModeFunction, kUsesArguments, kHasDuplicateParameters, - kNative + kNative, + kBoundFunction, + kIsAnonymous, + kNameShouldPrintAsAnonymous, + kCompilerHintsCount // Pseudo entry }; private: @@ -4940,6 +4949,9 @@ class SharedFunctionInfo: public HeapObject { static const int kCompilerHintsSize = kIntSize; #endif + STATIC_ASSERT(SharedFunctionInfo::kCompilerHintsCount <= + SharedFunctionInfo::kCompilerHintsSize * kBitsPerByte); + public: // Constants for optimizing codegen for strict mode function and // native tests. diff --git a/src/parser.cc b/src/parser.cc index ed6ff49..dc5c7d6 100644 --- a/src/parser.cc +++ b/src/parser.cc @@ -731,10 +731,14 @@ FunctionLiteral* Parser::ParseLazy(CompilationInfo* info, FunctionLiteralType type = shared_info->is_expression() ? EXPRESSION : DECLARATION; + Handle function_name = + shared_info->is_anonymous() ? Handle::null() : name; bool ok = true; - result = ParseFunctionLiteral(name, - false, // Strict mode name already checked. - RelocInfo::kNoPosition, type, &ok); + result = ParseFunctionLiteral(function_name, + false, // Strict mode name already checked. + RelocInfo::kNoPosition, + type, + &ok); // Make sure the results agree. ASSERT(ok == (result != NULL)); } @@ -2842,8 +2846,11 @@ Expression* Parser::ParseMemberWithNewPrefixesExpression(PositionStack* stack, name = ParseIdentifierOrStrictReservedWord(&is_strict_reserved_name, CHECK_OK); } - result = ParseFunctionLiteral(name, is_strict_reserved_name, - function_token_position, NESTED, CHECK_OK); + result = ParseFunctionLiteral(name, + is_strict_reserved_name, + function_token_position, + EXPRESSION, + CHECK_OK); } else { result = ParsePrimaryExpression(CHECK_OK); } @@ -3412,7 +3419,7 @@ ObjectLiteral::Property* Parser::ParseObjectLiteralGetSet(bool is_getter, ParseFunctionLiteral(name, false, // reserved words are allowed here RelocInfo::kNoPosition, - DECLARATION, + EXPRESSION, CHECK_OK); // Allow any number of parameters for compatiabilty with JSC. // Specification only allows zero parameters for get and one for set. @@ -3619,25 +3626,22 @@ ZoneList* Parser::ParseArguments(bool* ok) { } -FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, +FunctionLiteral* Parser::ParseFunctionLiteral(Handle function_name, bool name_is_strict_reserved, int function_token_position, FunctionLiteralType type, bool* ok) { // Function :: // '(' FormalParameterList? ')' '{' FunctionBody '}' - bool is_named = !var_name.is_null(); - // The name associated with this function. If it's a function expression, - // this is the actual function name, otherwise this is the name of the - // variable declared and initialized with the function (expression). In - // that case, we don't have a function name (it's empty). - Handle name = - is_named ? var_name : isolate()->factory()->empty_symbol(); - // The function name, if any. - Handle function_name = isolate()->factory()->empty_symbol(); - if (is_named && (type == EXPRESSION || type == NESTED)) { - function_name = name; + // Anonymous functions were passed either the empty symbol or a null + // handle as the function name. Remember if we were passed a non-empty + // handle to decide whether to invoke function name inference. + bool should_infer_name = function_name.is_null(); + + // We want a non-null handle as the function name. + if (should_infer_name) { + function_name = isolate()->factory()->empty_symbol(); } int num_parameters = 0; @@ -3655,7 +3659,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, bool has_duplicate_parameters = false; // Parse function body. { LexicalScope lexical_scope(this, scope, isolate()); - top_scope_->SetScopeName(name); + top_scope_->SetScopeName(function_name); // FormalParameterList :: // '(' (Identifier)*[','] ')' @@ -3705,7 +3709,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, // NOTE: We create a proxy and resolve it here so that in the // future we can change the AST to only refer to VariableProxies // instead of Variables and Proxis as is the case now. - if (!function_name.is_null() && function_name->length() > 0) { + if (type == EXPRESSION && function_name->length() > 0) { Variable* fvar = top_scope_->DeclareFunctionVar(function_name); VariableProxy* fproxy = top_scope_->NewUnresolved(function_name, inside_with()); @@ -3739,7 +3743,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, end_pos = entry.end_pos(); if (end_pos <= function_block_pos) { // End position greater than end of stream is safe, and hard to check. - ReportInvalidPreparseData(name, CHECK_OK); + ReportInvalidPreparseData(function_name, CHECK_OK); } isolate()->counters()->total_preparse_skipped()->Increment( end_pos - function_block_pos); @@ -3769,7 +3773,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, // Validate strict mode. if (top_scope_->is_strict_mode()) { - if (IsEvalOrArguments(name)) { + if (IsEvalOrArguments(function_name)) { int position = function_token_position != RelocInfo::kNoPosition ? function_token_position : (start_pos > 0 ? start_pos - 1 : start_pos); @@ -3813,7 +3817,7 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, FunctionLiteral* function_literal = new(zone()) FunctionLiteral(isolate(), - name, + function_name, scope, body, materialized_literal_count, @@ -3823,11 +3827,11 @@ FunctionLiteral* Parser::ParseFunctionLiteral(Handle var_name, num_parameters, start_pos, end_pos, - (function_name->length() > 0), + type == EXPRESSION, has_duplicate_parameters); function_literal->set_function_token_position(function_token_position); - if (fni_ != NULL && !is_named) fni_->AddFunction(function_literal); + if (fni_ != NULL && should_infer_name) fni_->AddFunction(function_literal); return function_literal; } diff --git a/src/parser.h b/src/parser.h index 13a3603..c89c7f5 100644 --- a/src/parser.h +++ b/src/parser.h @@ -554,8 +554,7 @@ class Parser { enum FunctionLiteralType { EXPRESSION, - DECLARATION, - NESTED + DECLARATION }; ZoneList* ParseArguments(bool* ok); diff --git a/src/runtime.cc b/src/runtime.cc index 42df28d..82733a7 100644 --- a/src/runtime.cc +++ b/src/runtime.cc @@ -615,8 +615,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_CreateJSProxy) { RUNTIME_FUNCTION(MaybeObject*, Runtime_IsJSProxy) { ASSERT(args.length() == 1); Object* obj = args[0]; - return obj->IsJSProxy() - ? isolate->heap()->true_value() : isolate->heap()->false_value(); + return isolate->heap()->ToBoolean(obj->IsJSProxy()); } @@ -1038,8 +1037,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_IsExtensible) { ASSERT(proto->IsJSGlobalObject()); obj = JSObject::cast(proto); } - return obj->map()->is_extensible() ? isolate->heap()->true_value() - : isolate->heap()->false_value(); + return isolate->heap()->ToBoolean(obj->map()->is_extensible()); } @@ -1105,8 +1103,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DisableAccessChecks) { Map::cast(new_map)->set_is_access_check_needed(false); object->set_map(Map::cast(new_map)); } - return needs_access_checks ? isolate->heap()->true_value() - : isolate->heap()->false_value(); + return isolate->heap()->ToBoolean(needs_access_checks); } @@ -1917,6 +1914,24 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionSetName) { } +RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionNameShouldPrintAsAnonymous) { + NoHandleAllocation ha; + ASSERT(args.length() == 1); + CONVERT_CHECKED(JSFunction, f, args[0]); + return isolate->heap()->ToBoolean( + f->shared()->name_should_print_as_anonymous()); +} + + +RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionMarkNameShouldPrintAsAnonymous) { + NoHandleAllocation ha; + ASSERT(args.length() == 1); + CONVERT_CHECKED(JSFunction, f, args[0]); + f->shared()->set_name_should_print_as_anonymous(true); + return isolate->heap()->undefined_value(); +} + + RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionSetBound) { HandleScope scope(isolate); ASSERT(args.length() == 1); @@ -2097,8 +2112,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionIsAPIFunction) { ASSERT(args.length() == 1); CONVERT_CHECKED(JSFunction, f, args[0]); - return f->shared()->IsApiFunction() ? isolate->heap()->true_value() - : isolate->heap()->false_value(); + return isolate->heap()->ToBoolean(f->shared()->IsApiFunction()); } @@ -2107,8 +2121,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_FunctionIsBuiltin) { ASSERT(args.length() == 1); CONVERT_CHECKED(JSFunction, f, args[0]); - return f->IsBuiltin() ? isolate->heap()->true_value() : - isolate->heap()->false_value(); + return isolate->heap()->ToBoolean(f->IsBuiltin()); } @@ -9980,9 +9993,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DebugGetPropertyDetails) { details->set(0, *value); details->set(1, property_details); if (hasJavaScriptAccessors) { - details->set(2, - caught_exception ? isolate->heap()->true_value() - : isolate->heap()->false_value()); + details->set(2, isolate->heap()->ToBoolean(caught_exception)); details->set(3, FixedArray::cast(*result_callback_obj)->get(0)); details->set(4, FixedArray::cast(*result_callback_obj)->get(1)); } @@ -12169,8 +12180,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_DeleteLOL) { #ifdef LIVE_OBJECT_LIST CONVERT_SMI_ARG_CHECKED(id, 0); bool success = LiveObjectList::Delete(id); - return success ? isolate->heap()->true_value() : - isolate->heap()->false_value(); + return isolate->heap()->ToBoolean(success); #else return isolate->heap()->undefined_value(); #endif diff --git a/src/runtime.h b/src/runtime.h index 4de92a6..a52672a 100644 --- a/src/runtime.h +++ b/src/runtime.h @@ -214,6 +214,8 @@ namespace internal { F(FunctionSetReadOnlyPrototype, 1, 1) \ F(FunctionGetName, 1, 1) \ F(FunctionSetName, 2, 1) \ + F(FunctionNameShouldPrintAsAnonymous, 1, 1) \ + F(FunctionMarkNameShouldPrintAsAnonymous, 1, 1) \ F(FunctionSetBound, 1, 1) \ F(FunctionRemovePrototype, 1, 1) \ F(FunctionGetSourceCode, 1, 1) \ diff --git a/src/v8natives.js b/src/v8natives.js index 29eb0f3..88525f6 100644 --- a/src/v8natives.js +++ b/src/v8natives.js @@ -1,4 +1,4 @@ -// Copyright 2006-2008 the V8 project authors. All rights reserved. +// Copyright 2011 the V8 project authors. All rights reserved. // Redistribution and use in source and binary forms, with or without // modification, are permitted provided that the following conditions are // met: @@ -1428,7 +1428,9 @@ function FunctionSourceString(func) { } } - var name = %FunctionGetName(func); + var name = %FunctionNameShouldPrintAsAnonymous(func) + ? 'anonymous' + : %FunctionGetName(func); return 'function ' + name + source; } @@ -1523,7 +1525,7 @@ function NewFunction(arg1) { // length == 1 // The call to SetNewFunctionAttributes will ensure the prototype // property of the resulting function is enumerable (ECMA262, 15.3.5.2). var f = %CompileString(source)(); - %FunctionSetName(f, "anonymous"); + %FunctionMarkNameShouldPrintAsAnonymous(f); return %SetNewFunctionAttributes(f); } diff --git a/test/mjsunit/regress/regress-1583.js b/test/mjsunit/regress/regress-1583.js new file mode 100644 index 0000000..4849555 --- /dev/null +++ b/test/mjsunit/regress/regress-1583.js @@ -0,0 +1,50 @@ +// Copyright 2011 the V8 project authors. All rights reserved. +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following +// disclaimer in the documentation and/or other materials provided +// with the distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived +// from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +// Flags: --allow-natives-syntax + +// Regression test for a bug in recompilation of anonymous functions inside +// catch. We would incorrectly hoist them outside the catch in some cases. +function f() { + try { + throw 0; + } catch (e) { + try { + var x = { a: 'hest' }; + x.m = function (e) { return x.a; }; + } catch (e) { + } + } + return x; +} + +var o = f(); +assertEquals('hest', o.m()); +assertEquals('hest', o.m()); +assertEquals('hest', o.m()); +%OptimizeFunctionOnNextCall(o.m); +assertEquals('hest', o.m()); -- 2.7.4