Remove synthetic unresolved variables from async/await desugaring

This patch uses temporaries rather than unresolved variables for
.promise and .debug_is_active. For .promise, a new field is added
to the FunctionState, similarly to .generator_object. This change
fixes a bug where .promise was locally shadowable by with, affecting
program semantics.

BUG=v8:5405

Review-Url: https://codereview.chromium.org/2359513002
Cr-Commit-Position: refs/heads/master@{#39566}

src/ast/ast-value-factory.h

@@ -284,11 +284,9 @@ class AstValue : public ZoneObject {
   F(done, "done")                               \
   F(dot, ".")                                   \
   F(dot_class_field_init, ".class-field-init")  \
-  F(dot_debug_is_active, ".debug_is_active")    \
   F(dot_for, ".for")                            \
   F(dot_generator_object, ".generator_object")  \
   F(dot_iterator, ".iterator")                  \
-  F(dot_promise, ".promise")                    \
   F(dot_result, ".result")                      \
   F(dot_switch_tag, ".switch_tag")              \
   F(dot_catch, ".catch")                        \

src/parsing/parser-base.h

@@ -125,7 +125,7 @@ struct FormalParametersBase {
 //   typedef Impl;
 //   // TODO(nikolaos): this one will probably go away, as it is
 //   // not related to pure parsing.
-//   typedef GeneratorVariable;
+//   typedef Variable;
 //   // Return types for traversing functions.
 //   typedef Identifier;
 //   typedef Expression;
@@ -405,16 +405,24 @@ class ParserBase {
     FunctionKind kind() const { return kind_; }
     FunctionState* outer() const { return outer_function_state_; }
 
-    void set_generator_object_variable(
-        typename Types::GeneratorVariable* variable) {
+    void set_generator_object_variable(typename Types::Variable* variable) {
       DCHECK(variable != NULL);
       DCHECK(is_resumable());
       generator_object_variable_ = variable;
     }
-    typename Types::GeneratorVariable* generator_object_variable() const {
+    typename Types::Variable* generator_object_variable() const {
       return generator_object_variable_;
     }
 
+    void set_promise_variable(typename Types::Variable* variable) {
+      DCHECK(variable != NULL);
+      DCHECK(is_async_function());
+      promise_variable_ = variable;
+    }
+    typename Types::Variable* promise_variable() const {
+      return promise_variable_;
+    }
+
     const ZoneList<DestructuringAssignment>&
         destructuring_assignments_to_rewrite() const {
       return destructuring_assignments_to_rewrite_;
@@ -490,6 +498,9 @@ class ParserBase {
     // is used by yield expressions and return statements. It is not necessary
     // for generator functions to have this variable set.
     Variable* generator_object_variable_;
+    // For async functions, this variable holds a temporary for the Promise
+    // being created as output of the async function.
+    Variable* promise_variable_;
 
     FunctionState** function_state_stack_;
     FunctionState* outer_function_state_;
@@ -1429,7 +1440,8 @@ ParserBase<Impl>::FunctionState::FunctionState(
       next_materialized_literal_index_(0),
       expected_property_count_(0),
       kind_(kind),
-      generator_object_variable_(NULL),
+      generator_object_variable_(nullptr),
+      promise_variable_(nullptr),
       function_state_stack_(function_state_stack),
       outer_function_state_(*function_state_stack),
       destructuring_assignments_to_rewrite_(16, scope->zone()),

src/parsing/parser.cc

@@ -3362,8 +3362,8 @@ Block* Parser::BuildParameterInitializationBlock(
 }
 
 Block* Parser::BuildRejectPromiseOnException(Block* inner_block, bool* ok) {
-  // var .promise = %CreatePromise();
-  // var .debug_is_active = %_DebugIsActive();
+  // .promise = %CreatePromise();
+  // .debug_is_active = %_DebugIsActive();
   // if (.debug_is_active) %DebugPushPromise(.promise);
   // try {
   //   <inner_block>
@@ -3375,32 +3375,31 @@ Block* Parser::BuildRejectPromiseOnException(Block* inner_block, bool* ok) {
   // }
   Block* result = factory()->NewBlock(nullptr, 4, true, kNoSourcePosition);
 
-  // var .promise = %CreatePromise();
+  // .promise = %CreatePromise();
   Statement* set_promise;
   {
-    DeclareVariable(ast_value_factory()->dot_promise_string(), VAR,
-                    kNoSourcePosition, CHECK_OK);
     Expression* create_promise = factory()->NewCallRuntime(
         Context::PROMISE_CREATE_INDEX,
         new (zone()) ZoneList<Expression*>(0, zone()), kNoSourcePosition);
     Assignment* assign_promise = factory()->NewAssignment(
-        Token::INIT, BuildDotPromise(), create_promise, kNoSourcePosition);
+        Token::INIT, factory()->NewVariableProxy(PromiseVariable()),
+        create_promise, kNoSourcePosition);
     set_promise =
         factory()->NewExpressionStatement(assign_promise, kNoSourcePosition);
   }
   result->statements()->Add(set_promise, zone());
 
-  // var .debug_is_active = %_DebugIsActive();
+  Variable* debug_is_active =
+      scope()->NewTemporary(ast_value_factory()->empty_string());
+  // .debug_is_active = %_DebugIsActive();
   Statement* set_debug_is_active;
   {
-    DeclareVariable(ast_value_factory()->dot_debug_is_active_string(), VAR,
-                    kNoSourcePosition, CHECK_OK);
-    Expression* debug_is_active = factory()->NewCallRuntime(
+    Expression* call_debug_is_active = factory()->NewCallRuntime(
         Runtime::kInlineDebugIsActive,
         new (zone()) ZoneList<Expression*>(0, zone()), kNoSourcePosition);
-    Assignment* assign_debug_is_active =
-        factory()->NewAssignment(Token::INIT, BuildDotDebugIsActive(),
-                                 debug_is_active, kNoSourcePosition);
+    Assignment* assign_debug_is_active = factory()->NewAssignment(
+        Token::INIT, factory()->NewVariableProxy(debug_is_active),
+        call_debug_is_active, kNoSourcePosition);
     set_debug_is_active = factory()->NewExpressionStatement(
         assign_debug_is_active, kNoSourcePosition);
   }
@@ -3410,13 +3409,13 @@ Block* Parser::BuildRejectPromiseOnException(Block* inner_block, bool* ok) {
   Statement* conditionally_debug_push_promise;
   {
     ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(1, zone());
-    args->Add(BuildDotPromise(), zone());
+    args->Add(factory()->NewVariableProxy(PromiseVariable()), zone());
     Expression* call_push_promise = factory()->NewCallRuntime(
         Runtime::kDebugPushPromise, args, kNoSourcePosition);
     Statement* debug_push_promise =
         factory()->NewExpressionStatement(call_push_promise, kNoSourcePosition);
     conditionally_debug_push_promise = factory()->NewIfStatement(
-        BuildDotDebugIsActive(), debug_push_promise,
+        factory()->NewVariableProxy(debug_is_active), debug_push_promise,
         factory()->NewEmptyStatement(kNoSourcePosition), kNoSourcePosition);
   }
   result->statements()->Add(conditionally_debug_push_promise, zone());
@@ -3455,7 +3454,7 @@ Block* Parser::BuildRejectPromiseOnException(Block* inner_block, bool* ok) {
     Statement* debug_pop_promise =
         factory()->NewExpressionStatement(call_pop_promise, kNoSourcePosition);
     Statement* conditionally_debug_pop_promise = factory()->NewIfStatement(
-        BuildDotDebugIsActive(), debug_pop_promise,
+        factory()->NewVariableProxy(debug_is_active), debug_pop_promise,
         factory()->NewEmptyStatement(kNoSourcePosition), kNoSourcePosition);
     finally_block->statements()->Add(conditionally_debug_pop_promise, zone());
   }
@@ -3481,12 +3480,13 @@ Expression* Parser::BuildCreateJSGeneratorObject(int pos, FunctionKind kind) {
 Expression* Parser::BuildResolvePromise(Expression* value, int pos) {
   // %ResolvePromise(.promise, value), .promise
   ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
-  args->Add(BuildDotPromise(), zone());
+  args->Add(factory()->NewVariableProxy(PromiseVariable()), zone());
   args->Add(value, zone());
   Expression* call_runtime =
       factory()->NewCallRuntime(Context::PROMISE_RESOLVE_INDEX, args, pos);
-  return factory()->NewBinaryOperation(Token::COMMA, call_runtime,
-                                       BuildDotPromise(), pos);
+  return factory()->NewBinaryOperation(
+      Token::COMMA, call_runtime,
+      factory()->NewVariableProxy(PromiseVariable()), pos);
 }
 
 Expression* Parser::BuildRejectPromise(Expression* value, int pos) {
@@ -3495,20 +3495,25 @@ Expression* Parser::BuildRejectPromise(Expression* value, int pos) {
   // rejection since a debug event already happened for the exception that got
   // us here.
   ZoneList<Expression*>* args = new (zone()) ZoneList<Expression*>(2, zone());
-  args->Add(BuildDotPromise(), zone());
+  args->Add(factory()->NewVariableProxy(PromiseVariable()), zone());
   args->Add(value, zone());
   Expression* call_runtime = factory()->NewCallRuntime(
       Context::REJECT_PROMISE_NO_DEBUG_EVENT_INDEX, args, pos);
-  return factory()->NewBinaryOperation(Token::COMMA, call_runtime,
-                                       BuildDotPromise(), pos);
-}
-
-VariableProxy* Parser::BuildDotPromise() {
-  return NewUnresolved(ast_value_factory()->dot_promise_string(), VAR);
+  return factory()->NewBinaryOperation(
+      Token::COMMA, call_runtime,
+      factory()->NewVariableProxy(PromiseVariable()), pos);
 }
 
-VariableProxy* Parser::BuildDotDebugIsActive() {
-  return NewUnresolved(ast_value_factory()->dot_debug_is_active_string(), VAR);
+Variable* Parser::PromiseVariable() {
+  // Based on the various compilation paths, there are many different code
+  // paths which may be the first to access the Promise temporary. Whichever
+  // comes first should create it and stash it in the FunctionState.
+  Variable* promise = function_state_->promise_variable();
+  if (function_state_->promise_variable() == nullptr) {
+    promise = scope()->NewTemporary(ast_value_factory()->empty_string());
+    function_state_->set_promise_variable(promise);
+  }
+  return promise;
 }
 
 ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
@@ -4562,10 +4567,9 @@ Expression* Parser::ExpressionListToExpression(ZoneList<Expression*>* args) {
 
 Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) {
   // yield do {
-  //   promise_tmp = .promise;
   //   tmp = <operand>;
-  //   %AsyncFunctionAwait(.generator_object, tmp, promise_tmp);
-  //   promise_tmp
+  //   %AsyncFunctionAwait(.generator_object, tmp, .promise);
+  //   .promise
   // }
   // The value of the expression is returned to the caller of the async
   // function for the first yield statement; for this, .promise is the
@@ -4590,15 +4594,9 @@ Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) {
 
   const int nopos = kNoSourcePosition;
 
-  Block* do_block = factory()->NewBlock(nullptr, 3, false, nopos);
+  Block* do_block = factory()->NewBlock(nullptr, 2, false, nopos);
 
-  Variable* promise_temp_var =
-      NewTemporary(ast_value_factory()->empty_string());
-  Expression* promise_assignment = factory()->NewAssignment(
-      Token::ASSIGN, factory()->NewVariableProxy(promise_temp_var),
-      BuildDotPromise(), nopos);
-  do_block->statements()->Add(
-      factory()->NewExpressionStatement(promise_assignment, nopos), zone());
+  Variable* promise = PromiseVariable();
 
   // Wrap value evaluation to provide a break location.
   Variable* temp_var = NewTemporary(ast_value_factory()->empty_string());
@@ -4614,8 +4612,7 @@ Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) {
       factory()->NewVariableProxy(generator_object_variable);
   async_function_await_args->Add(generator_object, zone());
   async_function_await_args->Add(factory()->NewVariableProxy(temp_var), zone());
-  async_function_await_args->Add(factory()->NewVariableProxy(promise_temp_var),
-                                 zone());
+  async_function_await_args->Add(factory()->NewVariableProxy(promise), zone());
 
   // The parser emits calls to AsyncFunctionAwaitCaught, but the
   // AstNumberingVisitor will rewrite this to AsyncFunctionAwaitUncaught
@@ -4628,8 +4625,7 @@ Expression* Parser::RewriteAwaitExpression(Expression* value, int await_pos) {
       zone());
 
   // Wrap await to provide a break location between value evaluation and yield.
-  Expression* do_expr =
-      factory()->NewDoExpression(do_block, promise_temp_var, nopos);
+  Expression* do_expr = factory()->NewDoExpression(do_block, promise, nopos);
 
   generator_object = factory()->NewVariableProxy(generator_object_variable);
   return factory()->NewYield(generator_object, do_expr, nopos,

src/parsing/parser.h

@@ -143,7 +143,7 @@ struct ParserTypes<Parser> {
   typedef ParserBase<Parser> Base;
   typedef Parser Impl;
 
-  typedef Variable GeneratorVariable;
+  typedef v8::internal::Variable Variable;
 
   // Return types for traversing functions.
   typedef const AstRawString* Identifier;
@@ -583,8 +583,7 @@ class Parser : public ParserBase<Parser> {
   Expression* BuildCreateJSGeneratorObject(int pos, FunctionKind kind);
   Expression* BuildResolvePromise(Expression* value, int pos);
   Expression* BuildRejectPromise(Expression* value, int pos);
-  VariableProxy* BuildDotPromise();
-  VariableProxy* BuildDotDebugIsActive();
+  Variable* PromiseVariable();
 
   // Generic AST generator for throwing errors from compiled code.
   Expression* NewThrowError(Runtime::FunctionId function_id,

src/parsing/preparser.h

@@ -705,7 +705,7 @@ struct ParserTypes<PreParser> {
   typedef PreParser Impl;
 
   // PreParser doesn't need to store generator variables.
-  typedef void GeneratorVariable;
+  typedef void Variable;
 
   // Return types for traversing functions.
   typedef PreParserIdentifier Identifier;

test/mjsunit/regress/regress-5405.js

+// Copyright 2016 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.
+
+// Flags: --harmony-async-await --allow-natives-syntax
+
+let log = [];
+
+(async function() {
+  with ({get ['.promise']() { log.push('async') }}) {
+    return 10;
+  }
+})();
+%RunMicrotasks();
+
+(function() {
+  with ({get ['.new.target']() { log.push('new.target') }}) {
+    return new.target;
+  }
+})();
+
+(function() {
+  with ({get ['this']() { log.push('this') }}) {
+    return this;
+  }
+})();
+
+// TODO(v8:5405): The log should actually be empty and not
+// contain new.target.
+assertArrayEquals(['new.target'], log);