[strong] Declaration-after-use errors.

We cannot yet detect use-before-declaration in general, because for that we'd
need to analyze the context when compiling. But we can detect an error case
where we first see a use, then a declaration.

For this, I also added end position tracking (needed for error messages) to
VariableProxy.

Note: the position naming is completely inconsistent: start_position &
end_position, position & end_position, pos & end_pos, beg_pos & end_pos, to name
a few. This doesn't fix all of it, but tries to unify towards start_position &
end_position whenever possible w/ minimal changes.

BUG=

Review URL: https://codereview.chromium.org/943543002

Cr-Commit-Position: refs/heads/master@{#26880}

src/ast.cc

@@ -59,24 +59,27 @@ bool Expression::IsUndefinedLiteral(Isolate* isolate) const {
 }
 
 
-VariableProxy::VariableProxy(Zone* zone, Variable* var, int position)
-    : Expression(zone, position),
+VariableProxy::VariableProxy(Zone* zone, Variable* var, int start_position,
+                             int end_position)
+    : Expression(zone, start_position),
       bit_field_(IsThisField::encode(var->is_this()) |
                  IsAssignedField::encode(false) |
                  IsResolvedField::encode(false)),
       variable_feedback_slot_(FeedbackVectorICSlot::Invalid()),
-      raw_name_(var->raw_name()) {
+      raw_name_(var->raw_name()),
+      end_position_(end_position) {
   BindTo(var);
 }
 
 
 VariableProxy::VariableProxy(Zone* zone, const AstRawString* name, bool is_this,
-                             int position)
-    : Expression(zone, position),
+                             int start_position, int end_position)
+    : Expression(zone, start_position),
       bit_field_(IsThisField::encode(is_this) | IsAssignedField::encode(false) |
                  IsResolvedField::encode(false)),
       variable_feedback_slot_(FeedbackVectorICSlot::Invalid()),
-      raw_name_(name) {}
+      raw_name_(name),
+      end_position_(end_position) {}
 
 
 void VariableProxy::BindTo(Variable* var) {

src/ast.h

@@ -1631,6 +1631,8 @@ class VariableProxy FINAL : public Expression {
     bit_field_ = IsResolvedField::update(bit_field_, true);
   }
 
+  int end_position() const { return end_position_; }
+
   // Bind this proxy to the variable var.
   void BindTo(Variable* var);
 
@@ -1653,10 +1655,11 @@ class VariableProxy FINAL : public Expression {
   }
 
  protected:
-  VariableProxy(Zone* zone, Variable* var, int position);
+  VariableProxy(Zone* zone, Variable* var, int start_position,
+                int end_position);
 
   VariableProxy(Zone* zone, const AstRawString* name, bool is_this,
-                int position);
+                int start_position, int end_position);
 
   class IsThisField : public BitField8<bool, 0, 1> {};
   class IsAssignedField : public BitField8<bool, 1, 1> {};
@@ -1670,6 +1673,10 @@ class VariableProxy FINAL : public Expression {
     const AstRawString* raw_name_;  // if !is_resolved_
     Variable* var_;                 // if is_resolved_
   };
+  // Position is stored in the AstNode superclass, but VariableProxy needs to
+  // know its end position too (for error messages). It cannot be inferred from
+  // the variable name length because it can contain escapes.
+  int end_position_;
 };
 
 
@@ -3353,14 +3360,16 @@ class AstNodeFactory FINAL BASE_EMBEDDED {
   }
 
   VariableProxy* NewVariableProxy(Variable* var,
-                                  int pos = RelocInfo::kNoPosition) {
-    return new (zone_) VariableProxy(zone_, var, pos);
+                                  int start_position = RelocInfo::kNoPosition,
+                                  int end_position = RelocInfo::kNoPosition) {
+    return new (zone_) VariableProxy(zone_, var, start_position, end_position);
   }
 
