Add strict mode, sloppy mode and strong mode UseCounters

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/parser.h"
 
 #include "src/api.h"
 #include "src/ast.h"
 #include "src/ast-literal-reindexer.h"
 #include "src/bailout-reason.h"
@@ skipping 328 matching lines @@
                                             LanguageMode language_mode) {
   int materialized_literal_count = -1;
   int expected_property_count = -1;
   int parameter_count = 0;
   const AstRawString* name = ast_value_factory()->empty_string();
 
 
   FunctionKind kind = call_super ? FunctionKind::kDefaultSubclassConstructor
                                  : FunctionKind::kDefaultBaseConstructor;
   Scope* function_scope = NewScope(scope, FUNCTION_SCOPE, kind);
-  function_scope->SetLanguageMode(
-      static_cast<LanguageMode>(language_mode | STRICT));
+  SetLanguageMode(function_scope,
+                  static_cast<LanguageMode>(language_mode | STRICT));
   // Set start and end position to the same value
   function_scope->set_start_position(pos);
   function_scope->set_end_position(pos);
   ZoneList<Statement*>* body = NULL;
 
   {
     AstNodeFactory function_factory(ast_value_factory());
     FunctionState function_state(&function_state_, &scope_, function_scope,
                                  kind, &function_factory);
 
@@ skipping 681 matching lines @@
     }
 
     scope->set_start_position(0);
 
     // Enter 'scope' with the given parsing mode.
     ParsingModeScope parsing_mode_scope(this, parsing_mode);
     AstNodeFactory function_factory(ast_value_factory());
     FunctionState function_state(&function_state_, &scope_, scope,
                                  kNormalFunction, &function_factory);
 
+    // Don't count the mode in the use counters--give the program a chance

[adamk, 2015/11/04 00:53:54]

This doesn't sound right to me. I suspect it means we wouldn't measure a strict
eval as strict (see compiler.cc for where ParseInfo::language_mode_ is set).

[Dan Ehrenberg, 2015/11/04 17:55:35]

I think that's OK--the exact counting doesn't matter, it just matters whether we
have gotten that the page includes strict code and no sloppy code. I think it'd
get that right in this case.

+    // to enable script/module-wide strict/strong mode below.
     scope_->SetLanguageMode(info->language_mode());
     ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(16, zone());
     bool ok = true;
     int beg_pos = scanner()->location().beg_pos;
     if (info->is_module()) {
       ParseModuleItemList(body, &ok);
     } else {
       ParseStatementList(body, Token::EOS, &ok);
     }
 
@@ skipping 122 matching lines @@
     FunctionLiteral::FunctionType function_type = shared_info->is_expression()
         ? (shared_info->is_anonymous()
               ? FunctionLiteral::ANONYMOUS_EXPRESSION
               : FunctionLiteral::NAMED_EXPRESSION)
         : FunctionLiteral::DECLARATION;
     bool ok = true;
 
     if (shared_info->is_arrow()) {
       Scope* scope =
           NewScope(scope_, FUNCTION_SCOPE, FunctionKind::kArrowFunction);
-      scope->SetLanguageMode(shared_info->language_mode());
+      SetLanguageMode(scope, shared_info->language_mode());
       scope->set_start_position(shared_info->start_position());
       ExpressionClassifier formals_classifier;
       ParserFormalParameters formals(scope);
       Checkpoint checkpoint(this);
       {
         // Parsing patterns as variable reference expression creates
         // NewUnresolved references in current scope. Entrer arrow function
         // scope for formal parameter parsing.
         BlockState block_state(&scope_, scope);
         if (Check(Token::LPAREN)) {
@@ skipping 124 matching lines @@
         bool use_strong_found =
             allow_strong_mode() &&
             literal->raw_value()->AsString() ==
                 ast_value_factory()->use_strong_string() &&
             token_loc.end_pos - token_loc.beg_pos ==
                 ast_value_factory()->use_strong_string()->length() + 2;
         if (use_strict_found || use_strong_found) {
           // Strong mode implies strict mode. If there are several "use strict"
           // / "use strong" directives, do the strict mode changes only once.
           if (is_sloppy(scope_->language_mode())) {
-            scope_->SetLanguageMode(
-                static_cast<LanguageMode>(scope_->language_mode() | STRICT));
+            EnableLanguageMode(STRICT);

[adamk, 2015/11/03 22:23:15]

I was expecting this function to be the only place touched in this patch. I
guess it depends on what exactly we're trying to measure: the use of directives,
or just the existence of strict code. I was expecting the former (which would
exclude, e.g., classes from this measurement).

[Dan Ehrenberg, 2015/11/04 17:55:35]

I think it'd be more useful to measure how often strict code happens. This would
help guide both our optimization and language design efforts. Not sure exactly
why we'd care about the explicit declarations.

[adamk, 2015/11/04 20:18:53]

I was assuming we were trying to measure developer intentions, I guess. Maybe
rossberg's a better reviewer for this patch, given it was his idea originally to
add a use strict counter? It seems there are a variety of reasons we might want
these counters and what we want to measure with them, and different ideas would
result in different approaches.

           }
 
           if (use_strong_found) {
-            scope_->SetLanguageMode(
-                static_cast<LanguageMode>(scope_->language_mode() | STRONG));
+            EnableLanguageMode(STRONG);
             if (IsClassConstructor(function_state_->kind())) {
               // "use strong" cannot occur in a class constructor body, to avoid
               // unintuitive strong class object semantics.
               ParserTraits::ReportMessageAt(
                   token_loc, MessageTemplate::kStrongConstructorDirective);
               *ok = false;
               return nullptr;
             }
           }
           if (!scope_->HasSimpleParameters()) {
@@ skipping 15 matching lines @@
           // probably not a win.
           if (scope_->is_eval_scope()) mode_ = PARSE_EAGERLY;
         } else if (literal->raw_value()->AsString() ==
                        ast_value_factory()->use_asm_string() &&
                    token_loc.end_pos - token_loc.beg_pos ==
                        ast_value_factory()->use_asm_string()->length() + 2) {
           // Store the usage count; The actual use counter on the isolate is
           // incremented after parsing is done.
           ++use_counts_[v8::Isolate::kUseAsm];
           scope_->SetAsmModule();
+        } else {
+          // Should not change mode, but will increment UseCounter
+          // if appropriate. Ditto usages below.
+          EnableLanguageMode(SLOPPY);

[adamk, 2015/11/04 00:53:54]

I think the weird thing about this, to me, is that it's a no-op except for the
use-counter. Same goes for the EnableLanguageMode things below.

[Dan Ehrenberg, 2015/11/04 17:55:35]

Well, I could write the code instead like

else if (is_sloppy(language_mode_)) {
  ++use_counters[kSloppyMode];
}

Just a matter of taste which is cleaner. I thought this way was cleaner (it's
definitely less code, and I though the comment cleared it up) but I see how it
can be viewed otherwise.

         }
       } else {
         // End of the directive prologue.
         directive_prologue = false;
+        EnableLanguageMode(SLOPPY);
       }
+    } else {
+      EnableLanguageMode(SLOPPY);
     }
 
     body->Add(stat, zone());
   }
 
   return 0;
 }
 
 
 Statement* Parser::ParseStatementListItem(bool* ok) {
@@ skipping 56 matching lines @@
 
 void* Parser::ParseModuleItemList(ZoneList<Statement*>* body, bool* ok) {
   // (Ecma 262 6th Edition, 15.2):
   // Module :
   //    ModuleBody?
   //
   // ModuleBody :
   //    ModuleItem*
 
   DCHECK(scope_->is_module_scope());
-  scope_->SetLanguageMode(
-      static_cast<LanguageMode>(scope_->language_mode() | STRICT));
+  EnableLanguageMode(STRICT);
 
   while (peek() != Token::EOS) {
     Statement* stat = ParseModuleItem(CHECK_OK);
     if (stat && !stat->IsEmpty()) {
       body->Add(stat, zone());
     }
   }
 
   // Check that all exports are bound.
   ModuleDescriptor* descriptor = scope_->module();
@@ skipping 2702 matching lines @@
   //   nested function, and hoisting works normally relative to that.
   Scope* declaration_scope = scope_->DeclarationScope();
   Scope* original_declaration_scope = original_scope_->DeclarationScope();
   Scope* scope = function_type == FunctionLiteral::DECLARATION &&
                          is_sloppy(language_mode) &&
                          !allow_harmony_sloppy_function() &&
                          (original_scope_ == original_declaration_scope ||
                           declaration_scope != original_declaration_scope)
                      ? NewScope(declaration_scope, FUNCTION_SCOPE, kind)
                      : NewScope(scope_, FUNCTION_SCOPE, kind);
-  scope->SetLanguageMode(language_mode);
+  SetLanguageMode(scope, language_mode);
   ZoneList<Statement*>* body = NULL;
   int arity = -1;
   int materialized_literal_count = -1;
   int expected_property_count = -1;
   DuplicateFinder duplicate_finder(scanner()->unicode_cache());
   ExpressionClassifier formals_classifier(&duplicate_finder);
   FunctionLiteral::EagerCompileHint eager_compile_hint =
       parenthesized_function_ ? FunctionLiteral::kShouldEagerCompile
                               : FunctionLiteral::kShouldLazyCompile;
   bool should_be_used_once_hint = false;
@@ skipping 211 matching lines @@
       scanner()->SeekForward(entry.end_pos() - 1);
 
       scope_->set_end_position(entry.end_pos());
       Expect(Token::RBRACE, ok);
       if (!*ok) {
         return;
       }
       total_preparse_skipped_ += scope_->end_position() - function_block_pos;
       *materialized_literal_count = entry.literal_count();
       *expected_property_count = entry.property_count();
-      scope_->SetLanguageMode(entry.language_mode());
+      SetLanguageMode(scope_, entry.language_mode());

[adamk, 2015/11/04 00:53:54]

Why isn't this EnableLanguageMode()?

[Dan Ehrenberg, 2015/11/04 17:55:35]

Because it doesn't bitor the language mode with the scope_'s previous mode, and
I didn't read enough of the surrounding code to know whether or not I can prove
that that doesn't matter.

       if (entry.uses_super_property()) scope_->RecordSuperPropertyUsage();
       if (entry.calls_eval()) scope_->RecordEvalCall();
       return;
     }
     cached_parse_data_->Reject();
   }
   // With no cached data, we partially parse the function, without building an
   // AST. This gathers the data needed to build a lazy function.
   SingletonLogger logger;
   PreParser::PreParseResult result =
@@ skipping 15 matching lines @@
     return;
   }
   scope_->set_end_position(logger.end());
   Expect(Token::RBRACE, ok);
   if (!*ok) {
     return;
   }
   total_preparse_skipped_ += scope_->end_position() - function_block_pos;
   *materialized_literal_count = logger.literals();
   *expected_property_count = logger.properties();
