AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert of [modules] Properly initialize declared variables. (patchset #5 id:80001 of https://codereview.chromium.org/2375793002/ )

Browse Source 
Reason for revert:
Suspect for causing win64 debug problems:
https://build.chromium.org/p/client.v8/builders/V8%20Win64%20-%20debug/builds/12646

Original issue's description:
> [modules] Properly initialize declared variables.
>
> Before evaluating a module, all variables declared at the top-level
> in _any_ of the modules in the dependency graph must be initialized.
> This is observable because a module A can access a variable imported
> from module B (e.g. a function) at a point when module B's body hasn't
> been evaluated yet.
>
> We achieve this by implementing modules internally as generators with
> two states (not initialized, initialized).
>
> R=adamk@chromium.org
> BUG=v8:1569
>
> Committed: https://crrev.com/f4dfb6fbe1cdd9a0f287a1a9c496e1f69f6f5d20
> Cr-Commit-Position: refs/heads/master@{#39871}

TBR=adamk@chromium.org,mstarzinger@chromium.org,neis@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=v8:1569

Review-Url: https://codereview.chromium.org/2379063002
Cr-Commit-Position: refs/heads/master@{#39873}
This commit is contained in:
machenbach 2016-09-29 08:10:07 -07:00 committed by Commit bot
parent 8c87212186
commit 7496c9de94
16 changed files with 49 additions and 164 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/ast/scopes.cc
View File

@ -152,8 +152,7 @@ DeclarationScope::DeclarationScope(Zone* zone, Scope* outer_scope,
ModuleScope::ModuleScope(DeclarationScope* script_scope,
AstValueFactory* ast_value_factory)
: DeclarationScope(ast_value_factory->zone(), script_scope, MODULE_SCOPE,
kModule) {
: DeclarationScope(ast_value_factory->zone(), script_scope, MODULE_SCOPE) {
Zone* zone = ast_value_factory->zone();
module_descriptor_ = new (zone) ModuleDescriptor(zone);
set_language_mode(STRICT);

6
src/bootstrapper.cc
View File

@ -703,12 +703,6 @@ void Genesis::CreateIteratorMaps(Handle<JSFunction> empty) {
SimpleInstallFunction(generator_object_prototype, "throw",
Builtins::kGeneratorPrototypeThrow, 1, true);
// Internal version of generator_prototype_next, flagged as non-native.
Handle<JSFunction> generator_next_internal =
SimpleCreateFunction(isolate(), factory()->next_string(),
Builtins::kGeneratorPrototypeNext, 1, true);
native_context()->set_generator_next_internal(*generator_next_internal);
// Create maps for generator functions and their prototypes. Store those
// maps in the native context. The "prototype" property descriptor is
// writable, non-enumerable, and non-configurable (as per ES6 draft

1
src/contexts.h
View File

@ -36,7 +36,6 @@ enum ContextLookupFlags {
#define NATIVE_CONTEXT_INTRINSIC_FUNCTIONS(V) \
V(IS_ARRAYLIKE, JSFunction, is_arraylike) \
V(GENERATOR_NEXT_INTERNAL, JSFunction, generator_next_internal) \
V(GET_TEMPLATE_CALL_SITE_INDEX, JSFunction, get_template_call_site) \
V(MAKE_ERROR_INDEX, JSFunction, make_error) \
V(MAKE_RANGE_ERROR_INDEX, JSFunction, make_range_error) \

9
src/globals.h
View File

@ -1078,7 +1078,6 @@ enum FunctionKind : uint16_t {
kGetterFunction = 1 << 6,
kSetterFunction = 1 << 7,
kAsyncFunction = 1 << 8,
kModule = 1 << 9,
kAccessorFunction = kGetterFunction | kSetterFunction,
kDefaultBaseConstructor = kDefaultConstructor | kBaseConstructor,
kDefaultSubclassConstructor = kDefaultConstructor | kSubclassConstructor,
@ -1092,7 +1091,6 @@ inline bool IsValidFunctionKind(FunctionKind kind) {
return kind == FunctionKind::kNormalFunction ||
kind == FunctionKind::kArrowFunction ||
kind == FunctionKind::kGeneratorFunction ||
kind == FunctionKind::kModule ||
kind == FunctionKind::kConciseMethod ||
kind == FunctionKind::kConciseGeneratorMethod ||
kind == FunctionKind::kGetterFunction ||
@ -1119,18 +1117,13 @@ inline bool IsGeneratorFunction(FunctionKind kind) {
return kind & FunctionKind::kGeneratorFunction;
}
inline bool IsModule(FunctionKind kind) {
DCHECK(IsValidFunctionKind(kind));
return kind & FunctionKind::kModule;
}
inline bool IsAsyncFunction(FunctionKind kind) {
DCHECK(IsValidFunctionKind(kind));
return kind & FunctionKind::kAsyncFunction;
}
inline bool IsResumableFunction(FunctionKind kind) {
return IsGeneratorFunction(kind) || IsAsyncFunction(kind) || IsModule(kind);
return IsGeneratorFunction(kind) || IsAsyncFunction(kind);
}
inline bool IsConciseMethod(FunctionKind kind) {

11
src/interpreter/bytecode-generator.cc
View File

@ -932,12 +932,7 @@ void BytecodeGenerator::VisitVariableDeclaration(VariableDeclaration* decl) {
break;
}
case VariableLocation::MODULE:
if (variable->IsExport() && variable->binding_needs_init()) {
builder()->LoadTheHole();
VisitVariableAssignment(variable, Token::INIT,
FeedbackVectorSlot::Invalid());
}
// Nothing to do for imports.
// Nothing to do here.
break;
}
}
@ -977,8 +972,7 @@ void BytecodeGenerator::VisitFunctionDeclaration(FunctionDeclaration* decl) {
break;
}
case VariableLocation::MODULE:
DCHECK_EQ(variable->mode(), LET);
DCHECK(variable->IsExport());
DCHECK(variable->mode() == LET);
VisitForAccumulatorValue(decl->fun());
VisitVariableAssignment(variable, Token::INIT,
FeedbackVectorSlot::Invalid());
@ -2024,7 +2018,6 @@ void BytecodeGenerator::VisitVariableLoad(Variable* variable,
.StoreAccumulatorInRegister(module_request)
.CallRuntime(Runtime::kLoadModuleImport, import_name, 2);
}
BuildHoleCheckForVariableLoad(variable);
break;
}
}

23
src/objects.cc
View File

@ -19906,7 +19906,7 @@ bool Module::Instantiate(Handle<Module> module, v8::Local<v8::Context> context,
}
MaybeHandle<Object> Module::Evaluate(Handle<Module> module) {
DCHECK(module->code()->IsJSFunction()); // Instantiated.
DCHECK(module->code()->IsJSFunction());
Isolate* isolate = module->GetIsolate();
@ -19914,28 +19914,17 @@ MaybeHandle<Object> Module::Evaluate(Handle<Module> module) {
if (module->evaluated()) return isolate->factory()->undefined_value();
module->set_evaluated(true);
// Initialization.
Handle<JSFunction> function(JSFunction::cast(module->code()), isolate);
DCHECK_EQ(MODULE_SCOPE, function->shared()->scope_info()->scope_type());
Handle<Object> receiver = isolate->factory()->undefined_value();
Handle<Object> argv[] = {module};
Handle<Object> generator;
ASSIGN_RETURN_ON_EXCEPTION(
isolate, generator,
Execution::Call(isolate, function, receiver, arraysize(argv), argv),
Object);
// Recursion.
Handle<FixedArray> requested_modules(module->requested_modules(), isolate);
for (int i = 0, length = requested_modules->length(); i < length; ++i) {
Handle<Module> import(Module::cast(requested_modules->get(i)), isolate);
RETURN_ON_EXCEPTION(isolate, Evaluate(import), Object);
}
// Evaluation of module body.
Handle<JSFunction> resume(
isolate->native_context()->generator_next_internal(), isolate);
return Execution::Call(isolate, resume, generator, 0, nullptr);
Handle<JSFunction> function(JSFunction::cast(module->code()), isolate);
DCHECK_EQ(MODULE_SCOPE, function->shared()->scope_info()->scope_type());
Handle<Object> receiver = isolate->factory()->undefined_value();
Handle<Object> argv[] = {module};
return Execution::Call(isolate, function, receiver, arraysize(argv), argv);
}
} // namespace internal

4
src/objects.h
View File

@ -4542,7 +4542,7 @@ class ScopeInfo : public FixedArray {
class HasSimpleParametersField
: public BitField<bool, AsmFunctionField::kNext, 1> {};
class FunctionKindField
: public BitField<FunctionKind, HasSimpleParametersField::kNext, 10> {};
: public BitField<FunctionKind, HasSimpleParametersField::kNext, 9> {};
class HasOuterScopeInfoField
: public BitField<bool, FunctionKindField::kNext, 1> {};
class IsDebugEvaluateScopeField
@ -7730,7 +7730,7 @@ class SharedFunctionInfo: public HeapObject {
ASSERT_FUNCTION_KIND_ORDER(kSetterFunction, kIsSetterFunction);
#undef ASSERT_FUNCTION_KIND_ORDER
class FunctionKindBits : public BitField<FunctionKind, kIsArrow, 10> {};
class FunctionKindBits : public BitField<FunctionKind, kIsArrow, 9> {};
class DeoptCountBits : public BitField<int, 0, 4> {};
class OptReenableTriesBits : public BitField<int, 4, 18> {};

79
src/parsing/parser.cc
View File

@ -768,13 +768,12 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
{
Scope* outer = original_scope_;
DCHECK_NOT_NULL(outer);
parsing_module_ = info->is_module();
if (info->is_eval()) {
if (!outer->is_script_scope() || is_strict(info->language_mode())) {
parsing_mode = PARSE_EAGERLY;
}
outer = NewEvalScope(outer);
} else if (parsing_module_) {
} else if (info->is_module()) {
DCHECK_EQ(outer, info->script_scope());
outer = NewModuleScope(info->script_scope());
// Never do lazy parsing in modules. If we want to support this in the
@ -794,6 +793,7 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
ZoneList<Statement*>* body = new(zone()) ZoneList<Statement*>(16, zone());
bool ok = true;
int beg_pos = scanner()->location().beg_pos;
parsing_module_ = info->is_module();
if (parsing_module_) {
// Declare the special module parameter.
auto name = ast_value_factory()->empty_string();
@ -805,13 +805,6 @@ FunctionLiteral* Parser::DoParseProgram(ParseInfo* info) {
DCHECK(!is_duplicate);
var->AllocateTo(VariableLocation::PARAMETER, 0);
PrepareGeneratorVariables(&function_state);
Expression* initial_yield =
BuildInitialYield(kNoSourcePosition, kGeneratorFunction);
body->Add(
factory()->NewExpressionStatement(initial_yield, kNoSourcePosition),
zone());
ParseModuleItemList(body, &ok);
ok = ok &&
module()->Validate(this->scope()->AsModuleScope(),
@ -2536,20 +2529,6 @@ void Parser::ReindexLiterals(const ParserFormalParameters& parameters) {
}
}
void Parser::PrepareGeneratorVariables(FunctionState* function_state) {
// For generators, allocating variables in contexts is currently a win
// because it minimizes the work needed to suspend and resume an
// activation. The machine code produced for generators (by full-codegen)
// relies on this forced context allocation, but not in an essential way.
scope()->ForceContextAllocation();
// Calling a generator returns a generator object. That object is stored
// in a temporary variable, a definition that is used by "yield"
// expressions.
Variable* temp =
NewTemporary(ast_value_factory()->dot_generator_object_string());
function_state->set_generator_object_variable(temp);
}
FunctionLiteral* Parser::ParseFunctionLiteral(
const AstRawString* function_name, Scanner::Location function_name_location,
@ -2671,7 +2650,20 @@ FunctionLiteral* Parser::ParseFunctionLiteral(
ExpressionClassifier formals_classifier(this, &duplicate_finder);
if (is_generator) PrepareGeneratorVariables(&function_state);
if (is_generator) {
// For generators, allocating variables in contexts is currently a win
// because it minimizes the work needed to suspend and resume an
// activation. The machine code produced for generators (by full-codegen)
// relies on this forced context allocation, but not in an essential way.
this->scope()->ForceContextAllocation();
// Calling a generator returns a generator object. That object is stored
// in a temporary variable, a definition that is used by "yield"
// expressions. This also marks the FunctionState as a generator.
Variable* temp =
NewTemporary(ast_value_factory()->dot_generator_object_string());
function_state.set_generator_object_variable(temp);
}
Expect(Token::LPAREN, CHECK_OK);
int start_position = scanner()->location().beg_pos;
@ -3132,21 +3124,6 @@ Variable* Parser::PromiseVariable() {
return promise;
}
Expression* Parser::BuildInitialYield(int pos, FunctionKind kind) {
Expression* allocation = BuildCreateJSGeneratorObject(pos, kind);
VariableProxy* init_proxy =
factory()->NewVariableProxy(function_state_->generator_object_variable());
Assignment* assignment = factory()->NewAssignment(
Token::INIT, init_proxy, allocation, kNoSourcePosition);
VariableProxy* get_proxy =
factory()->NewVariableProxy(function_state_->generator_object_variable());
// The position of the yield is important for reporting the exception
// caused by calling the .throw method on a generator suspended at the
// initial yield (i.e. right after generator instantiation).
return factory()->NewYield(get_proxy, assignment, scope()->start_position(),
Yield::kOnExceptionThrow);
}
ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
const AstRawString* function_name, int pos,
const ParserFormalParameters& parameters, FunctionKind kind,
@ -3199,10 +3176,26 @@ ZoneList<Statement*>* Parser::ParseEagerFunctionBody(
Block* try_block =
factory()->NewBlock(nullptr, 3, false, kNoSourcePosition);
Expression* initial_yield = BuildInitialYield(pos, kind);
try_block->statements()->Add(
factory()->NewExpressionStatement(initial_yield, kNoSourcePosition),
zone());
{
Expression* allocation = BuildCreateJSGeneratorObject(pos, kind);
VariableProxy* init_proxy = factory()->NewVariableProxy(
function_state_->generator_object_variable());
Assignment* assignment = factory()->NewAssignment(
Token::INIT, init_proxy, allocation, kNoSourcePosition);
VariableProxy* get_proxy = factory()->NewVariableProxy(
function_state_->generator_object_variable());
// The position of the yield is important for reporting the exception
// caused by calling the .throw method on a generator suspended at the
// initial yield (i.e. right after generator instantiation).
Yield* yield = factory()->NewYield(get_proxy, assignment,
scope()->start_position(),
Yield::kOnExceptionThrow);
try_block->statements()->Add(
factory()->NewExpressionStatement(yield, kNoSourcePosition),
zone());
}
ParseStatementList(try_block->statements(), Token::RBRACE, CHECK_OK);
Statement* final_return = factory()->NewReturnStatement(

3
src/parsing/parser.h
View File

@ -216,8 +216,6 @@ class Parser : public ParserBase<Parser> {
return scope()->NewTemporary(name);
}
void PrepareGeneratorVariables(FunctionState* function_state);
// Limit the allowed number of local variables in a function. The hard limit
// is that offsets computed by FullCodeGenerator::StackOperand and similar
// functions are ints, and they should not overflow. In addition, accessing
@ -577,7 +575,6 @@ class Parser : public ParserBase<Parser> {
friend class InitializerRewriter;
void RewriteParameterInitializer(Expression* expr, Scope* scope);
Expression* BuildInitialYield(int pos, FunctionKind kind);
Expression* BuildCreateJSGeneratorObject(int pos, FunctionKind kind);
Expression* BuildResolvePromise(Expression* value, int pos);
Expression* BuildRejectPromise(Expression* value, int pos);

2
test/cctest/interpreter/bytecode_expectations/CallRuntime.golden
View File

@ -79,7 +79,7 @@ bytecodes: [
/* 15 S> */ B(LdrUndefined), R(0),
B(CreateArrayLiteral), U8(0), U8(0), U8(9),
B(Star), R(1),
B(CallJSRuntime), U8(141), R(0), U8(2),
B(CallJSRuntime), U8(140), R(0), U8(2),
/* 44 S> */ B(Return),
]
constant pool: [

9
test/mjsunit/modules-error-trace.js
View File

@ -1,9 +0,0 @@
// Copyright 2016 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.
//
// MODULE
// Make sure the generator resume function doesn't show up in the stack trace.
const stack = (new Error).stack;
assertEquals(2, stack.split(/\r\n|\r|\n/).length);

9
test/mjsunit/modules-init1.js
View File

@ -1,9 +0,0 @@
// Copyright 2016 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.
//
// MODULE
import "modules-skip-init1.js";
export function bar() { return 42 };
bar = 5;

8
test/mjsunit/modules-init2.js
View File

@ -1,8 +0,0 @@
// Copyright 2016 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.
//
// MODULE
import {bar} from "modules-init1.js";
assertEquals(5, bar);

20
test/mjsunit/modules-init3.js
View File

@ -1,20 +0,0 @@
// Copyright 2016 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.
//
// MODULE
import {check} from "modules-skip-init3.js";
assertSame(undefined, w);
assertThrows(() => x, ReferenceError);
assertThrows(() => y, ReferenceError);
assertThrows(() => z, ReferenceError);
export function* v() { return 40 }
export var w = 41;
export let x = 42;
export class y {};
export const z = "hello world";
assertTrue(check());

6
test/mjsunit/modules-skip-init1.js
View File

@ -1,6 +0,0 @@
// 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.
import {bar} from "modules-init1.js";
assertEquals(42, bar());

20
test/mjsunit/modules-skip-init3.js
View File

@ -1,20 +0,0 @@
// Copyright 2016 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.
import {v, w, x, y, z} from "modules-init3.js";
assertEquals({value: 40, done: true}, v().next());
assertSame(undefined, w);
assertThrows(() => x, ReferenceError);
assertThrows(() => y, ReferenceError);
assertThrows(() => z, ReferenceError);
export function check() {
assertEquals({value: 40, done: true}, v().next());
assertEquals(41, w);
assertEquals(42, x);
assertEquals("y", y.name);
assertEquals("hello world", z);
return true;
}