Fix access to type variables in initializer expressions

runtime/vm/scopes.cc

 // Copyright (c) 2012, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/scopes.h"
 
 #include "vm/ast.h"
 #include "vm/bit_vector.h"
 #include "vm/object.h"
 #include "vm/parser.h"
@@ skipping 291 matching lines @@
   }
   return NULL;
 }
 
 
 LocalVariable* LocalScope::LocalLookupVariable(const String& name) const {
   ASSERT(name.IsSymbol());
   for (intptr_t i = 0; i < variables_.length(); i++) {
     LocalVariable* var = variables_[i];
     ASSERT(var->name().IsSymbol());
-    if ((var->name().raw() == name.raw()) && !var->is_invisible_) {
+    if (var->name().raw() == name.raw()) {
       return var;
     }
   }
   return NULL;
 }
 
 
 LocalVariable* LocalScope::LookupVariable(const String& name, bool test_only) {
   LocalScope* current_scope = this;
   while (current_scope != NULL) {
     LocalVariable* var = current_scope->LocalLookupVariable(name);
-    if (var != NULL) {
+    if ((var != NULL) && !var->is_invisible_) {

[regis, 2013/08/07 18:28:26]

As far as I understand, if a variable is marked invisible, you are not going to
find it in the parent scope. I guess that most of the time, there is no parent
scope anyway, but still, it would be a bit more efficient to return immediately:
if (var != NULL) {
  if (var->is_invisible_) return NULL;
  ...

[hausner, 2013/08/08 09:26:06]

That may be a shortcut that works now, since we use the invisible flag only on
variables in the outermost scopes, as you say. But I think the optimization is
not worth the compromise in correctness (should we ever use "invisible" on other
variables).

       if (!test_only) {
         if (var->owner()->function_level() != function_level()) {
           var->set_is_captured();
         }
         // Insert aliases of the variable in intermediate scopes.
         LocalScope* intermediate_scope = this;
         while (intermediate_scope != current_scope) {
           intermediate_scope->variables_.Add(var);
           ASSERT(var->owner() != intermediate_scope);  // Item is an alias.
           intermediate_scope = intermediate_scope->parent();
         }
       }
       return var;
     }
     current_scope = current_scope->parent();
   }
   return NULL;
 }
 
 
+void LocalScope::CaptureVariable(const String& name) {

[regis, 2013/08/07 18:28:26]

I would still return the captured variable or a bool, so that the caller can
assert that the variable was found.

[hausner, 2013/08/08 09:26:06]

Ok, good point. will do in a follow-on CL.

+  ASSERT(name.IsSymbol());
+  LocalScope* current_scope = this;
+  while (current_scope != NULL) {
+    LocalVariable* var = current_scope->LocalLookupVariable(name);
+    if (var != NULL) {
+      if (var->owner()->function_level() != function_level()) {
+        var->set_is_captured();
+      }
+      // Insert aliases of the variable in intermediate scopes.
+      LocalScope* intermediate_scope = this;
+      while (intermediate_scope != current_scope) {
+        intermediate_scope->variables_.Add(var);
+        ASSERT(var->owner() != intermediate_scope);  // Item is an alias.
+        intermediate_scope = intermediate_scope->parent();
+      }
+      return;
+    }
+    current_scope = current_scope->parent();
+  }
+}
+
+
 SourceLabel* LocalScope::LookupLabel(const String& name) {
   LocalScope* current_scope = this;
   while (current_scope != NULL) {
     SourceLabel* label = current_scope->LocalLookupLabel(name);
     if (label != NULL) {
       return label;
     }
     current_scope = current_scope->parent();
   }
   return NULL;
@@ skipping 221 matching lines @@
     return fixed_parameter_count - (index() - kParamEndSlotFromFp);
   } else {
     // Shift negative indexes so that the lowest one is 0 (they are still
     // non-positive).
     return fixed_parameter_count - (index() - kFirstLocalSlotFromFp);
   }
 }
 
 
 }  // namespace dart