-  scope_->SetLanguageMode(logger.language_mode());
+  SetLanguageMode(scope_, logger.language_mode());

[adamk, 2015/11/04 00:53:54]

Same here as above, seems like this is EnableLanguageMode.

I think this is the reason that "Set" and "Enable" aren't helpful to me as names
for these operations.

[Dan Ehrenberg, 2015/11/04 17:55:35]

EnableLanguageMode means "the language mode should be at least this strong"--it
bitor's the language mode together with the scope's mode. The code takes about
two lines and it's a little verbose. I was thinking of making a single-argument
version of SetLanguageMode which got the scope from scope_ but it seemed
unnecessary.

   if (logger.uses_super_property()) {
     scope_->RecordSuperPropertyUsage();
   }
   if (logger.calls_eval()) {
     scope_->RecordEvalCall();
   }
   if (produce_cached_parse_data()) {
     DCHECK(log_);
     // Position right after terminal '}'.
     int body_end = scanner()->location().end_pos;
@@ skipping 272 matching lines @@
               zone());
     }
   }
 
   Expect(Token::RBRACE, CHECK_OK);
   scope_->set_end_position(scanner()->location().end_pos);
 
   if (!parameters.is_simple) {
     DCHECK_NOT_NULL(inner_scope);
     DCHECK_EQ(body, inner_block->statements());
-    scope_->SetLanguageMode(inner_scope->language_mode());
+    SetLanguageMode(scope_, inner_scope->language_mode());
     Block* init_block = BuildParameterInitializationBlock(parameters, CHECK_OK);
     DCHECK_NOT_NULL(init_block);
 
     inner_scope->set_end_position(scanner()->location().end_pos);
     inner_scope = inner_scope->FinalizeBlockScope();
     if (inner_scope != nullptr) {
       CheckConflictingVarDeclarations(inner_scope, CHECK_OK);
       InsertShadowingVarBindingInitializers(inner_block);
     }
 
@@ skipping 83 matching lines @@
     return NULL;
   }
   if (is_strong(language_mode()) && IsUndefined(name)) {
     ReportMessageAt(class_name_location, MessageTemplate::kStrongUndefined);
     *ok = false;
     return NULL;
   }
 
   Scope* block_scope = NewScope(scope_, BLOCK_SCOPE);
   BlockState block_state(&scope_, block_scope);
