Eagerly declare eval scopes, even for sloppy scopes

src/runtime/runtime-scopes.cc

 // Copyright 2014 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "src/v8.h"
 
 #include "src/accessors.h"
 #include "src/arguments.h"
 #include "src/frames-inl.h"
 #include "src/runtime/runtime-utils.h"
@@ skipping 221 matching lines @@
   Handle<Object> holder =
       context->Lookup(name, flags, &index, &attributes, &binding_flags);
 
   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()) ||
-      (context_arg->has_extension() &&
-       context_arg->extension()->IsJSGlobalObject())) {
+  if (attributes != ABSENT && holder->IsJSGlobalObject()) {
     return DeclareGlobals(isolate, Handle<JSGlobalObject>::cast(holder), name,
                           value, attr, is_var, is_const, is_function);
   }
+  if (context_arg->has_extension() &&
+      context_arg->extension()->IsJSGlobalObject()) {

[Toon Verwaest, 2015/04/20 10:59:14]

If the holder is a JSGlobalObject, shouldn't it == context_arg->extension()
since we DONT_FOLLOW_CHAINS? Am I missing something?

[wingo, 2015/04/20 11:23:18]

Thanks for looking into this.  I think the issue I was seeing was that "holder"
was NULL and attributes were ABSENT, yet the context_arg had an extension which
was the global object.  This case was not getting exercised before, AFAICT,
because of the scoping resolution issues I mentioned in an earlier comment.

+    Handle<JSGlobalObject> global(
+        JSGlobalObject::cast(context_arg->extension()), isolate);
+    return DeclareGlobals(isolate, global, name, value, attr, is_var, is_const,
+                          is_function);
+  }
 
   if (attributes != ABSENT) {
     // The name was declared before; check for conflicting re-declarations.
     if (is_const || (attributes & READ_ONLY) != 0) {
       return ThrowRedeclarationError(isolate, name);
     }
 
     // Skip var re-declarations.
     if (is_var) return isolate->heap()->undefined_value();
 
@@ skipping 856 matching lines @@
   return Smi::FromInt(frame->GetArgumentsLength());
 }
 
 
 RUNTIME_FUNCTION(Runtime_Arguments) {
   SealHandleScope shs(isolate);
   return __RT_impl_Runtime_GetArgumentsProperty(args, isolate);
 }
 }
 }  // namespace v8::internal