Reviewers: Dmitry Lomov (chromium),
Message:
dslomov, could you have an early look at this "dynamic strong mode scoping
errors CL?
The approach here is that when binding a script to a context, we check the
free
variables we know of (so, not inside lazy functions) against the context.
This CL does *not* implement checking variables inside lazy functions
whenever
the lazy func is compiled.
(Iirc this was the approach we talked about, though other approaches such as
doing the check whenever a function is called were on the table... so this
checks inside eager functions at compile time, not at call time.)
No tests yet because there's no trivial way to make a test consist of
multiple
.js files (though, d8 does the right thing w/ "d8 foo.js bar.js", so,
compiles
them separately).
Description:
Strawman: check strong mode free variables against the global object.
[Work in progress, not for committing]
BUG=
Please review this at https://codereview.chromium.org/1005063002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+75, -16 lines):
M src/api.cc
M src/messages.js
M src/objects.h
M src/scopeinfo.cc
M src/scopes.h
M src/scopes.cc
Index: src/api.cc
diff --git a/src/api.cc b/src/api.cc
index
2d2bd1f23b8b11d80b6c1bc190a856b43b15ac99..af7459cc09f18d5a92fe8b73138b6dc8e75a8626
100644
--- a/src/api.cc
+++ b/src/api.cc
@@ -36,6 +36,7 @@
#include "src/messages.h"
#include "src/natives.h"
#include "src/parser.h"
+#include "src/pending-compilation-error-handler.h"
#include "src/profile-generator-inl.h"
#include "src/property.h"
#include "src/property-details.h"
@@ -1490,9 +1491,29 @@ Local<Script> UnboundScript::BindToCurrentContext() {
i::Handle<i::HeapObject>::cast(Utils::OpenHandle(this));
i::Handle<i::SharedFunctionInfo>
function_info(i::SharedFunctionInfo::cast(*obj), obj->GetIsolate());
+ i::Isolate* isolate = obj->GetIsolate();
+
+ i::ScopeInfo* scope_info = function_info->scope_info();
+ i::Handle<i::JSReceiver>
global(isolate->native_context()->global_object());
+ for (int i = 0; i < scope_info->StrongModeFreeVariableCount(); ++i) {
+ i::Handle<i::Name> name(scope_info->StrongModeFreeVariableName(i));
+ Maybe<bool> has = i::JSReceiver::HasOwnProperty(global, name);
+ if (has.IsJust() && !has.FromJust()) {
+ i::PendingCompilationErrorHandler pending_error_handler_;
+ pending_error_handler_.ReportMessageAt(
+ // FIXME: location
+ 0, 0, "strong_unbound_global", reinterpret_cast<const
char*>(NULL),
+ i::kReferenceError);
+ i::Handle<i::Script>
script(i::Script::cast(function_info->script()));
+ pending_error_handler_.ThrowPendingError(isolate, script);
+ isolate->ReportPendingMessages();
+ isolate->OptionalRescheduleException(true);
+ return Local<Script>();
+ }
+ }
i::Handle<i::JSFunction> function =
obj->GetIsolate()->factory()->NewFunctionFromSharedFunctionInfo(
- function_info, obj->GetIsolate()->native_context());
+ function_info, isolate->native_context());
return ToApiHandle<Script>(function);
}
@@ -1940,7 +1961,9 @@ MaybeLocal<Script>
ScriptCompiler::Compile(Local<Context> context,
i::Handle<i::SharedFunctionInfo> result(raw_result, isolate);
Local<UnboundScript> generic = ToApiHandle<UnboundScript>(result);
if (generic.IsEmpty()) return Local<Script>();
- RETURN_ESCAPED(generic->BindToCurrentContext());
+ Local<Script> bound = generic->BindToCurrentContext();
+ if (bound.IsEmpty()) return Local<Script>();
+ RETURN_ESCAPED(bound);
}
Index: src/messages.js
diff --git a/src/messages.js b/src/messages.js
index
12170c463a54cfca49b984226a5e07964ed57f32..b7df55df51f5a3d51a4cab067fa7d68521c64452
100644
--- a/src/messages.js
+++ b/src/messages.js
@@ -172,6 +172,7 @@ var kMessages = {
strong_for_in: ["In strong mode, 'for'-'in' loops are
deprecated, use 'for'-'of' instead"],
strong_empty: ["In strong mode, empty sub-statements
are deprecated, make them explicit with '{}' instead"],
strong_use_before_declaration: ["In strong mode, declaring
variable '", "%0", "' before its use is required"],
+ strong_unbound_global: ["In strong mode, using an undeclared
global variable is not allowed"],
strong_super_call_missing: ["In strong mode, invoking the super
constructor in a subclass is required"],
strong_super_call_duplicate: ["In strong mode, invoking the super
constructor multiple times is deprecated"],
strong_super_call_nested: ["In strong mode, invoking the super
constructor nested inside another statement or expression is deprecated"],
Index: src/objects.h
diff --git a/src/objects.h b/src/objects.h
index
cf6d1613e6545724162d07b7fa22e4ab759f58c6..2c4f3ddb45cd3cd5c588555646966c7ee22119ac
100644
--- a/src/objects.h
+++ b/src/objects.h
@@ -4306,6 +4306,8 @@ class ScopeInfo : public FixedArray {
// exposed to the user in a debugger.
bool LocalIsSynthetic(int var);
+ String* StrongModeFreeVariableName(int var);
+
// Lookup support for serialized scope info. Returns the
// the stack slot index for a given slot name if the slot is
// present; otherwise returns a value < 0. The name must be an
internalized
@@ -4358,11 +4360,12 @@ class ScopeInfo : public FixedArray {
// 3. The number of non-parameter variables allocated on the stack.
// 4. The number of non-parameter and parameter variables allocated in
the
// context.
-#define FOR_EACH_NUMERIC_FIELD(V) \
- V(Flags) \
- V(ParameterCount) \
- V(StackLocalCount) \
- V(ContextLocalCount)
+#define FOR_EACH_NUMERIC_FIELD(V) \
+ V(Flags) \
+ V(ParameterCount) \
+ V(StackLocalCount) \
+ V(ContextLocalCount) \
+ V(StrongModeFreeVariableCount)
#define FIELD_ACCESSORS(name) \
void Set##name(int value) { \
@@ -4414,10 +4417,12 @@ class ScopeInfo : public FixedArray {
// information about the function variable. It always occupies two
array
// slots: a. The name of the function variable.
// b. The context or stack slot index for the variable.
+ // FIXME: update the comment
int ParameterEntriesIndex();
int StackLocalEntriesIndex();
int ContextLocalNameEntriesIndex();
int ContextLocalInfoEntriesIndex();
+ int StrongModeFreeVariableEntriesIndex();
int FunctionNameEntryIndex();
// Location of the function variable for named function expressions.
Index: src/scopeinfo.cc
diff --git a/src/scopeinfo.cc b/src/scopeinfo.cc
index
d62bc4129588196d3f3faf6d7c1fd035b39bbaa6..ed4705465e0e293b0baad49035bfdb38e1e32141
100644
--- a/src/scopeinfo.cc
+++ b/src/scopeinfo.cc
@@ -18,9 +18,14 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate,
Zone* zone,
// Collect stack and context locals.
ZoneList<Variable*> stack_locals(scope->StackLocalCount(), zone);
ZoneList<Variable*> context_locals(scope->ContextLocalCount(), zone);
- scope->CollectStackAndContextLocals(&stack_locals, &context_locals);
+ ZoneList<Variable*> strong_mode_free_variables(0, zone);
+
+ scope->CollectStackAndContextLocals(&stack_locals, &context_locals,
+ &strong_mode_free_variables);
const int stack_local_count = stack_locals.length();
const int context_local_count = context_locals.length();
+ const int strong_mode_free_variable_count =
+ strong_mode_free_variables.length();
// Make sure we allocate the correct amount.
DCHECK(scope->StackLocalCount() == stack_local_count);
DCHECK(scope->ContextLocalCount() == context_local_count);
@@ -49,9 +54,9 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate,
Zone* zone,
const bool has_function_name = function_name_info != NONE;
const int parameter_count = scope->num_parameters();
- const int length = kVariablePartIndex
- + parameter_count + stack_local_count + 2 * context_local_count
- + (has_function_name ? 2 : 0);
+ const int length = kVariablePartIndex + parameter_count +
stack_local_count +
+ 2 * context_local_count + (has_function_name ? 2 : 0)
+
+ strong_mode_free_variable_count;
Factory* factory = isolate->factory();
Handle<ScopeInfo> scope_info = factory->NewScopeInfo(length);
@@ -71,6 +76,7 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate,
Zone* zone,
scope_info->SetParameterCount(parameter_count);
scope_info->SetStackLocalCount(stack_local_count);
scope_info->SetContextLocalCount(context_local_count);
+
scope_info->SetStrongModeFreeVariableCount(strong_mode_free_variable_count);
int index = kVariablePartIndex;
// Add parameters.
@@ -113,6 +119,11 @@ Handle<ScopeInfo> ScopeInfo::Create(Isolate* isolate,
Zone* zone,
scope_info->set(index++, Smi::FromInt(value));
}
+ DCHECK(index == scope_info->StrongModeFreeVariableEntriesIndex());
+ for (int i = 0; i < strong_mode_free_variable_count; ++i) {
+ scope_info->set(index++, *strong_mode_free_variables[i]->name());
+ }
+
// If present, add the function variable name and its index.
DCHECK(index == scope_info->FunctionNameEntryIndex());
if (has_function_name) {
@@ -285,6 +296,13 @@ bool ScopeInfo::LocalIsSynthetic(int var) {
}
+String* ScopeInfo::StrongModeFreeVariableName(int var) {
+ DCHECK(0 <= var && var < StrongModeFreeVariableCount());
+ int info_index = StrongModeFreeVariableEntriesIndex() + var;
+ return String::cast(get(info_index));
+}
+
+
int ScopeInfo::StackSlotIndex(String* name) {
DCHECK(name->IsInternalizedString());
if (length() > 0) {
@@ -432,11 +450,16 @@ int ScopeInfo::ContextLocalInfoEntriesIndex() {
}
-int ScopeInfo::FunctionNameEntryIndex() {
+int ScopeInfo::StrongModeFreeVariableEntriesIndex() {
return ContextLocalInfoEntriesIndex() + ContextLocalCount();
}
+int ScopeInfo::FunctionNameEntryIndex() {
+ return StrongModeFreeVariableEntriesIndex() +
StrongModeFreeVariableCount();
+}
+
+
int ContextSlotCache::Hash(Object* data, String* name) {
// Uses only lower 32 bits if pointers are larger.
uintptr_t addr_hash =
Index: src/scopes.cc
diff --git a/src/scopes.cc b/src/scopes.cc
index
c744dd418bbaaca2b74567048f30ecf208274da3..7e7955fba17c3dc0e13703b5ed5a0c2ce2ac0f24
100644
--- a/src/scopes.cc
+++ b/src/scopes.cc
@@ -593,8 +593,9 @@ class VarAndOrder {
};
-void Scope::CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
- ZoneList<Variable*>*
context_locals) {
+void Scope::CollectStackAndContextLocals(
+ ZoneList<Variable*>* stack_locals, ZoneList<Variable*>* context_locals,
+ ZoneList<Variable*>* strong_mode_free_variables) {
DCHECK(stack_locals != NULL);
DCHECK(context_locals != NULL);
@@ -628,6 +629,11 @@ void
Scope::CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
p != NULL;
p = variables_.Next(p)) {
Variable* var = reinterpret_cast<Variable*>(p->value);
+ if (is_strong(language_mode()) && strong_mode_free_variables &&
+ var->mode() == DYNAMIC_GLOBAL) {
+ strong_mode_free_variables->Add(var, zone());
+ }
+
if (var->is_used()) {
vars.Add(VarAndOrder(var, p->order), zone());
}
Index: src/scopes.h
diff --git a/src/scopes.h b/src/scopes.h
index
44434fa3622091d19d6b9e511e881883fb0b802f..25c629a2e3a89824df2ab9fa18dced4e73f1849d
100644
--- a/src/scopes.h
+++ b/src/scopes.h
@@ -415,8 +415,9 @@ class Scope: public ZoneObject {
// Collect stack and context allocated local variables in this scope.
Note
// that the function variable - if present - is not collected and should
be
// handled separately.
- void CollectStackAndContextLocals(ZoneList<Variable*>* stack_locals,
- ZoneList<Variable*>* context_locals);
+ void CollectStackAndContextLocals(
+ ZoneList<Variable*>* stack_locals, ZoneList<Variable*>*
context_locals,
+ ZoneList<Variable*>* strong_mode_free_variables = nullptr);
// Current number of var or const locals.
int num_var_or_const() { return num_var_or_const_; }
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