-  VariableProxy* NewVariableProxy(const AstRawString* name,
-                                  bool is_this,
-                                  int position = RelocInfo::kNoPosition) {
-    return new (zone_) VariableProxy(zone_, name, is_this, position);
+  VariableProxy* NewVariableProxy(const AstRawString* name, bool is_this,
+                                  int start_position = RelocInfo::kNoPosition,
+                                  int end_position = RelocInfo::kNoPosition) {
+    return new (zone_)
+        VariableProxy(zone_, name, is_this, start_position, end_position);
   }
 
   Property* NewProperty(Expression* obj, Expression* key, int pos) {

src/messages.js

@@ -168,6 +168,7 @@ var kMessages = {
   strong_var:                    ["Please don't use 'var' in strong mode, use 'let' or 'const' instead"],
   strong_for_in:                 ["Please don't use 'for'-'in' loops in strong mode, use 'for'-'of' instead"],
   strong_empty:                  ["Please don't use empty sub-statements in strong mode, make them explicit with '{}' instead"],
+  strong_use_before_declaration: ["Please declare variable '", "%0", "' before use in strong mode"],
   sloppy_lexical:                ["Block-scoped declarations (let, const, function, class) not yet supported outside strict mode"],
   malformed_arrow_function_parameter_list: ["Malformed arrow function parameter list"],
   generator_poison_pill:         ["'caller' and 'arguments' properties may not be accessed on generator functions."],

src/parser.h

@@ -542,7 +542,8 @@ class ParserTraits {
                                  int end_pos);
   Literal* ExpressionFromLiteral(Token::Value token, int pos, Scanner* scanner,
                                  AstNodeFactory* factory);
-  Expression* ExpressionFromIdentifier(const AstRawString* name, int pos,
+  Expression* ExpressionFromIdentifier(const AstRawString* name,
+                                       int start_position, int end_position,
                                        Scope* scope, AstNodeFactory* factory);
   Expression* ExpressionFromString(int pos, Scanner* scanner,
                                    AstNodeFactory* factory);
@@ -799,7 +800,7 @@ class Parser : public ParserBase<ParserTraits> {
 
   // Parser support
   VariableProxy* NewUnresolved(const AstRawString* name, VariableMode mode);
-  void Declare(Declaration* declaration, bool resolve, bool* ok);
+  Variable* Declare(Declaration* declaration, bool resolve, bool* ok);
 
   bool TargetStackContainsLabel(const AstRawString* label);
   BreakableStatement* LookupBreakTarget(const AstRawString* label, bool* ok);

src/preparser.h

@@ -1408,8 +1408,8 @@ class PreParserTraits {
   }
 
   static PreParserExpression ExpressionFromIdentifier(
-      PreParserIdentifier name, int pos, Scope* scope,
-      PreParserFactory* factory) {
+      PreParserIdentifier name, int start_position, int end_position,
+      Scope* scope, PreParserFactory* factory) {
     return PreParserExpression::FromIdentifier(name);
   }
 
@@ -1859,14 +1859,15 @@ ParserBase<Traits>::ParsePrimaryExpression(bool* ok) {
   //   '(' Expression ')'
   //   TemplateLiteral
 
-  int pos = peek_position();
+  int beg_pos = scanner()->peek_location().beg_pos;
+  int end_pos = scanner()->peek_location().end_pos;
   ExpressionT result = this->EmptyExpression();
   Token::Value token = peek();
   switch (token) {
     case Token::THIS: {
       Consume(Token::THIS);
       scope_->RecordThisUsage();
-      result = this->ThisExpression(scope_, factory(), pos);
+      result = this->ThisExpression(scope_, factory(), beg_pos);
       break;
     }
 
@@ -1875,7 +1876,8 @@ ParserBase<Traits>::ParsePrimaryExpression(bool* ok) {
     case Token::FALSE_LITERAL:
     case Token::NUMBER:
       Next();
-      result = this->ExpressionFromLiteral(token, pos, scanner(), factory());
+      result =
+          this->ExpressionFromLiteral(token, beg_pos, scanner(), factory());
       break;
 
     case Token::IDENTIFIER:
@@ -1885,13 +1887,14 @@ ParserBase<Traits>::ParsePrimaryExpression(bool* ok) {
     case Token::FUTURE_STRICT_RESERVED_WORD: {
       // Using eval or arguments in this context is OK even in strict mode.
       IdentifierT name = ParseIdentifier(kAllowEvalOrArguments, CHECK_OK);
-      result = this->ExpressionFromIdentifier(name, pos, scope_, factory());
+      result = this->ExpressionFromIdentifier(name, beg_pos, end_pos, scope_,
+                                              factory());
       break;
     }
 
     case Token::STRING: {
       Consume(Token::STRING);
-      result = this->ExpressionFromString(pos, scanner(), factory());
+      result = this->ExpressionFromString(beg_pos, scanner(), factory());
       break;
     }
 
@@ -1918,7 +1921,7 @@ ParserBase<Traits>::ParsePrimaryExpression(bool* ok) {
         // for which an empty parameter list "()" is valid input.
         Consume(Token::RPAREN);
         result = this->ParseArrowFunctionLiteral(
-            pos, this->EmptyArrowParamList(), CHECK_OK);
+            beg_pos, this->EmptyArrowParamList(), CHECK_OK);
       } else {
         // Heuristically try to detect immediately called functions before
         // seeing the call parentheses.
@@ -1953,8 +1956,8 @@ ParserBase<Traits>::ParsePrimaryExpression(bool* ok) {
 
     case Token::TEMPLATE_SPAN:
     case Token::TEMPLATE_TAIL:
-      result =
-          this->ParseTemplateLiteral(Traits::NoTemplateTag(), pos, CHECK_OK);
+      result = this->ParseTemplateLiteral(Traits::NoTemplateTag(), beg_pos,
+                                          CHECK_OK);
       break;
 
     case Token::MOD:
@@ -2100,7 +2103,8 @@ ParserBase<Traits>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
   bool is_generator = allow_harmony_object_literals_ && Check(Token::MUL);
 
   Token::Value name_token = peek();
-  int next_pos = peek_position();
+  int next_beg_pos = scanner()->peek_location().beg_pos;
+  int next_end_pos = scanner()->peek_location().end_pos;
   ExpressionT name_expression = ParsePropertyName(
       &name, &is_get, &is_set, &name_is_static, is_computed_name,
       CHECK_OK_CUSTOM(EmptyObjectLiteralProperty));
@@ -2195,7 +2199,8 @@ ParserBase<Traits>::ParsePropertyDefinition(ObjectLiteralCheckerBase* checker,
                                  this->is_generator())) {
     DCHECK(!*is_computed_name);
     DCHECK(!is_static);
-    value = this->ExpressionFromIdentifier(name, next_pos, scope_, factory());
+    value = this->ExpressionFromIdentifier(name, next_beg_pos, next_end_pos,
+                                           scope_, factory());
     return factory()->NewObjectLiteralProperty(
         name_expression, value, ObjectLiteralProperty::COMPUTED, false, false);
 

src/scopes.cc

@@ -263,7 +263,12 @@ bool Scope::Analyze(CompilationInfo* info) {
   // Allocate the variables.
   {
     AstNodeFactory ast_node_factory(info->ast_value_factory());
-    if (!top->AllocateVariables(info, &ast_node_factory)) return false;
+    if (!top->AllocateVariables(info, &ast_node_factory)) {
+      DCHECK(top->pending_error_handler_.has_pending_error());
+      top->pending_error_handler_.ThrowPendingError(info->isolate(),
+                                                    info->script());
+      return false;
+    }
   }
 
 #ifdef DEBUG
@@ -461,7 +466,7 @@ Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode mode,
 
 
 Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
-                              InitializationFlag init_flag,
+                              InitializationFlag init_flag, Variable::Kind kind,
                               MaybeAssignedFlag maybe_assigned_flag) {
   DCHECK(!already_resolved());
   // This function handles VAR, LET, and CONST modes.  DYNAMIC variables are
@@ -469,7 +474,7 @@ Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
   // explicitly, and TEMPORARY variables are allocated via NewTemporary().
   DCHECK(IsDeclaredVariableMode(mode));
   ++num_var_or_const_;
-  return variables_.Declare(this, name, mode, true, Variable::NORMAL, init_flag,
+  return variables_.Declare(this, name, mode, true, kind, init_flag,
                             maybe_assigned_flag);
 }
 
@@ -768,6 +773,20 @@ void Scope::GetNestedScopeChain(Isolate* isolate,
 }
 
 
+void Scope::ReportMessage(int start_position, int end_position,
+                          const char* message, const AstRawString* arg) {
+  // Propagate the error to the topmost scope targeted by this scope analysis
+  // phase.
+  Scope* top = this;
+  while (!top->is_script_scope() && !top->outer_scope()->already_resolved()) {
+    top = top->outer_scope();
+  }
+
+  top->pending_error_handler_.ReportMessageAt(start_position, end_position,
+                                              message, arg, kReferenceError);
+}
+
+
 #ifdef DEBUG
 static const char* Header(ScopeType scope_type) {
   switch (scope_type) {
@@ -1050,6 +1069,31 @@ bool Scope::ResolveVariable(CompilationInfo* info, VariableProxy* proxy,
   switch (binding_kind) {
     case BOUND:
       // We found a variable binding.
+      if (is_strong(language_mode())) {
+        // Check for declaration-after use (for variables) in strong mode. Note
+        // that we can only do this in the case where we have seen the
+        // declaration. And we always allow referencing functions (for now).
+
+        // If both the use and the declaration are inside an eval scope
+        // (possibly indirectly), or one of them is, we need to check whether
+        // they are inside the same eval scope or different
+        // ones.
+
+        // TODO(marja,rossberg): Detect errors across different evals (depends
+        // on the future of eval in strong mode).
+        const Scope* eval_for_use = NearestOuterEvalScope();
+        const Scope* eval_for_declaration =
+            var->scope()->NearestOuterEvalScope();
+
+        if (proxy->position() != RelocInfo::kNoPosition &&
+            proxy->position() < var->initializer_position() &&
+            !var->is_function() && eval_for_use == eval_for_declaration) {
+          DCHECK(proxy->end_position() != RelocInfo::kNoPosition);
+          ReportMessage(proxy->position(), proxy->end_position(),
+                        "strong_use_before_declaration", proxy->raw_name());
+          return false;
+        }
+      }
       break;
 
     case BOUND_EVAL_SHADOWED:

src/scopes.h

@@ -6,6 +6,7 @@
 #define V8_SCOPES_H_
 
 #include "src/ast.h"
+#include "src/pending-compilation-error-handler.h"
 #include "src/zone.h"
 
 namespace v8 {
@@ -130,7 +131,7 @@ class Scope: public ZoneObject {
   // Declare a local variable in this scope. If the variable has been
   // declared before, the previously declared variable is returned.
   Variable* DeclareLocal(const AstRawString* name, VariableMode mode,
-                         InitializationFlag init_flag,
+                         InitializationFlag init_flag, Variable::Kind kind,
                          MaybeAssignedFlag maybe_assigned_flag = kNotAssigned);
 
   // Declare an implicit global variable in this scope which must be a
@@ -142,12 +143,14 @@ class Scope: public ZoneObject {
   // Create a new unresolved variable.
   VariableProxy* NewUnresolved(AstNodeFactory* factory,
                                const AstRawString* name,
-                               int position = RelocInfo::kNoPosition) {
+                               int start_position = RelocInfo::kNoPosition,
+                               int end_position = RelocInfo::kNoPosition) {
     // Note that we must not share the unresolved variables with
     // the same name because they may be removed selectively via
     // RemoveUnresolved().
     DCHECK(!already_resolved());
-    VariableProxy* proxy = factory->NewVariableProxy(name, false, position);
+    VariableProxy* proxy =
+        factory->NewVariableProxy(name, false, start_position, end_position);
     unresolved_.Add(proxy, zone_);
     return proxy;
   }
@@ -317,6 +320,12 @@ class Scope: public ZoneObject {
   // Does any inner scope access "this".
   bool inner_uses_this() const { return inner_scope_uses_this_; }
 
+  const Scope* NearestOuterEvalScope() const {
+    if (is_eval_scope()) return this;
+    if (outer_scope() == nullptr) return nullptr;
+    return outer_scope()->NearestOuterEvalScope();
+  }
+
   // ---------------------------------------------------------------------------
   // Accessors.
 
@@ -470,6 +479,10 @@ class Scope: public ZoneObject {
     return params_.Contains(variables_.Lookup(name));
   }
 
+  // Error handling.
+  void ReportMessage(int start_position, int end_position, const char* message,
+                     const AstRawString* arg);
+
   // ---------------------------------------------------------------------------
   // Debugging.
 
@@ -696,6 +709,8 @@ class Scope: public ZoneObject {
 
   AstValueFactory* ast_value_factory_;
   Zone* zone_;
+
+  PendingCompilationErrorHandler pending_error_handler_;
 };
 
 } }  // namespace v8::internal

src/variables.h

@@ -18,7 +18,7 @@ namespace internal {
 
 class Variable: public ZoneObject {
  public:
-  enum Kind { NORMAL, THIS, NEW_TARGET, ARGUMENTS };
+  enum Kind { NORMAL, FUNCTION, THIS, NEW_TARGET, ARGUMENTS };
 
   enum Location {
     // Before and during variable allocation, a variable whose location is
@@ -98,6 +98,7 @@ class Variable: public ZoneObject {
     return initialization_flag_ == kNeedsInitialization;
   }
 
+  bool is_function() const { return kind_ == FUNCTION; }
   bool is_this() const { return kind_ == THIS; }
   bool is_new_target() const { return kind_ == NEW_TARGET; }
   bool is_arguments() const { return kind_ == ARGUMENTS; }

test/mjsunit/strong/declaration-after-use.js

+// Copyright 2015 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: --strong-mode --harmony_rest_parameters --harmony_arrow_functions --harmony_classes --harmony-computed-property-names
+
+// Note that it's essential for these tests that the reference is inside dead
+// code (because we already produce ReferenceErrors for run-time unresolved
+// variables and don't want to confuse those with strong mode errors). But the
+// errors should *not* be inside lazy, unexecuted functions, since lazy parsing
+// doesn't produce strong mode scoping errors).
+
+// In addition, assertThrows will call eval and that changes variable binding
+// types (see e.g., UNBOUND_EVAL_SHADOWED). We can avoid unwanted side effects
+// by wrapping the code to be tested inside an outer function.
+function assertThrowsHelper(code, error) {
+  "use strict";
+  let prologue = "(function outer() { ";
+  let epilogue = " })();";
+  assertThrows(prologue + code + epilogue, error);
+}
+
+(function DeclarationAfterUse() {
+  // Note that these tests only test cases where the declaration is found but is
+  // after the use. In particular, we cannot yet detect cases where the use can
+  // possibly bind to a global variable.
+  assertThrowsHelper("'use strong'; if (false) { x; let x = 0; }",
+                     ReferenceError);
+  assertThrowsHelper(
+      "function f() { 'use strong'; if (false) { x; let x = 0; } } f();",
+      ReferenceError);
+  assertThrowsHelper(
+      "'use strong'; function f() { if (false) { x; } } let x = 0; f();",
+      ReferenceError);
+
+  assertThrowsHelper(
+      "function f() { 'use strong'; if (false) { x; } } var x = 0; f();",
+      ReferenceError);
+  assertThrowsHelper(
+      "function f() { 'use strong'; if (false) { x; } } var x; f();",
+      ReferenceError);
+  // Errors are also detected when the declaration and the use are in the same
+  // eval scope.
+  assertThrowsHelper("'use strong'; eval('x; let x = 0;')", ReferenceError);
+
+  // Use occurring in the initializer of the declaration:
+  assertThrowsHelper("'use strong'; if (false) { let x = x + 1; }",
+                     ReferenceError);
+  assertThrowsHelper("'use strong'; if (false) { let x = x; }",
+                     ReferenceError);
+  assertThrowsHelper("'use strong'; if (false) { let x = y, y = 4; }",
+                     ReferenceError);
+  assertThrowsHelper("'use strong'; if (false) { let x = function() { x; } }",
+                     ReferenceError);
+  assertThrowsHelper("'use strong'; if (false) { let x = a => { x; } }",
+                     ReferenceError);
+  assertThrowsHelper(
+      "'use strong'; if (false) { function f() {}; let x = f(x); }",
+      ReferenceError);
+  assertThrowsHelper("'use strong'; if (false) { const x = x; }",
+                     ReferenceError);
+  assertThrowsHelper("'use strong'; if (false) { const x = function() { x; } }",
+                     ReferenceError);
+  assertThrowsHelper("'use strong'; if (false) { const x = a => { x; } }",
+                     ReferenceError);
+  assertThrowsHelper(
+      "'use strong'; if (false) { function f() {}; const x = f(x); }",
+      ReferenceError);
+
+  assertThrowsHelper(
+      "'use strong'; if (false) { for (let x = x; ; ) { } }",
+      ReferenceError);
+  assertThrowsHelper(
+      "'use strong'; if (false) { for (const x = x; ; ) { } }",
+      ReferenceError);
+  assertThrowsHelper(
+      "'use strong'; if (false) { for (let x = y, y; ; ) { } }",
+      ReferenceError);
+  assertThrowsHelper(
+      "'use strong'; if (false) { for (const x = y, y = 0; ; ) { } }",
+      ReferenceError);
+
+  // Computed property names
+  assertThrowsHelper(
+      "'use strong'; if (false) { let o = { 'a': 'b', [o.a]: 'c'}; }",
+      ReferenceError);
+})();
+
+
+(function DeclarationAfterUseInClasses() {
+  assertThrowsHelper("'use strong'; if (false) { class C extends C { } }",
+                     ReferenceError);
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 extends C { } }",
+      ReferenceError);
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 extends C2 { } }",
+      ReferenceError);
+
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 { constructor() { C; } } }",
+      ReferenceError);
+
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 { method() { C; } } }",
+      ReferenceError);
+
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 { " +
+          "static a() { return 'A'; } [C.a()]() { return 'B'; } }; }",
+      ReferenceError);
+
+  // TODO(marja, rossberg): More tests related to computed property names in
+  // classes + recognize more errors. This one is not recognized as an error
+  // yet:
+  // let C = class C2 {
+  //  static a() { return 'A'; }
+  //  [C2.a()]() { return 'B'; } << C2 should not be allowed here
+  // };
+})();
+
+
+(function UsesWhichAreFine() {
+  "use strong";
+
+  let var1 = 0;
+  var1;
+
+  let var2a = 0, var2b = var2a + 1, var2c = 2 + var2b;
+
+  for (let var3 = 0; var3 < 1; var3++) {
+    var3;
+  }
+
+  for (let var4a = 0, var4b = var4a; var4a + var4b < 4; var4a++, var4b++) {
+    var4a;
+    var4b;
+  }
+
+  let var5 = 5;
+  for (; var5 < 10; ++var5) { }
+
+  let arr = [1, 2];
+  for (let i of arr) {
+    i;
+  }
+
+  let var6 = [1, 2];
+  // The second var6 resolves to outside (not to the first var6).
+  for (let var6 of var6) { var6; }
+
+  try {
+    throw "error";
+  } catch (e) {
+    e;
+  }
+
+  function func1() { func1; this; }
+  func1();
+  func1;
+
+  function * func2() { func2; this; }
+  func2();
+  func2;
+
+  function func4(p, ...rest) { p; rest; this; func2; }
+  func4();
+
+  let func5 = (p1, p2) => { p1; p2; };
+  func5();
+
+  function func6() {
+    var1, var2a, var2b, var2c;
+  }
+
+  (function eval1() {
+    let var7 = 0; // Declaration position will be something large.
+    // But use position will be something small, however, this is not an error,
+    // since the use is inside an eval scope.
+    eval("var7;");
+  })();
+
+  class C1 { constructor() { C1; } }; new C1();
+  let C2 = class C3 { constructor() { C3; } }; new C2();
+
+  class C4 { method() { C4; method; } }; new C4();
+  let C5 = class C6 { method() { C6; method; } }; new C5();
+})();