Filter out synthetic variables from with scopes

src/ast/scopes.cc

 // Copyright 2012 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/ast/scopes.h"
 
 #include <set>
 
 #include "src/accessors.h"
 #include "src/ast/ast.h"
@@ skipping 1430 matching lines @@
 
   // The variable could not be resolved statically.
   if (var == nullptr) return var;
 
   if (is_function_scope() && !var->is_dynamic()) {
     var->ForceContextAllocation();
   }
   // "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
     // statically resolved. However, note that it was necessary to do a lookup
     // in the outer scope anyway, because if a binding exists in an outer
     // scope, the associated variable has to be marked as potentially being
     // accessed from inside of an inner with scope (the property may not be in
     // the 'with' object).
     if (!var->is_dynamic() && var->IsUnallocated()) {
       DCHECK(!already_resolved_);
@@ skipping 350 matching lines @@
   Variable* function =
       is_function_scope() ? AsDeclarationScope()->function_var() : nullptr;
   bool is_function_var_in_context =
       function != nullptr && function->IsContextSlot();
   return num_heap_slots() - Context::MIN_CONTEXT_SLOTS -
          (is_function_var_in_context ? 1 : 0);
 }
 
 }  // namespace internal
 }  // namespace v8