-  scope_->SetLanguageMode(
-      static_cast<LanguageMode>(scope_->language_mode() | STRICT));
+  EnableLanguageMode(STRICT);
   scope_->SetScopeName(name);
 
   VariableProxy* proxy = NULL;
   if (name != NULL) {
     proxy = NewUnresolved(name, CONST);
     const bool is_class_declaration = true;
     Declaration* declaration = factory()->NewVariableDeclaration(
         proxy, CONST, block_scope, pos, is_class_declaration,
         scope_->class_declaration_group_start());
     Declare(declaration, DeclarationDescriptor::NORMAL, true, CHECK_OK);
@@ skipping 1516 matching lines @@
 }
 
 
 Expression* Parser::SpreadCallNew(Expression* function,
                                   ZoneList<v8::internal::Expression*>* args,
                                   int pos) {
   args->InsertAt(0, function, zone());
 
   return factory()->NewCallRuntime(Context::REFLECT_CONSTRUCT_INDEX, args, pos);
 }
+
+
+void Parser::SetLanguageMode(Scope* scope, LanguageMode mode) {
+  v8::Isolate::UseCounterFeature feature;
+  if (is_sloppy(mode))
+    feature = v8::Isolate::kSloppyMode;
+  else if (is_strict(mode))
+    feature = v8::Isolate::kStrictMode;
+  else if (is_strong(mode))

[adamk, 2015/11/03 22:23:15]

This has to go before is_strict, since strong mode implies strict mode.

[Dan Ehrenberg, 2015/11/04 17:55:36]

Yeah, this needs to be fixed.

[Dan Ehrenberg, 2015/11/04 20:27:53]

Fixed

+    feature = v8::Isolate::kStrongMode;
+  else
+    UNREACHABLE();
+  ++use_counts_[feature];
+  scope->SetLanguageMode(mode);
+}
+
+
+void Parser::EnableLanguageMode(LanguageMode mode) {
+  SetLanguageMode(scope_,
+                  static_cast<LanguageMode>(scope_->language_mode() | mode));
+}
+
 }  // namespace internal
 }  // namespace v8