Revision: 8858
Author: [email protected]
Date: Mon Aug 8 09:14:46 2011
Log: 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.
[email protected]
BUG=1583
TEST=regress-1583
Review URL: http://codereview.chromium.org/7491097
http://code.google.com/p/v8/source/detail?r=8858
Added:
/branches/bleeding_edge/test/mjsunit/regress/regress-1583.js
Modified:
/branches/bleeding_edge/src/compiler.cc
/branches/bleeding_edge/src/heap.cc
/branches/bleeding_edge/src/objects-inl.h
/branches/bleeding_edge/src/objects.h
/branches/bleeding_edge/src/parser.cc
/branches/bleeding_edge/src/parser.h
/branches/bleeding_edge/src/runtime.cc
/branches/bleeding_edge/src/runtime.h
/branches/bleeding_edge/src/v8natives.js
=======================================
--- /dev/null
+++ /branches/bleeding_edge/test/mjsunit/regress/regress-1583.js Mon Aug 8
09:14:46 2011
@@ -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());
=======================================
--- /branches/bleeding_edge/src/compiler.cc Tue Jul 19 01:19:31 2011
+++ /branches/bleeding_edge/src/compiler.cc Mon Aug 8 09:14:46 2011
@@ -736,6 +736,7 @@
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(
=======================================
--- /branches/bleeding_edge/src/heap.cc Fri Aug 5 02:44:30 2011
+++ /branches/bleeding_edge/src/heap.cc Mon Aug 8 09:14:46 2011
@@ -2399,40 +2399,41 @@
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 = SharedFunctionInfo::cast(result);
+ SharedFunctionInfo* share;
+ MaybeObject* maybe = Allocate(shared_function_info_map(),
OLD_POINTER_SPACE);
+ if (!maybe->To<SharedFunctionInfo>(&share)) return maybe;
+
+ // 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;
}
=======================================
--- /branches/bleeding_edge/src/objects-inl.h Fri Aug 5 02:20:08 2011
+++ /branches/bleeding_edge/src/objects-inl.h Mon Aug 8 09:14:46 2011
@@ -3534,35 +3534,14 @@
}
-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)
=======================================
--- /branches/bleeding_edge/src/objects.h Fri Aug 5 02:20:08 2011
+++ /branches/bleeding_edge/src/objects.h Mon Aug 8 09:14:46 2011
@@ -4666,12 +4666,10 @@
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 @@
// 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 @@
// 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 @@
kStrictModeFunction,
kUsesArguments,
kHasDuplicateParameters,
- kNative
+ kNative,
+ kBoundFunction,
+ kIsAnonymous,
+ kNameShouldPrintAsAnonymous,
+ kCompilerHintsCount // Pseudo entry
};
private:
@@ -4940,6 +4949,9 @@
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.
=======================================
--- /branches/bleeding_edge/src/parser.cc Fri Aug 5 02:20:08 2011
+++ /branches/bleeding_edge/src/parser.cc Mon Aug 8 09:14:46 2011
@@ -731,10 +731,14 @@
FunctionLiteralType type =
shared_info->is_expression() ? EXPRESSION : DECLARATION;
+ Handle<String> function_name =
+ shared_info->is_anonymous() ? Handle<String>::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 @@
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 @@
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 @@
}
-FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> var_name,
+FunctionLiteral* Parser::ParseFunctionLiteral(Handle<String> 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<String> name =
- is_named ? var_name : isolate()->factory()->empty_symbol();
- // The function name, if any.
- Handle<String> 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 @@
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 @@
// 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 @@
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 @@
// 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* function_literal =
new(zone()) FunctionLiteral(isolate(),
- name,
+ function_name,
scope,
body,
materialized_literal_count,
@@ -3823,11 +3827,11 @@
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;
}
=======================================
--- /branches/bleeding_edge/src/parser.h Fri Aug 5 02:20:08 2011
+++ /branches/bleeding_edge/src/parser.h Mon Aug 8 09:14:46 2011
@@ -554,8 +554,7 @@
enum FunctionLiteralType {
EXPRESSION,
- DECLARATION,
- NESTED
+ DECLARATION
};
ZoneList<Expression*>* ParseArguments(bool* ok);
=======================================
--- /branches/bleeding_edge/src/runtime.cc Fri Aug 5 05:00:57 2011
+++ /branches/bleeding_edge/src/runtime.cc Mon Aug 8 09:14:46 2011
@@ -615,8 +615,7 @@
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 @@
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 @@
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);
}
@@ -1915,6 +1912,24 @@
f->shared()->set_name(name);
return isolate->heap()->undefined_value();
}
+
+
+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) {
@@ -2097,8 +2112,7 @@
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 @@
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 @@
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 @@
#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
=======================================
--- /branches/bleeding_edge/src/runtime.h Fri Aug 5 02:20:08 2011
+++ /branches/bleeding_edge/src/runtime.h Mon Aug 8 09:14:46 2011
@@ -214,6 +214,8 @@
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) \
=======================================
--- /branches/bleeding_edge/src/v8natives.js Fri Aug 5 02:20:08 2011
+++ /branches/bleeding_edge/src/v8natives.js Mon Aug 8 09:14:46 2011
@@ -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 @@
}
}
- var name = %FunctionGetName(func);
+ var name = %FunctionNameShouldPrintAsAnonymous(func)
+ ? 'anonymous'
+ : %FunctionGetName(func);
return 'function ' + name + source;
}
@@ -1523,7 +1525,7 @@
// 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);
}
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
