Filter out synthetic variables from with scopes

src/ast/scopes.cc

Index: src/ast/scopes.cc
diff --git a/src/ast/scopes.cc b/src/ast/scopes.cc
index 87ffb9dd7a529969e4667b51f0ae0f3bf74d9b35..bfbae7626aeb0ea0036a76be7b1336962181b7a2 100644
--- a/src/ast/scopes.cc
+++ b/src/ast/scopes.cc
@@ -1448,7 +1448,7 @@ Variable* Scope::LookupRecursive(VariableProxy* proxy, Scope* outer_scope_end) {
   // "this" can't be shadowed by "eval"-introduced bindings or by "with"
   // scopes.
   // TODO(wingo): There are other variables in this category; add them.
-  if (var->is_this()) return var;
+  if (Scope::VariableIsSynthetic(var->raw_name())) return var;

[Toon Verwaest, 2016/09/19 20:01:50]

I'd be much in favor of having specialized scope resolution for those variables,
done on-the-fly when dealing with those specific vars. This just slows down
scope resolution for al other vars unnecessarily.

[Dan Ehrenberg, 2016/09/20 17:22:13]

I looked into some ways to mitigate the performance regression here in a more
limited way, without a separate scope resolution mechanism, but it seems like it
would result in at least reading another flag/having a conditional per variable
at some point or other.

What if we backed out this portion of the patch, as it hurts performance without
being what we want for the long-term structure, but left in the context part as
a quick, hopefully temporary fix for the new.target case? The context code
change shouldn't hurt performance much (I don't think we should care about with
getting slightly slower). For the promise case, I'm working on switching that to
directly using temporaries at https://codereview.chromium.org/2359513002 , but
new.target is a little tougher because it is captured by arrow functions.

 
   if (is_with_scope()) {
     // The current scope is a with scope, so the variable binding can not be