Commit ddd3f318 authored by marja's avatar marja Committed by Commit bot

[strong] Stricter check for referring to other classes inside methods.

Add the restriction that both classes must be declared inside the same
consectutive class declaration batch.

Dependency analysis not implemented yet.

BUG=v8:3956
LOG=N

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

Cr-Commit-Position: refs/heads/master@{#28032}
parent d5fd5816
......@@ -570,13 +570,22 @@ class VariableDeclaration final : public Declaration {
bool is_class_declaration() const { return is_class_declaration_; }
// VariableDeclarations can be grouped into consecutive declaration
// groups. Each VariableDeclaration is associated with the start position of
// the group it belongs to. The positions are used for strong mode scope
// checks for classes and functions.
int declaration_group_start() const { return declaration_group_start_; }
protected:
VariableDeclaration(Zone* zone, VariableProxy* proxy, VariableMode mode,
Scope* scope, int pos, bool is_class_declaration = false)
Scope* scope, int pos, bool is_class_declaration = false,
int declaration_group_start = -1)
: Declaration(zone, proxy, mode, scope, pos),
is_class_declaration_(is_class_declaration) {}
is_class_declaration_(is_class_declaration),
declaration_group_start_(declaration_group_start) {}
bool is_class_declaration_;
int declaration_group_start_;
};
......@@ -3220,9 +3229,10 @@ class AstNodeFactory final BASE_EMBEDDED {
VariableDeclaration* NewVariableDeclaration(
VariableProxy* proxy, VariableMode mode, Scope* scope, int pos,
bool is_class_declaration = false) {
return new (zone_) VariableDeclaration(zone_, proxy, mode, scope, pos,
is_class_declaration);
bool is_class_declaration = false, int declaration_group_start = -1) {
return new (zone_)
VariableDeclaration(zone_, proxy, mode, scope, pos,
is_class_declaration, declaration_group_start);
}
FunctionDeclaration* NewFunctionDeclaration(VariableProxy* proxy,
......
......@@ -1298,10 +1298,20 @@ Statement* Parser::ParseStatementListItem(bool* ok) {
// Statement
// Declaration
if (peek() != Token::CLASS) {
// No more classes follow; reset the start position for the consecutive
// class declaration group.
scope_->set_class_declaration_group_start(-1);
}
switch (peek()) {
case Token::FUNCTION:
return ParseFunctionDeclaration(NULL, ok);
case Token::CLASS:
if (scope_->class_declaration_group_start() < 0) {
scope_->set_class_declaration_group_start(
scanner()->peek_location().beg_pos);
}
return ParseClassDeclaration(NULL, ok);
case Token::CONST:
case Token::VAR:
......@@ -1931,14 +1941,18 @@ Variable* Parser::Declare(Declaration* declaration, bool resolve, bool* ok) {
if (var == NULL) {
// Declare the name.
Variable::Kind kind = Variable::NORMAL;
int declaration_group_start = -1;
if (declaration->IsFunctionDeclaration()) {
kind = Variable::FUNCTION;
} else if (declaration->IsVariableDeclaration() &&
declaration->AsVariableDeclaration()->is_class_declaration()) {
kind = Variable::CLASS;
declaration_group_start =
declaration->AsVariableDeclaration()->declaration_group_start();
}
var = declaration_scope->DeclareLocal(
name, mode, declaration->initialization(), kind, kNotAssigned);
name, mode, declaration->initialization(), kind, kNotAssigned,
declaration_group_start);
} else if (IsLexicalVariableMode(mode) ||
IsLexicalVariableMode(var->mode()) ||
((mode == CONST_LEGACY || var->mode() == CONST_LEGACY) &&
......@@ -2158,9 +2172,22 @@ Statement* Parser::ParseClassDeclaration(ZoneList<const AstRawString*>* names,
VariableProxy* proxy = NewUnresolved(name, mode);
const bool is_class_declaration = true;
Declaration* declaration = factory()->NewVariableDeclaration(
proxy, mode, scope_, pos, is_class_declaration);
Declare(declaration, true, CHECK_OK);
proxy, mode, scope_, pos, is_class_declaration,
scope_->class_declaration_group_start());
Variable* outer_class_variable = Declare(declaration, true, CHECK_OK);
proxy->var()->set_initializer_position(position());
if (value->class_variable_proxy() && value->class_variable_proxy()->var() &&
outer_class_variable->is_class()) {
// In some cases, the outer variable is not detected as a class variable;
// this happens e.g., for lazy methods. They are excluded from strong mode
// checks for now. TODO(marja, rossberg): re-create variables with the
// correct Kind and remove this hack.
value->class_variable_proxy()
->var()
->AsClassVariable()
->set_corresponding_outer_class_variable(
outer_class_variable->AsClassVariable());
}
Token::Value init_op =
is_strong(language_mode()) ? Token::INIT_CONST : Token::INIT_LET;
......@@ -4323,8 +4350,10 @@ ClassLiteral* Parser::ParseClassLiteral(const AstRawString* name,
VariableProxy* proxy = NULL;
if (name != NULL) {
proxy = NewUnresolved(name, CONST);
Declaration* declaration =
factory()->NewVariableDeclaration(proxy, CONST, block_scope, pos);
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, true, CHECK_OK);
}
......
......@@ -32,7 +32,8 @@ VariableMap::~VariableMap() {}
Variable* VariableMap::Declare(Scope* scope, const AstRawString* name,
VariableMode mode, Variable::Kind kind,
InitializationFlag initialization_flag,
MaybeAssignedFlag maybe_assigned_flag) {
MaybeAssignedFlag maybe_assigned_flag,
int declaration_group_start) {
// AstRawStrings are unambiguous, i.e., the same string is always represented
// by the same AstRawString*.
// FIXME(marja): fix the type of Lookup.
......@@ -42,8 +43,14 @@ Variable* VariableMap::Declare(Scope* scope, const AstRawString* name,
if (p->value == NULL) {
// The variable has not been declared yet -> insert it.
DCHECK(p->key == name);
p->value = new (zone()) Variable(scope, name, mode, kind,
initialization_flag, maybe_assigned_flag);
if (kind == Variable::CLASS) {
p->value = new (zone())
ClassVariable(scope, name, mode, kind, initialization_flag,
maybe_assigned_flag, declaration_group_start);
} else {
p->value = new (zone()) Variable(
scope, name, mode, kind, initialization_flag, maybe_assigned_flag);
}
}
return reinterpret_cast<Variable*>(p->value);
}
......@@ -76,7 +83,8 @@ Scope::Scope(Zone* zone, Scope* outer_scope, ScopeType scope_type,
scope_type == MODULE_SCOPE ? ModuleDescriptor::New(zone) : NULL),
already_resolved_(false),
ast_value_factory_(ast_value_factory),
zone_(zone) {
zone_(zone),
class_declaration_group_start_(-1) {
SetDefaults(scope_type, outer_scope, Handle<ScopeInfo>::null(),
function_kind);
// The outermost scope must be a script scope.
......@@ -97,7 +105,8 @@ Scope::Scope(Zone* zone, Scope* inner_scope, ScopeType scope_type,
module_descriptor_(NULL),
already_resolved_(true),
ast_value_factory_(value_factory),
zone_(zone) {
zone_(zone),
class_declaration_group_start_(-1) {
SetDefaults(scope_type, NULL, scope_info);
if (!scope_info.is_null()) {
num_heap_slots_ = scope_info_->ContextLength();
......@@ -122,7 +131,8 @@ Scope::Scope(Zone* zone, Scope* inner_scope,
module_descriptor_(NULL),
already_resolved_(true),
ast_value_factory_(value_factory),
zone_(zone) {
zone_(zone),
class_declaration_group_start_(-1) {
SetDefaults(CATCH_SCOPE, NULL, Handle<ScopeInfo>::null());
AddInnerScope(inner_scope);
++num_var_or_const_;
......@@ -411,6 +421,7 @@ Variable* Scope::LookupLocal(const AstRawString* name) {
maybe_assigned_flag = kMaybeAssigned;
}
// TODO(marja, rossberg): Declare variables of the right Kind.
Variable* var = variables_.Declare(this, name, mode, Variable::NORMAL,
init_flag, maybe_assigned_flag);
var->AllocateTo(location, index);
......@@ -472,7 +483,8 @@ Variable* Scope::DeclareParameter(const AstRawString* name, VariableMode mode,
Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
InitializationFlag init_flag, Variable::Kind kind,
MaybeAssignedFlag maybe_assigned_flag) {
MaybeAssignedFlag maybe_assigned_flag,
int declaration_group_start) {
DCHECK(!already_resolved());
// This function handles VAR, LET, and CONST modes. DYNAMIC variables are
// introduces during variable allocation, INTERNAL variables are allocated
......@@ -480,7 +492,7 @@ Variable* Scope::DeclareLocal(const AstRawString* name, VariableMode mode,
DCHECK(IsDeclaredVariableMode(mode));
++num_var_or_const_;
return variables_.Declare(this, name, mode, kind, init_flag,
maybe_assigned_flag);
maybe_assigned_flag, declaration_group_start);
}
......@@ -1134,6 +1146,11 @@ bool Scope::CheckStrongModeDeclaration(VariableProxy* proxy, Variable* var) {
// we can only do this in the case where we have seen the declaration. And we
// always allow referencing functions (for now).
// This might happen during lazy compilation; we don't keep track of
// initializer positions for variables stored in ScopeInfo, so we cannot check
// bindings against them. TODO(marja, rossberg): remove this hack.
if (var->initializer_position() == RelocInfo::kNoPosition) return true;
// 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;
......@@ -1143,27 +1160,57 @@ bool Scope::CheckStrongModeDeclaration(VariableProxy* proxy, Variable* var) {
}
// Allow references from methods to classes declared later, if we detect no
// problematic dependency cycles.
if (ClassVariableForMethod() && var->is_class()) {
// A method is referring to some other class, possibly declared
// later. Referring to a class declared earlier is always OK and covered by
// the code outside this if. Here we only need to allow special cases for
// referring to a class which is declared later.
// Referring to a class C declared later is OK under the following
// circumstances:
// 1. The class declarations are in a consecutive group with no other
// declarations or statements in between, and
// 2. There is no dependency cycle where the first edge is an initialization
// time dependency (computed property name or extends clause) from C to
// something that depends on this class directly or transitively.
// problematic dependency cycles. Note that we can be inside multiple methods
// at the same time, and it's enough if we find one where the reference is
// allowed.
if (var->is_class() &&
var->AsClassVariable()->declaration_group_start() >= 0) {
for (scope = this; scope && scope != var->scope();
scope = scope->outer_scope()) {
ClassVariable* class_var = scope->ClassVariableForMethod();
if (class_var) {
// A method is referring to some other class, possibly declared
// later. Referring to a class declared earlier is always OK and covered
// by the code outside this if. Here we only need to allow special cases
// for referring to a class which is declared later.
// Referring to a class C declared later is OK under the following
// circumstances:
// 1. The class declarations are in a consecutive group with no other
// declarations or statements in between, and
// 2. There is no dependency cycle where the first edge is an
// initialization time dependency (computed property name or extends
// clause) from C to something that depends on this class directly or
// transitively.
// This is needed because a class ("class Name { }") creates two
// bindings (one in the outer scope, and one in the class scope). The
// method is a function scope inside the inner scope (class scope). The
// consecutive class declarations are in the outer scope.
class_var = class_var->corresponding_outer_class_variable();
if (class_var &&
class_var->declaration_group_start() ==
var->AsClassVariable()->declaration_group_start()) {
return true;
}
// TODO(marja,rossberg): implement these checks. Here we undershoot the
// target and allow referring to any class.
return true;
// TODO(marja,rossberg): implement the dependency cycle detection. Here
// we undershoot the target and allow referring to any class in the same
// consectuive declaration group.
// The cycle detection can work roughly like this: 1) detect init-time
// references here (they are free variables which are inside the class
// scope but not inside a method scope - no parser changes needed to
// detect them) 2) if we encounter an init-time reference here, allow
// it, but record it for a later dependency cycle check 3) also record
// non-init-time references here 4) after scope analysis is done,
// analyse the dependency cycles: an illegal cycle is one starting with
// an init-time reference and leading back to the starting point with
// either non-init-time and init-time references.
}
}
}
// If both the use and the declaration are inside an eval scope (possibly
......@@ -1187,7 +1234,11 @@ bool Scope::CheckStrongModeDeclaration(VariableProxy* proxy, Variable* var) {
}
Variable* Scope::ClassVariableForMethod() const {
ClassVariable* Scope::ClassVariableForMethod() const {
// TODO(marja, rossberg): This fails to find a class variable in the following
// cases:
// let A = class { ... }
// It needs to be investigated whether this causes any practical problems.
if (!is_function_scope()) return nullptr;
if (IsInObjectLiteral(function_kind_)) return nullptr;
if (!IsConciseMethod(function_kind_) && !IsConstructor(function_kind_) &&
......@@ -1200,7 +1251,9 @@ Variable* Scope::ClassVariableForMethod() const {
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);
Variable* var = reinterpret_cast<Variable*>(p->value);
if (!var->is_class()) return nullptr;
return var->AsClassVariable();
}
......
......@@ -23,7 +23,8 @@ class VariableMap: public ZoneHashMap {
Variable* Declare(Scope* scope, const AstRawString* name, VariableMode mode,
Variable::Kind kind, InitializationFlag initialization_flag,
MaybeAssignedFlag maybe_assigned_flag = kNotAssigned);
MaybeAssignedFlag maybe_assigned_flag = kNotAssigned,
int declaration_group_start = -1);
Variable* Lookup(const AstRawString* name);
......@@ -131,7 +132,8 @@ class Scope: public ZoneObject {
// declared before, the previously declared variable is returned.
Variable* DeclareLocal(const AstRawString* name, VariableMode mode,
InitializationFlag init_flag, Variable::Kind kind,
MaybeAssignedFlag maybe_assigned_flag = kNotAssigned);
MaybeAssignedFlag maybe_assigned_flag = kNotAssigned,
int declaration_group_start = -1);
// Declare an implicit global variable in this scope which must be a
// script scope. The variable was introduced (possibly from an inner
......@@ -411,6 +413,15 @@ class Scope: public ZoneObject {
// The ModuleDescriptor for this scope; only for module scopes.
ModuleDescriptor* module() const { return module_descriptor_; }
void set_class_declaration_group_start(int position) {
class_declaration_group_start_ = position;
}
int class_declaration_group_start() const {
return class_declaration_group_start_;
}
// ---------------------------------------------------------------------------
// Variable allocation.
......@@ -677,7 +688,7 @@ class Scope: public ZoneObject {
// If this scope is a method scope of a class, return the corresponding
// class variable, otherwise nullptr.
Variable* ClassVariableForMethod() const;
ClassVariable* ClassVariableForMethod() const;
// Scope analysis.
void PropagateScopeInfo(bool outer_scope_calls_sloppy_eval);
......@@ -732,6 +743,10 @@ class Scope: public ZoneObject {
Zone* zone_;
PendingCompilationErrorHandler pending_error_handler_;
// For tracking which classes are declared consecutively. Needed for strong
// mode.
int class_declaration_group_start_;
};
} } // namespace v8::internal
......
......@@ -16,6 +16,8 @@ namespace internal {
// they are maintained by scopes, and referred to from VariableProxies and Slots
// after binding and variable allocation.
class ClassVariable;
class Variable: public ZoneObject {
public:
enum Kind { NORMAL, FUNCTION, CLASS, THIS, NEW_TARGET, ARGUMENTS };
......@@ -51,6 +53,8 @@ class Variable: public ZoneObject {
InitializationFlag initialization_flag,
MaybeAssignedFlag maybe_assigned_flag = kNotAssigned);
virtual ~Variable() {}
// Printing support
static const char* Mode2String(VariableMode mode);
......@@ -102,6 +106,11 @@ class Variable: public ZoneObject {
bool is_new_target() const { return kind_ == NEW_TARGET; }
bool is_arguments() const { return kind_ == ARGUMENTS; }
ClassVariable* AsClassVariable() {
DCHECK(is_class());
return reinterpret_cast<ClassVariable*>(this);
}
// True if the variable is named eval and not known to be shadowed.
bool is_possibly_eval(Isolate* isolate) const {
return IsVariable(isolate->factory()->eval_string());
......@@ -175,7 +184,33 @@ class Variable: public ZoneObject {
MaybeAssignedFlag maybe_assigned_;
};
class ClassVariable : public Variable {
public:
ClassVariable(Scope* scope, const AstRawString* name, VariableMode mode,
Kind kind, InitializationFlag initialization_flag,
MaybeAssignedFlag maybe_assigned_flag = kNotAssigned,
int declaration_group_start = -1)
: Variable(scope, name, mode, kind, initialization_flag,
maybe_assigned_flag),
declaration_group_start_(declaration_group_start),
corresponding_outer_class_variable_(nullptr) {}
int declaration_group_start() const { return declaration_group_start_; }
ClassVariable* corresponding_outer_class_variable() const {
return corresponding_outer_class_variable_;
}
void set_corresponding_outer_class_variable(ClassVariable* var) {
corresponding_outer_class_variable_ = var;
}
private:
// For classes we keep track of consecutive groups of delcarations. They are
// needed for strong mode scoping checks. TODO(marja, rossberg): Implement
// checks for functions too.
int declaration_group_start_;
ClassVariable* corresponding_outer_class_variable_;
};
} } // namespace v8::internal
#endif // V8_VARIABLES_H_
......@@ -2,23 +2,20 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --strong-mode
// Flags: --strong-mode --harmony-arrow-functions
"use strict"
// 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).
let prologue_dead = "(function outer() { if (false) { ";
let epilogue_dead = " } })();";
// 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) {
"use strict";
let prologue_dead = "(function outer() { if (false) { ";
let epilogue_dead = " } })();";
let prologue_live = "(function outer() { ";
let epilogue_live = "})();";
assertThrows("'use strong'; " + prologue_dead + code + epilogue_dead, ReferenceError);
// For code which already throws a run-time error in non-strong mode; we assert
// that we now get the error already compilation time.
function assertLateErrorsBecomeEarly(code) {
assertThrows("'use strong'; " + prologue_dead + code + epilogue_dead,
ReferenceError);
// Make sure the error happens only in strong mode (note that we need strict
// mode here because of let).
......@@ -26,24 +23,35 @@ function assertThrowsHelper(code) {
// But if we don't put the references inside a dead code, it throws a run-time
// error (also in strict mode).
let prologue_live = "(function outer() { ";
let epilogue_live = "})();";
assertThrows("'use strong'; " + prologue_live + code + epilogue_live,
ReferenceError);
assertThrows("'use strict'; " + prologue_live + code + epilogue_live,
ReferenceError);
}
// For code which doesn't throw an error at all in non-strong mode.
function assertNonErrorsBecomeEarly(code) {
assertThrows("'use strong'; " + prologue_dead + code + epilogue_dead,
ReferenceError);
assertDoesNotThrow("'use strict'; " + prologue_dead + code + epilogue_dead);
assertThrows("'use strong'; " + prologue_live + code + epilogue_live, ReferenceError);
assertThrows("'use strict'; " + prologue_live + code + epilogue_live, ReferenceError);
assertThrows("'use strong'; " + prologue_live + code + epilogue_live,
ReferenceError);
assertDoesNotThrow("'use strict'; " + prologue_live + code + epilogue_live,
ReferenceError);
}
(function InitTimeReferenceForward() {
// It's never OK to have an init time reference to a class which hasn't been
// declared.
assertThrowsHelper(
`class A extends B { };
assertLateErrorsBecomeEarly(
`class A extends B { }
class B {}`);
assertThrowsHelper(
assertLateErrorsBecomeEarly(
`class A {
[B.sm()]() { }
};
}
class B {
static sm() { return 0; }
}`);
......@@ -54,7 +62,7 @@ function assertThrowsHelper(code) {
"use strong";
class A {
static sm() { return 0; }
};
}
let i = "making these classes non-consecutive";
class B extends A {};
"by inserting statements and declarations in between";
......@@ -68,10 +76,154 @@ function assertThrowsHelper(code) {
class A {
m() { B; }
static sm() { B; }
};
}
// No statements or declarations between the classes.
class B {
m() { A; }
static sm() { A; }
};
}
})();
(function MutualRecursionWithMoreClasses() {
"use strong";
class A {
m() { B; C; }
static sm() { B; C; }
}
class B {
m() { A; C; }
static sm() { A; C; }
}
class C {
m() { A; B; }
static sm() { A; B; }
}
})();
(function ReferringForwardInDeeperScopes() {
"use strong";
function foo() {
class A1 {
m() { B1; }
}
class B1 { }
}
class Outer {
m() {
class A2 {
m() { B2; }
}
class B2 { }
}
}
for (let i = 0; i < 1; ++i) {
class A3 {
m() { B3; }
}
class B3 { }
}
(a, b) => {
class A4 {
m() { B4; }
}
class B4 { }
}
})();
(function ReferringForwardButClassesNotConsecutive() {
assertNonErrorsBecomeEarly(
`class A {
m() { B; }
}
;
class B {}`);
assertNonErrorsBecomeEarly(
`let A = class {
m() { B; }
}
class B {}`);
assertNonErrorsBecomeEarly(
`class A {
m() { B1; } // Just a normal use-before-declaration.
}
let B1 = class B2 {}`);
assertNonErrorsBecomeEarly(
`class A {
m() { B; }
}
let i = 0;
class B {}`);
assertNonErrorsBecomeEarly(
`class A {
m() { B; }
}
function foo() {}
class B {}`);
assertNonErrorsBecomeEarly(
`function foo() {
class A {
m() { B; }
}
}
class B {}`);
assertNonErrorsBecomeEarly(
`class A extends class B { m() { C; } } {
}
class C { }`);
assertLateErrorsBecomeEarly(
`class A extends class B { [C.sm()]() { } } {
}
class C { static sm() { return 'a';} }`);
assertLateErrorsBecomeEarly(
`class A extends class B extends C { } {
}
class C { }`);
})();
(function RegressionForClassResolution() {
assertNonErrorsBecomeEarly(
`let A = class B {
m() { C; }
}
;;;;
class C {}
class B {}`);
})();
(function TestMultipleMethodScopes() {
"use strong";
// Test cases where the reference is inside multiple method scopes.
class A1 {
m() {
class C1 {
m() { B1; }
}
}
}
class B1 { }
;
class A2 {
m() {
class C2 extends B2 {
}
}
}
class B2 { }
})();
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment