[strong] Fix scoping related errors for methods.

Methods can refer to the class name.

BUG=v8:3927
LOG=N

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

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

src/objects.h

@@ -4268,6 +4268,8 @@ class ScopeInfo : public FixedArray {
   // must be an internalized string.
   int FunctionContextSlotIndex(String* name, VariableMode* mode);
 
+  bool block_scope_is_class_scope();
+  FunctionKind function_kind();
 
   // Copies all the context locals into an object used to materialize a scope.
   static bool CopyContextLocalsToScopeObject(Handle<ScopeInfo> scope_info,
@@ -4373,6 +4375,10 @@ class ScopeInfo : public FixedArray {
   class AsmFunctionField : public BitField<bool, 13, 1> {};
   class IsSimpleParameterListField
       : public BitField<bool, AsmFunctionField::kNext, 1> {};
+  class BlockScopeIsClassScopeField
+      : public BitField<bool, IsSimpleParameterListField::kNext, 1> {};
+  class FunctionKindField
+      : public BitField<FunctionKind, BlockScopeIsClassScopeField::kNext, 7> {};
 
   // BitFields representing the encoded information for context locals in the
   // ContextLocalInfoEntries part.

src/parser.cc

@@ -4100,7 +4100,11 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
     return NULL;
   }
 
+  // Create a block scope which is additionally tagged as class scope; this is
+  // important for resolving variable references to the class name in the strong
+  // mode.
   Scope* block_scope = NewScope(scope_, BLOCK_SCOPE);
+  block_scope->tag_as_class_scope();
   BlockState block_state(&scope_, block_scope);
   scope_->SetLanguageMode(
       static_cast<LanguageMode>(scope_->language_mode() | STRICT_BIT));
@@ -4164,12 +4168,14 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
   }
 
   block_scope->set_end_position(end_pos);
-  block_scope = block_scope->FinalizeBlockScope();
 
   if (name != NULL) {
     DCHECK_NOT_NULL(proxy);
-    DCHECK_NOT_NULL(block_scope);
     proxy->var()->set_initializer_position(end_pos);
+  } else {
+    // Unnamed classes should not have scopes (the scope will be empty).
+    DCHECK_EQ(block_scope->num_var_or_const(), 0);
+    block_scope = nullptr;
   }
 
   return factory()->NewClassLiteral(name, block_scope, proxy, extends,

src/preparser.h

@@ -317,10 +317,9 @@ class ParserBase : public Traits {
     DCHECK(scope_type != MODULE_SCOPE || allow_harmony_modules());
     DCHECK((scope_type == FUNCTION_SCOPE && IsValidFunctionKind(kind)) ||
            kind == kNormalFunction);
-    Scope* result =
+    Scope* result = new (zone())
-        new (zone()) Scope(zone(), parent, scope_type, ast_value_factory());
+        Scope(zone(), parent, scope_type, ast_value_factory(), kind);
-    bool uninitialized_this = IsSubclassConstructor(kind);
+    result->Initialize();
-    result->Initialize(uninitialized_this);
     return result;
   }
 

src/scopeinfo.cc

@@ -64,7 +64,9 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate, Zone* zone,
               FunctionVariableMode::encode(function_variable_mode) |
               AsmModuleField::encode(scope->asm_module()) |
               AsmFunctionField::encode(scope->asm_function()) |
-              IsSimpleParameterListField::encode(simple_parameter_list);
+              IsSimpleParameterListField::encode(simple_parameter_list) |
+              BlockScopeIsClassScopeField::encode(scope->is_class_scope()) |
+              FunctionKindField::encode(scope->function_kind());
   scope_info->SetFlags(flags);
   scope_info->SetParameterCount(parameter_count);
   scope_info->SetStackLocalCount(stack_local_count);
@@ -373,6 +375,16 @@ int ScopeInfo::FunctionContextSlotIndex(String* name, VariableMode* mode) {
 }
 
 
+bool ScopeInfo::block_scope_is_class_scope() {
+  return BlockScopeIsClassScopeField::decode(Flags());
+}
+
+
+FunctionKind ScopeInfo::function_kind() {
+  return FunctionKindField::decode(Flags());
+}
+
+
 bool ScopeInfo::CopyContextLocalsToScopeObject(Handle<ScopeInfo> scope_info,
                                                Handle<Context> context,
                                                Handle<JSObject> scope_object) {

src/scopes.cc

@@ -66,7 +66,7 @@ Variable* VariableMap::Lookup(const AstRawString* name) {
 // Implementation of Scope
 
 Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
-             AstValueFactory* ast_value_factory)
+             AstValueFactory* ast_value_factory, FunctionKind function_kind)
     : inner_scopes_(4, zone),
       variables_(zone),
       internals_(4, zone),
@@ -79,7 +79,8 @@ Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
       already_resolved_(false),
       ast_value_factory_(ast_value_factory),
       zone_(zone) {
-  SetDefaults(scope_type, outer_scope, Handle<ScopeInfo>::null());
+  SetDefaults(scope_type, outer_scope, Handle<ScopeInfo>::null(),
+              function_kind);
   // The outermost scope must be a script scope.
   DCHECK(scope_type == SCRIPT_SCOPE || outer_scope != NULL);
   DCHECK(!HasIllegalRedeclaration());
@@ -138,11 +139,13 @@ Scope::Scope(Zone* zone, Scope* inner_scope,
 }
 
 
-void Scope::SetDefaults(ScopeType scope_type,
+void Scope::SetDefaults(ScopeType scope_type, Scope* outer_scope,
-                        Scope* outer_scope,
+                        Handle<ScopeInfo> scope_info,
-                        Handle<ScopeInfo> scope_info) {
+                        FunctionKind function_kind) {
   outer_scope_ = outer_scope;
   scope_type_ = scope_type;
+  function_kind_ = function_kind;
+  block_scope_is_class_scope_ = false;
   scope_name_ = ast_value_factory_->empty_string();
   dynamics_ = NULL;
   receiver_ = NULL;
@@ -181,6 +184,8 @@ void Scope::SetDefaults(ScopeType scope_type,
   if (!scope_info.is_null()) {
     scope_calls_eval_ = scope_info->CallsEval();
     language_mode_ = scope_info->language_mode();
+    block_scope_is_class_scope_ = scope_info->block_scope_is_class_scope();
+    function_kind_ = scope_info->function_kind();
   }
 }
 
@@ -284,7 +289,8 @@ bool Scope::Analyze(CompilationInfo* info) {
 }
 
 
-void Scope::Initialize(bool subclass_constructor) {
+void Scope::Initialize() {
+  bool subclass_constructor = IsSubclassConstructor(function_kind_);
   DCHECK(!already_resolved());
 
   // Add this scope as a new inner scope of the outer scope.
@@ -1070,29 +1076,7 @@ bool Scope::ResolveVariable(CompilationInfo* info, VariableProxy* proxy,
     case BOUND:
       // We found a variable binding.
       if (is_strong(language_mode())) {
-        // Check for declaration-after use (for variables) in strong mode. Note
+        if (!CheckStrongModeDeclaration(proxy, var)) return false;
-        // 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;
 
@@ -1137,6 +1121,56 @@ bool Scope::ResolveVariable(CompilationInfo* info, VariableProxy* proxy,
 }
 
 
+bool Scope::CheckStrongModeDeclaration(VariableProxy* proxy, Variable* var) {
+  // 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).
+
+  // Allow referencing the class name from methods of that class, even though
+  // the initializer position for class names is only after the body.
+  Scope* scope = this;
+  while (scope) {
+    if (scope->ClassVariableForMethod() == var) return true;
+    scope = scope->outer_scope();
+  }
+
+  // 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;
+  }
+  return true;
+}
+
+
+Variable* Scope::ClassVariableForMethod() const {
+  if (!is_function_scope()) return nullptr;
+  if (!IsConciseMethod(function_kind_) && !IsConstructor(function_kind_) &&
+      !IsAccessorFunction(function_kind_)) {
+    return nullptr;
+  }
+  DCHECK_NOT_NULL(outer_scope_);
+  DCHECK(outer_scope_->is_class_scope());
+  // The class scope contains at most one variable, the class name.
+  DCHECK(outer_scope_->variables_.occupancy() <= 1);
+  if (outer_scope_->variables_.occupancy() == 0) return nullptr;
+  VariableMap::Entry* p = outer_scope_->variables_.Start();
+  return reinterpret_cast<Variable*>(p->value);
+}
+
+
 bool Scope::ResolveVariablesRecursively(CompilationInfo* info,
                                         AstNodeFactory* factory) {
   DCHECK(info->script_scope()->is_script_scope());
@@ -1430,5 +1464,4 @@ int Scope::ContextLocalCount() const {
   return num_heap_slots() - Context::MIN_CONTEXT_SLOTS -
       (function_ != NULL && function_->proxy()->var()->IsContextSlot() ? 1 : 0);
 }
-
 } }  // namespace v8::internal

src/scopes.h

@@ -73,7 +73,8 @@ class Scope: public ZoneObject {
   // Construction
 
   Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
-        AstValueFactory* value_factory);
+        AstValueFactory* value_factory,
+        FunctionKind function_kind = kNormalFunction);
 
   // Compute top scope and allocate variables. For lazy compilation the top
   // scope only contains the single lazily compiled function, so this
@@ -88,7 +89,7 @@ class Scope: public ZoneObject {
     scope_name_ = scope_name;
   }
 
-  void Initialize(bool subclass_constructor = false);
+  void Initialize();
 
   // Checks if the block scope is redundant, i.e. it does not contain any
   // block scoped declarations. In that case it is removed from the scope
@@ -281,6 +282,13 @@ class Scope: public ZoneObject {
   bool is_block_scope() const { return scope_type_ == BLOCK_SCOPE; }
   bool is_with_scope() const { return scope_type_ == WITH_SCOPE; }
   bool is_arrow_scope() const { return scope_type_ == ARROW_SCOPE; }
+  void tag_as_class_scope() {
+    DCHECK(is_block_scope());
+    block_scope_is_class_scope_ = true;
+  }
+  bool is_class_scope() const {
+    return is_block_scope() && block_scope_is_class_scope_;
+  }
   bool is_declaration_scope() const {
     return is_eval_scope() || is_function_scope() ||
         is_module_scope() || is_script_scope();
@@ -332,6 +340,8 @@ class Scope: public ZoneObject {
   // The type of this scope.
   ScopeType scope_type() const { return scope_type_; }
 
+  FunctionKind function_kind() const { return function_kind_; }
+
   // The language mode of this scope.
   LanguageMode language_mode() const { return language_mode_; }
 
@@ -501,6 +511,10 @@ class Scope: public ZoneObject {
 
   // The scope type.
   ScopeType scope_type_;
+  // Some block scopes are tagged as class scopes.
+  bool block_scope_is_class_scope_;
+  // If the scope is a function scope, this is the function kind.
+  FunctionKind function_kind_;
 
   // Debugging support.
   const AstRawString* scope_name_;
@@ -658,6 +672,12 @@ class Scope: public ZoneObject {
   bool ResolveVariablesRecursively(CompilationInfo* info,
                                    AstNodeFactory* factory);
 
+  bool CheckStrongModeDeclaration(VariableProxy* proxy, Variable* var);
+
+  // If this scope is a method scope of a class, return the corresponding
+  // class variable, otherwise nullptr.
+  Variable* ClassVariableForMethod() const;
+
   // Scope analysis.
   void PropagateScopeInfo(bool outer_scope_calls_sloppy_eval);
   bool HasTrivialContext() const;
@@ -703,9 +723,9 @@ class Scope: public ZoneObject {
     }
   }
 
-  void SetDefaults(ScopeType type,
+  void SetDefaults(ScopeType type, Scope* outer_scope,
-                   Scope* outer_scope,
+                   Handle<ScopeInfo> scope_info,
-                   Handle<ScopeInfo> scope_info);
+                   FunctionKind function_kind = kNormalFunction);
 
   AstValueFactory* ast_value_factory_;
   Zone* zone_;

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

@@ -110,13 +110,70 @@ function assertThrowsHelper(code, error) {
           "static a() { return 'A'; } [C.a()]() { return 'B'; } }; }",
       ReferenceError);
 
-  // TODO(marja, rossberg): More tests related to computed property names in
+  assertThrowsHelper(
-  // classes + recognize more errors. This one is not recognized as an error
+      "'use strong'; if (false) { let C = class C2 { " +
-  // yet:
+          "static a() { return 'A'; } [C2.a()]() { return 'B'; } }; }",
-  // let C = class C2 {
+       ReferenceError);
-  //  static a() { return 'A'; }
+
-  //  [C2.a()]() { return 'B'; } << C2 should not be allowed here
+  assertThrowsHelper(
-  // };
+      "'use strong'; if (false) { let C = class C2 { " +
+          "[(function() { C; return 'A';})()]() { return 'B'; } }; }",
+      ReferenceError);
+
+  // The reference to C or C2 is inside a function, but not a method.
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 { " +
+          "[(function() { C2; return 'A';})()]() { return 'B'; } }; }",
+      ReferenceError);
+
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 { " +
+          "[(function() { C; return 'A';})()]() { return 'B'; } }; }",
+      ReferenceError);
+
+  // The reference to C or C2 is inside a method, but it's not a method of the
+  // relevant class (C2).
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 { " +
+          "[(new (class D { m() { C2; return 'A'; } })).m()]() " +
+          "{ return 'B'; } } }",
+      ReferenceError);
+
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 { " +
+          "[(new (class D { m() { C; return 'A'; } })).m()]() " +
+          "{ return 'B'; } } }",
+      ReferenceError);
+
+  // Methods inside object literals are not sufficiently distinguished from
+  // methods inside classes.
+  // https://code.google.com/p/v8/issues/detail?id=3948
+  // assertThrowsHelper(
+  //     "'use strong'; if (false) { let C = class C2 { " +
+  //         "[({m() { C2; return 'A'; }}).m()]() " +
+  //         "{ return 'B'; } } }",
+  //     ReferenceError);
+
+  assertThrowsHelper(
+      "'use strong'; if (false) { let C = class C2 { " +
+          "[({m() { C; return 'A'; }}).m()]() " +
+          "{ return 'B'; } } }",
+      ReferenceError);
+
+  // assertThrowsHelper(
+  //     "'use strong';\n" +
+  //         "if (false) {\n" +
+  //         "  class COuter {\n" +
+  //         "    m() {\n" +
+  //         "      class CInner {\n" +
+  //         "        [({ m() { CInner; return 'A'; } }).m()]() {\n" +
+  //         "            return 'B';\n" +
+  //         "        }\n" +
+  //         "      }\n" +
+  //         "    }\n" +
+  //         "  }\n" +
+  //         "}",
+  //     ReferenceError);
 })();
 
 
@@ -180,10 +237,33 @@ function assertThrowsHelper(code, error) {
     eval("var7;");
   })();
 
-  // https://code.google.com/p/v8/issues/detail?id=3927
-  // class C1 { constructor() { C1; } }; new C1();
-  // let C2 = class C3 { constructor() { C3; } }; new C2();
 
-  // class C4 { method() { C4; method; } }; new C4();
+  class C1 { constructor() { C1; } }; new C1();
-  // let C5 = class C6 { method() { C6; method; } }; new C5();
+  let C2 = class C3 { constructor() { C3; } }; new C2();
+
+  class C4 { method() { C4; } }; new C4();
+  let C5 = class C6 { method() { C6; } }; new C5();
+
+  class C7 { static method() { C7; } }; new C7();
+  let C8 = class C9 { static method() { C9; } }; new C8();
+
+  class C10 { get x() { C10; } }; new C10();
+  let C11 = class C12 { get x() { C12; } }; new C11();
+
+  // Regression test for unnamed classes.
+  let C12 = class { m() { var1; } };
+
+  class COuter {
+    m() {
+      class CInner {
+        // Here we can refer to COuter but not to CInner (see corresponding
+        // assertion test):
+        [({ m() { COuter; return 'A'; } }).m()]() { return 'B'; }
+        // And here we can refer to both:
+        n() { COuter; CInner; }
+      }
+      return new CInner();
+    }
+  }
+  (new COuter()).m().n();
 })();