Eagerly declare eval scopes, even for sloppy scopes

src/runtime/runtime-scopes.cc

Index: src/runtime/runtime-scopes.cc
diff --git a/src/runtime/runtime-scopes.cc b/src/runtime/runtime-scopes.cc
index 2a2ab16d45ef357e928d49387042a8214caed154..7d4392e834a5337974dac2ce5098b09fb770fd4a 100644
--- a/src/runtime/runtime-scopes.cc
+++ b/src/runtime/runtime-scopes.cc
@@ -239,12 +239,17 @@ RUNTIME_FUNCTION(Runtime_DeclareLookupSlot) {
 
   // 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.