Allow closures to be optimized if outer contexts that call eval are all in strict mode.

R=kmillikin@chromium.org
BUG=
TEST=mjsunit/compiler/eval-introduced-closure.js

Review URL: http://codereview.chromium.org/6993008

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7853 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

src/compiler.cc

@@ -99,7 +99,7 @@ void CompilationInfo::DisableOptimization() {
     FLAG_optimize_closures &&
     closure_.is_null() &&
     !scope_->HasTrivialOuterContext() &&
-    !scope_->outer_scope_calls_eval() &&
+    !scope_->outer_scope_calls_non_strict_eval() &&
     !scope_->inside_with();
   SetMode(is_optimizable_closure ? BASE : NONOPT);
 }

src/contexts.cc

-// 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:
@@ -243,6 +243,27 @@ bool Context::GlobalIfNotShadowedByEval(Handle<String> name) {
 }
 
 
+void Context::ComputeEvalScopeInfo(bool* outer_scope_calls_eval,
+                                   bool* outer_scope_calls_non_strict_eval) {
+  Context* context = this;
+  while (true) {
+    Handle<SerializedScopeInfo> scope_info(
+        context->closure()->shared()->scope_info());
+    if (scope_info->CallsEval()) {
+      *outer_scope_calls_eval = true;
+      if (!scope_info->IsStrictMode()) {
+        // No need to go further since the answers will not change
+        // from here.
+        *outer_scope_calls_non_strict_eval = true;
+        return;
+      }
+    }
+    if (context->IsGlobalContext()) break;
+    context = Context::cast(context->closure()->context());
+  }
+}
+
+
 void Context::AddOptimizedFunction(JSFunction* function) {
   ASSERT(IsGlobalContext());
 #ifdef DEBUG

src/contexts.h

@@ -351,6 +351,11 @@ class Context: public FixedArray {
   // eval.
   bool GlobalIfNotShadowedByEval(Handle<String> name);
 
+  // Determine if any function scope in the context call eval and if
+  // any of those calls are in non-strict mode.
+  void ComputeEvalScopeInfo(bool* outer_scope_calls_eval,
+                            bool* outer_scope_calls_non_strict_eval);
+
   // Code generation support.
   static int SlotOffset(int index) {
     return kHeaderSize + index * kPointerSize - kHeapObjectTag;

src/scopeinfo.cc

-// 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:
@@ -52,6 +52,7 @@ template<class Allocator>
 ScopeInfo<Allocator>::ScopeInfo(Scope* scope)
     : function_name_(FACTORY->empty_symbol()),
       calls_eval_(scope->calls_eval()),
+      is_strict_mode_(scope->is_strict_mode()),
       parameters_(scope->num_parameters()),
       stack_slots_(scope->num_stack_slots()),
       context_slots_(scope->num_heap_slots()),
@@ -248,6 +249,7 @@ ScopeInfo<Allocator>::ScopeInfo(SerializedScopeInfo* data)
     Object** p = p0;
     p = ReadSymbol(p, &function_name_);
     p = ReadBool(p, &calls_eval_);
+    p = ReadBool(p, &is_strict_mode_);
     p = ReadList<Allocator>(p, &context_slots_, &context_modes_);
     p = ReadList<Allocator>(p, &parameters_);
     p = ReadList<Allocator>(p, &stack_slots_);
@@ -301,8 +303,8 @@ static Object** WriteList(Object** p,
 
 template<class Allocator>
 Handle<SerializedScopeInfo> ScopeInfo<Allocator>::Serialize() {
-  // function name, calls eval, length for 3 tables:
-  const int extra_slots = 1 + 1 + 3;
+  // function name, calls eval, is_strict_mode, length for 3 tables:
+  const int extra_slots = 1 + 1 + 1 + 3;
   int length = extra_slots +
                context_slots_.length() * 2 +
                parameters_.length() +
@@ -316,6 +318,7 @@ Handle<SerializedScopeInfo> ScopeInfo<Allocator>::Serialize() {
   Object** p = p0;
   p = WriteSymbol(p, function_name_);
   p = WriteBool(p, calls_eval_);
+  p = WriteBool(p, is_strict_mode_);
   p = WriteList(p, &context_slots_, &context_modes_);
   p = WriteList(p, &parameters_);
   p = WriteList(p, &stack_slots_);
@@ -363,7 +366,8 @@ SerializedScopeInfo* SerializedScopeInfo::Empty() {
 
 Object** SerializedScopeInfo::ContextEntriesAddr() {
   ASSERT(length() > 0);
-  return data_start() + 2;  // +2 for function name and calls eval.
+  // +3 for function name, calls eval, strict mode.
+  return data_start() + 3;
 }
 
 
@@ -392,7 +396,18 @@ bool SerializedScopeInfo::CallsEval() {
     p = ReadBool(p, &calls_eval);
     return calls_eval;
   }
-  return true;
+  return false;
+}
+
+
+bool SerializedScopeInfo::IsStrictMode() {
+  if (length() > 0) {
+    Object** p = data_start() + 2;  // +2 for function name, calls eval.
+    bool strict_mode;
+    p = ReadBool(p, &strict_mode);
+    return strict_mode;
+  }
+  return false;
 }
 
 

src/scopeinfo.h

-// 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:
@@ -93,6 +93,7 @@ class ScopeInfo BASE_EMBEDDED {
  private:
   Handle<String> function_name_;
   bool calls_eval_;
+  bool is_strict_mode_;
   List<Handle<String>, Allocator > parameters_;
   List<Handle<String>, Allocator > stack_slots_;
   List<Handle<String>, Allocator > context_slots_;
@@ -113,6 +114,9 @@ class SerializedScopeInfo : public FixedArray {
   // Does this scope call eval?
   bool CallsEval();
 
+  // Is this scope a strict mode scope?
+  bool IsStrictMode();
+
   // Does this scope have an arguments shadow?
   bool HasArgumentsShadow() {
     return StackSlotIndex(GetHeap()->arguments_shadow_symbol()) >= 0;

src/scopes.cc

-// Copyright 2010 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:
@@ -199,6 +199,7 @@ void Scope::SetDefaults(Type type,
   // Inherit the strict mode from the parent scope.
   strict_mode_ = (outer_scope != NULL) && outer_scope->strict_mode_;
   outer_scope_calls_eval_ = false;
+  outer_scope_calls_non_strict_eval_ = false;
   inner_scope_calls_eval_ = false;
   outer_scope_is_eval_scope_ = false;
   force_eager_compilation_ = false;
@@ -483,8 +484,17 @@ void Scope::AllocateVariables(Handle<Context> context) {
   // and assume they may invoke eval themselves. Eventually we could capture
   // this information in the ScopeInfo and then use it here (by traversing
   // the call chain stack, at compile time).
+
   bool eval_scope = is_eval_scope();
-  PropagateScopeInfo(eval_scope, eval_scope);
+  bool outer_scope_calls_eval = false;
+  bool outer_scope_calls_non_strict_eval = false;
+  if (!is_global_scope()) {
+    context->ComputeEvalScopeInfo(&outer_scope_calls_eval,
+                                  &outer_scope_calls_non_strict_eval);
+  }
+  PropagateScopeInfo(outer_scope_calls_eval,
+                     outer_scope_calls_non_strict_eval,
+                     eval_scope);
 
   // 2) Resolve variables.
   Scope* global_scope = NULL;
@@ -616,10 +626,14 @@ void Scope::Print(int n) {
   if (HasTrivialOuterContext()) {
     Indent(n1, "// scope has trivial outer context\n");
   }
+  if (is_strict_mode()) Indent(n1, "// strict mode scope\n");
   if (scope_inside_with_) Indent(n1, "// scope inside 'with'\n");
   if (scope_contains_with_) Indent(n1, "// scope contains 'with'\n");
   if (scope_calls_eval_) Indent(n1, "// scope calls 'eval'\n");
   if (outer_scope_calls_eval_) Indent(n1, "// outer scope calls 'eval'\n");
+  if (outer_scope_calls_non_strict_eval_) {
+    Indent(n1, "// outer scope calls 'eval' in non-strict context\n");
+  }
   if (inner_scope_calls_eval_) Indent(n1, "// inner scope calls 'eval'\n");
   if (outer_scope_is_eval_scope_) {
     Indent(n1, "// outer scope is 'eval' scope\n");
@@ -846,20 +860,30 @@ void Scope::ResolveVariablesRecursively(Scope* global_scope,
 
 
 bool Scope::PropagateScopeInfo(bool outer_scope_calls_eval,
+                               bool outer_scope_calls_non_strict_eval,
                                bool outer_scope_is_eval_scope) {
   if (outer_scope_calls_eval) {
     outer_scope_calls_eval_ = true;
   }
 
+  if (outer_scope_calls_non_strict_eval) {
+    outer_scope_calls_non_strict_eval_ = true;
+  }
+
   if (outer_scope_is_eval_scope) {
     outer_scope_is_eval_scope_ = true;
   }
 
   bool calls_eval = scope_calls_eval_ || outer_scope_calls_eval_;
   bool is_eval = is_eval_scope() || outer_scope_is_eval_scope_;
+  bool calls_non_strict_eval =
+      (scope_calls_eval_ && !is_strict_mode()) ||
+      outer_scope_calls_non_strict_eval_;
   for (int i = 0; i < inner_scopes_.length(); i++) {
     Scope* inner_scope = inner_scopes_[i];
-    if (inner_scope->PropagateScopeInfo(calls_eval, is_eval)) {
+    if (inner_scope->PropagateScopeInfo(calls_eval,
+                                        calls_non_strict_eval,
+                                        is_eval)) {
       inner_scope_calls_eval_ = true;
     }
     if (inner_scope->force_eager_compilation_) {

src/scopes.h

@@ -218,6 +218,9 @@ class Scope: public ZoneObject {
   // Information about which scopes calls eval.
   bool calls_eval() const { return scope_calls_eval_; }
   bool outer_scope_calls_eval() const { return outer_scope_calls_eval_; }
+  bool outer_scope_calls_non_strict_eval() const {
+    return outer_scope_calls_non_strict_eval_;
+  }
 
   // Is this scope inside a with statement.
   bool inside_with() const { return scope_inside_with_; }
@@ -372,6 +375,7 @@ class Scope: public ZoneObject {
 
   // Computed via PropagateScopeInfo.
   bool outer_scope_calls_eval_;
+  bool outer_scope_calls_non_strict_eval_;
   bool inner_scope_calls_eval_;
   bool outer_scope_is_eval_scope_;
   bool force_eager_compilation_;
@@ -400,6 +404,7 @@ class Scope: public ZoneObject {
 
   // Scope analysis.
   bool PropagateScopeInfo(bool outer_scope_calls_eval,
+                          bool outer_scope_calls_non_strict_eval,
                           bool outer_scope_is_eval_scope);
   bool HasTrivialContext() const;
 

test/mjsunit/compiler/eval-introduced-closure.js

+// 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.
+
+// Test that functions introduced by eval work both when there are
+// strict mode and non-strict mode eval in scopes.
+
+// Flags: --allow-natives-syntax
+
+var x = 27;
+
+function f() { return x; }
+
+assertEquals(27, f());
+
+function do_eval(str) {
+  "use strict";
+  return eval(str);
+}
+
+var eval_f = do_eval('(' + f + ')');
+for (var i = 0; i < 5; i++) assertEquals(27, eval_f());
+%OptimizeFunctionOnNextCall(eval_f);
+assertEquals(27, eval_f());
+
+function do_eval_local(str) {
+  "use strict";
+  var x = 42;
+  return eval(str);
+}
+
+eval_f = do_eval_local('(' + f + ')');
+for (var i = 0; i < 5; i++) assertEquals(42, eval_f());
+%OptimizeFunctionOnNextCall(eval_f);
+assertEquals(42, eval_f());
+
+function do_eval_with_other_eval_call(str) {
+  "use strict";
+  var f = eval(str);
+  eval('var x = 1');
+  return f;
+}
+
+eval_f = do_eval_with_other_eval_call('(' + f + ')');
+for (var i = 0; i < 5; i++) assertEquals(27, eval_f());
+%OptimizeFunctionOnNextCall(eval_f);
+assertEquals(27, eval_f());
+
+function test_non_strict_outer_eval() {
+  function strict_eval(str) { "use strict"; return eval(str); }
+  var eval_f = strict_eval('(' + f + ')');
+  for (var i = 0; i < 5; i++) assertEquals(27, eval_f());
+  %OptimizeFunctionOnNextCall(eval_f);
+  assertEquals(27, eval_f());
+  eval("var x = 3");
+  assertEquals(3, eval_f());
+}
+
+test_non_strict_outer_eval();
+
+function test_strict_outer_eval() {
+  "use strict";
+  function strict_eval(str) { "use strict"; return eval(str); }
+  var eval_f = strict_eval('(' + f + ')');
+  for (var i = 0; i < 5; i++) assertEquals(27, eval_f());
+  %OptimizeFunctionOnNextCall(eval_f);
+  assertEquals(27, eval_f());
+  eval("var x = 3");
+  assertEquals(27, eval_f());
+}
+
+test_non_strict_outer_eval();