[parser] use-strict directives in function body affect init block

src/parser.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/v8.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/ast-literal-reindexer.h"
 #include "src/bailout-reason.h"
@@ skipping 4289 matching lines @@
       RelocInfo::kNoPosition);
   IfStatement* if_statement = factory()->NewIfStatement(
       condition, factory()->NewExpressionStatement(throw_type_error,
                                                    RelocInfo::kNoPosition),
       factory()->NewEmptyStatement(RelocInfo::kNoPosition),
       RelocInfo::kNoPosition);
   return if_statement;
 }
 
 
+bool Parser::NeedsParameterInitializationBlock(

[rossberg, 2015/07/16 12:53:47]

Make this into IsSimpleParameterList, to match what's in the spec. In
particular, it needs to check for rest as well.

[caitp (gmail), 2015/07/16 13:03:57]

Acknowledged.

+    const ParserFormalParameterParsingState& formal_parameters) {
+  for (auto parameter : formal_parameters.params) {
+    if (parameter.pattern != nullptr) return true;
+  }
+  return false;
+}
+
+
 Block* Parser::BuildParameterInitializationBlock(
     const ParserFormalParameterParsingState& formal_parameters, bool* ok) {
+  DCHECK(NeedsParameterInitializationBlock(formal_parameters));
   DCHECK(scope_->is_function_scope());
-  Block* init_block = nullptr;
+  Block* init_block =
+      factory()->NewBlock(NULL, 1, true, RelocInfo::kNoPosition);
   for (auto parameter : formal_parameters.params) {
     if (parameter.pattern == nullptr) continue;
-    if (init_block == nullptr) {
-      init_block = factory()->NewBlock(NULL, 1, true, RelocInfo::kNoPosition);
-    }
-
     DeclarationDescriptor descriptor;
     descriptor.declaration_kind = DeclarationDescriptor::PARAMETER;
     descriptor.parser = this;
     descriptor.declaration_scope = scope_;
     descriptor.scope = scope_;
     descriptor.mode = LET;
     descriptor.is_const = false;
     descriptor.needs_init = true;
     descriptor.declaration_pos = parameter.pattern->position();
     descriptor.initialization_pos = parameter.pattern->position();
     descriptor.init_op = Token::INIT_LET;
     DeclarationParsingResult::Declaration decl(
         parameter.pattern, parameter.pattern->position(),
         factory()->NewVariableProxy(parameter.var));
     PatternRewriter::DeclareAndInitializeVariables(init_block, &descriptor,
                                                    &decl, nullptr, CHECK_OK);
   }
   return init_block;
 }
 
 
 ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
     const AstRawString* function_name, int pos,
     const ParserFormalParameterParsingState& formal_parameters, Variable* fvar,
     Token::Value fvar_init_op, FunctionKind kind, bool* ok) {
   // Everything inside an eagerly parsed function will be parsed eagerly
   // (see comment above).
   ParsingModeScope parsing_mode(this, PARSE_EAGERLY);
   ZoneList<Statement*>* result = new(zone()) ZoneList<Statement*>(8, zone());
+  Statement* init_fvar = nullptr;
+
   if (fvar != NULL) {
     VariableProxy* fproxy = scope_->NewUnresolved(factory(), function_name);
     fproxy->BindTo(fvar);
-    result->Add(factory()->NewExpressionStatement(
-        factory()->NewAssignment(fvar_init_op,
-                                 fproxy,
+    init_fvar = factory()->NewExpressionStatement(
+        factory()->NewAssignment(fvar_init_op, fproxy,
                                  factory()->NewThisFunction(pos),
                                  RelocInfo::kNoPosition),
-        RelocInfo::kNoPosition), zone());
-  }
-
-
-  // For concise constructors, check that they are constructed,
-  // not called.
-  if (i::IsConstructor(kind)) {
-    AddAssertIsConstruct(result, pos);
+        RelocInfo::kNoPosition);
   }
 
   ZoneList<Statement*>* body = result;
   Scope* inner_scope = nullptr;
   Block* inner_block = nullptr;
-  Block* init_block =
-      BuildParameterInitializationBlock(formal_parameters, CHECK_OK);
-  if (init_block != nullptr) {
-    body->Add(init_block, zone());
-    // Wrap the actual function body into an inner scope.
-    inner_block = factory()->NewBlock(NULL, 8, true, RelocInfo::kNoPosition);
-    body->Add(inner_block, zone());
-    body = inner_block->statements();
+
+  bool needs_inner_block = NeedsParameterInitializationBlock(formal_parameters);
+  if (needs_inner_block) {
     inner_scope = NewScope(scope_, BLOCK_SCOPE);
     inner_scope->set_is_declaration_scope();
     inner_scope->set_start_position(scanner()->location().beg_pos);
   }
 
   {
     BlockState block_state(&scope_, inner_scope ? inner_scope : scope_);
 
+    if (!needs_inner_block) {
+      if (fvar != nullptr) {

[rossberg, 2015/07/16 12:53:47]

Hm, why do you need to move (and duplicate) fvar initialisation and IsConstruct
assert?

[caitp (gmail), 2015/07/16 13:03:57]

it seems that these need to happen before the init block. I suppose it could
just use `result->insertAt(0)` instead though

[rossberg, 2015/07/16 13:21:05]

Not sure I understand. They were happening before the init block before, AFAICT?
Why did that need to change at all?

[caitp (gmail), 2015/07/16 14:44:31]

I've refactored this, it's much cleaner now

+        body->Add(init_fvar, zone());
+      }
+      // For concise constructors, check that they are constructed,
+      // not called.
+      if (i::IsConstructor(kind)) {
+        AddAssertIsConstruct(body, pos);
+      }
+    }
+
     // For generators, allocate and yield an iterator on function entry.
     if (IsGeneratorFunction(kind)) {
       ZoneList<Expression*>* arguments =
           new(zone()) ZoneList<Expression*>(0, zone());
       CallRuntime* allocation = factory()->NewCallRuntime(
           ast_value_factory()->empty_string(),
           Runtime::FunctionForId(Runtime::kCreateJSGeneratorObject), arguments,
           pos);
       VariableProxy* init_proxy = factory()->NewVariableProxy(
           function_state_->generator_object_variable());
@@ skipping 24 matching lines @@
       body->Add(
           factory()->NewReturnStatement(
               this->ThisExpression(scope_, factory(), RelocInfo::kNoPosition),
               RelocInfo::kNoPosition),
               zone());
     }
   }
 
   Expect(Token::RBRACE, CHECK_OK);
   scope_->set_end_position(scanner()->location().end_pos);
-  if (inner_scope != nullptr) {
-    DCHECK(inner_block != nullptr);
+
+  if (needs_inner_block) {
+    DCHECK_NOT_NULL(inner_scope);
+    scope_->SetLanguageMode(inner_scope->language_mode());
+    Block* init_block =
+        BuildParameterInitializationBlock(formal_parameters, CHECK_OK);
+    DCHECK_NOT_NULL(init_block);
+    inner_block =
+        factory()->NewBlock(NULL, body->length(), true, RelocInfo::kNoPosition);
+    inner_block->statements()->AddAll(*body, zone());
     inner_scope->set_end_position(scanner()->location().end_pos);
     inner_scope = inner_scope->FinalizeBlockScope();
     inner_block->set_scope(inner_scope);
+
+    result = new (zone()) ZoneList<Statement*>(8, zone());
+    if (fvar != nullptr) {
+      result->Add(init_fvar, zone());
+    }
+    // For concise constructors, check that they are constructed,
+    // not called.
+    if (i::IsConstructor(kind)) {
+      AddAssertIsConstruct(result, pos);
+    }
+    result->Add(init_block, zone());
+    // Wrap the actual function body into an inner scope.
+    result->Add(inner_block, zone());
   }
 
   return result;
 }
 
 
 PreParser::PreParseResult Parser::ParseLazyFunctionBodyWithPreParser(
     SingletonLogger* logger, Scanner::BookmarkScope* bookmark) {
   // This function may be called on a background thread too; record only the
   // main thread preparse times.
@@ skipping 1532 matching lines @@
 Expression* Parser::SpreadCallNew(Expression* function,
                                   ZoneList<v8::internal::Expression*>* args,
                                   int pos) {
   args->InsertAt(0, function, zone());
 
   return factory()->NewCallRuntime(
       ast_value_factory()->reflect_construct_string(), NULL, args, pos);
 }
 }  // namespace internal
 }  // namespace v8