Skip to content

Commit

Permalink
deps: backport 39642fa from upstream V8
Browse files Browse the repository at this point in the history
This is an almost clean cherry-pick of the original commit. The only
conflict was related to a rename of an internal class.

Original commit message:

    [async-await] (simpler) fix for Return in try/finally in async functions

    Alternative approach to https://codereview.chromium.org/2667983004/, which
    does not depend on implicit control flow changes from
    https://codereview.chromium.org/2664083002

    - Remove handling for `async function` from Parser::RewriteReturn(). This functionality
    is moved to BytecodeGenerator::BuildAsyncReturn(). This ensures that promise resolution
    is deferred until all finally blocks are evaluated fully.

    - Add a new deferred command (CMD_ASYNC_RETURN), which instructs ControlScope to
    generate return code using BuildAsyncReturn rather than BuildReturn.

    - Parser has a new `NewReturnStatement()` helper which determines what type of return
    statement to generate based on the type of function.

    BUG=v8:5896, v8:4483
    R=littledan@chromium.org, neis@chromium.org, rmcilroy@chromium.org, adamk@chromium.org, gsathya@chromium.org

    Review-Url: https://codereview.chromium.org/2685683002
    Cr-Commit-Position: refs/heads/master@{#43104}

Fixes: #11960
PR-URL: #11752
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
  • Loading branch information
targos committed Mar 25, 2017
1 parent 8394b05 commit 07088e6
Show file tree
Hide file tree
Showing 10 changed files with 373 additions and 38 deletions.
3 changes: 3 additions & 0 deletions deps/v8/src/ast/ast-numbering.cc
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ void AstNumberingVisitor::VisitExpressionStatement(ExpressionStatement* node) {
void AstNumberingVisitor::VisitReturnStatement(ReturnStatement* node) {
IncrementNodeCount();
Visit(node->expression());

DCHECK(!node->is_async_return() ||
properties_.flags() & AstProperties::kMustUseIgnitionTurbo);
}


Expand Down
20 changes: 17 additions & 3 deletions deps/v8/src/ast/ast.h
Original file line number Diff line number Diff line change
Expand Up @@ -895,17 +895,25 @@ class BreakStatement final : public JumpStatement {

class ReturnStatement final : public JumpStatement {
public:
enum Type { kNormal, kAsyncReturn };
Expression* expression() const { return expression_; }

void set_expression(Expression* e) { expression_ = e; }
Type type() const { return TypeField::decode(bit_field_); }
bool is_async_return() const { return type() == kAsyncReturn; }

private:
friend class AstNodeFactory;

ReturnStatement(Expression* expression, int pos)
: JumpStatement(pos, kReturnStatement), expression_(expression) {}
ReturnStatement(Expression* expression, Type type, int pos)
: JumpStatement(pos, kReturnStatement), expression_(expression) {
bit_field_ |= TypeField::encode(type);
}

Expression* expression_;

class TypeField
: public BitField<Type, JumpStatement::kNextBitFieldIndex, 1> {};
};


Expand Down Expand Up @@ -3217,7 +3225,13 @@ class AstNodeFactory final BASE_EMBEDDED {
}

ReturnStatement* NewReturnStatement(Expression* expression, int pos) {
return new (zone_) ReturnStatement(expression, pos);
return new (zone_)
ReturnStatement(expression, ReturnStatement::kNormal, pos);
}

ReturnStatement* NewAsyncReturnStatement(Expression* expression, int pos) {
return new (zone_)
ReturnStatement(expression, ReturnStatement::kAsyncReturn, pos);
}

WithStatement* NewWithStatement(Scope* scope,
Expand Down
27 changes: 22 additions & 5 deletions deps/v8/src/ast/scopes.cc
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ void DeclarationScope::SetDefaults() {
new_target_ = nullptr;
function_ = nullptr;
arguments_ = nullptr;
this_function_ = nullptr;
rare_data_ = nullptr;
should_eager_compile_ = false;
was_lazily_parsed_ = false;
#ifdef DEBUG
Expand Down Expand Up @@ -672,7 +672,7 @@ void DeclarationScope::DeclareDefaultFunctionVariables(

if (IsConciseMethod(function_kind_) || IsClassConstructor(function_kind_) ||
IsAccessorFunction(function_kind_)) {
this_function_ =
EnsureRareData()->this_function =
Declare(zone(), ast_value_factory->this_function_string(), CONST);
}
}
Expand All @@ -693,6 +693,24 @@ Variable* DeclarationScope::DeclareFunctionVar(const AstRawString* name) {
return function_;
}

Variable* DeclarationScope::DeclareGeneratorObjectVar(
const AstRawString* name) {
DCHECK(is_function_scope() || is_module_scope());
DCHECK_NULL(generator_object_var());

Variable* result = EnsureRareData()->generator_object = NewTemporary(name);
result->set_is_used();
return result;
}

Variable* DeclarationScope::DeclarePromiseVar(const AstRawString* name) {
DCHECK(is_function_scope());
DCHECK_NULL(promise_var());
Variable* result = EnsureRareData()->promise = NewTemporary(name);
result->set_is_used();
return result;
}

bool Scope::HasBeenRemoved() const {
if (sibling() == this) {
DCHECK_NULL(inner_scope_);
Expand Down Expand Up @@ -2053,9 +2071,8 @@ void DeclarationScope::AllocateLocals() {
new_target_ = nullptr;
}

if (this_function_ != nullptr && !MustAllocate(this_function_)) {
this_function_ = nullptr;
}
NullifyRareVariableIf(RareVariable::kThisFunction,
[=](Variable* var) { return !MustAllocate(var); });
}

void ModuleScope::AllocateModuleVariables() {
Expand Down
68 changes: 64 additions & 4 deletions deps/v8/src/ast/scopes.h
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,11 @@ class DeclarationScope : public Scope {
// calls sloppy eval.
Variable* DeclareFunctionVar(const AstRawString* name);

// Declare some special internal variables which must be accessible to
// Ignition without ScopeInfo.
Variable* DeclareGeneratorObjectVar(const AstRawString* name);
Variable* DeclarePromiseVar(const AstRawString* name);

// Declare a parameter in this scope. When there are duplicated
// parameters the rightmost one 'wins'. However, the implementation
// expects all parameters to be declared and from left to right.
Expand Down Expand Up @@ -714,6 +719,17 @@ class DeclarationScope : public Scope {
return function_;
}

Variable* generator_object_var() const {
DCHECK(is_function_scope() || is_module_scope());
return GetRareVariable(RareVariable::kGeneratorObject);
}

Variable* promise_var() const {
DCHECK(is_function_scope());
DCHECK(IsAsyncFunction(function_kind_));
return GetRareVariable(RareVariable::kPromise);
}

// Parameters. The left-most parameter has index 0.
// Only valid for function and module scopes.
Variable* parameter(int index) const {
Expand Down Expand Up @@ -754,12 +770,14 @@ class DeclarationScope : public Scope {
}

Variable* this_function_var() const {
Variable* this_function = GetRareVariable(RareVariable::kThisFunction);

// This is only used in derived constructors atm.
DCHECK(this_function_ == nullptr ||
DCHECK(this_function == nullptr ||
(is_function_scope() && (IsClassConstructor(function_kind()) ||
IsConciseMethod(function_kind()) ||
IsAccessorFunction(function_kind()))));
return this_function_;
return this_function;
}

// Adds a local variable in this scope's locals list. This is for adjusting
Expand Down Expand Up @@ -867,8 +885,50 @@ class DeclarationScope : public Scope {
Variable* new_target_;
// Convenience variable; function scopes only.
Variable* arguments_;
// Convenience variable; Subclass constructor only
Variable* this_function_;

struct RareData : public ZoneObject {
void* operator new(size_t size, Zone* zone) { return zone->New(size); }

// Convenience variable; Subclass constructor only
Variable* this_function = nullptr;

// Generator object, if any; generator function scopes and module scopes
// only.
Variable* generator_object = nullptr;
// Promise, if any; async function scopes only.
Variable* promise = nullptr;
};

enum class RareVariable {
kThisFunction = offsetof(RareData, this_function),
kGeneratorObject = offsetof(RareData, generator_object),
kPromise = offsetof(RareData, promise)
};

V8_INLINE RareData* EnsureRareData() {
if (rare_data_ == nullptr) {
rare_data_ = new (zone_) RareData;
}
return rare_data_;
}

V8_INLINE Variable* GetRareVariable(RareVariable id) const {
if (rare_data_ == nullptr) return nullptr;
return *reinterpret_cast<Variable**>(
reinterpret_cast<uint8_t*>(rare_data_) + static_cast<ptrdiff_t>(id));
}

// Set `var` to null if it's non-null and Predicate (Variable*) -> bool
// returns true.
template <typename Predicate>
V8_INLINE void NullifyRareVariableIf(RareVariable id, Predicate predicate) {
if (V8_LIKELY(rare_data_ == nullptr)) return;
Variable** var = reinterpret_cast<Variable**>(
reinterpret_cast<uint8_t*>(rare_data_) + static_cast<ptrdiff_t>(id));
if (*var && predicate(*var)) *var = nullptr;
}

RareData* rare_data_ = nullptr;
};

class ModuleScope final : public DeclarationScope {
Expand Down
45 changes: 43 additions & 2 deletions deps/v8/src/interpreter/bytecode-generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,19 @@ class BytecodeGenerator::ControlScope BASE_EMBEDDED {
void Break(Statement* stmt) { PerformCommand(CMD_BREAK, stmt); }
void Continue(Statement* stmt) { PerformCommand(CMD_CONTINUE, stmt); }
void ReturnAccumulator() { PerformCommand(CMD_RETURN, nullptr); }
void AsyncReturnAccumulator() { PerformCommand(CMD_ASYNC_RETURN, nullptr); }
void ReThrowAccumulator() { PerformCommand(CMD_RETHROW, nullptr); }

class DeferredCommands;

protected:
enum Command { CMD_BREAK, CMD_CONTINUE, CMD_RETURN, CMD_RETHROW };
enum Command {
CMD_BREAK,
CMD_CONTINUE,
CMD_RETURN,
CMD_ASYNC_RETURN,
CMD_RETHROW
};
void PerformCommand(Command command, Statement* statement);
virtual bool Execute(Command command, Statement* statement) = 0;

Expand Down Expand Up @@ -221,6 +228,9 @@ class BytecodeGenerator::ControlScopeForTopLevel final
case CMD_RETURN:
generator()->BuildReturn();
return true;
case CMD_ASYNC_RETURN:
generator()->BuildAsyncReturn();
return true;
case CMD_RETHROW:
generator()->BuildReThrow();
return true;
Expand Down Expand Up @@ -249,6 +259,7 @@ class BytecodeGenerator::ControlScopeForBreakable final
return true;
case CMD_CONTINUE:
case CMD_RETURN:
case CMD_ASYNC_RETURN:
case CMD_RETHROW:
break;
}
Expand Down Expand Up @@ -286,6 +297,7 @@ class BytecodeGenerator::ControlScopeForIteration final
loop_builder_->Continue();
return true;
case CMD_RETURN:
case CMD_ASYNC_RETURN:
case CMD_RETHROW:
break;
}
Expand All @@ -311,6 +323,7 @@ class BytecodeGenerator::ControlScopeForTryCatch final
case CMD_BREAK:
case CMD_CONTINUE:
case CMD_RETURN:
case CMD_ASYNC_RETURN:
break;
case CMD_RETHROW:
generator()->BuildReThrow();
Expand All @@ -337,6 +350,7 @@ class BytecodeGenerator::ControlScopeForTryFinally final
case CMD_BREAK:
case CMD_CONTINUE:
case CMD_RETURN:
case CMD_ASYNC_RETURN:
case CMD_RETHROW:
commands_->RecordCommand(command, statement);
try_finally_builder_->LeaveTry();
Expand Down Expand Up @@ -1045,7 +1059,12 @@ void BytecodeGenerator::VisitBreakStatement(BreakStatement* stmt) {
void BytecodeGenerator::VisitReturnStatement(ReturnStatement* stmt) {
builder()->SetStatementPosition(stmt);
VisitForAccumulatorValue(stmt->expression());
execution_control()->ReturnAccumulator();

if (stmt->is_async_return()) {
execution_control()->AsyncReturnAccumulator();
} else {
execution_control()->ReturnAccumulator();
}
}

void BytecodeGenerator::VisitWithStatement(WithStatement* stmt) {
Expand Down Expand Up @@ -1982,6 +2001,28 @@ void BytecodeGenerator::BuildReturn() {
builder()->Return();
}

void BytecodeGenerator::BuildAsyncReturn() {
DCHECK(IsAsyncFunction(info()->literal()->kind()));
RegisterAllocationScope register_scope(this);
RegisterList args = register_allocator()->NewRegisterList(3);
Register receiver = args[0];
Register promise = args[1];
Register return_value = args[2];
builder()->StoreAccumulatorInRegister(return_value);

Variable* var_promise = scope()->promise_var();
DCHECK_NOT_NULL(var_promise);
BuildVariableLoad(var_promise, FeedbackVectorSlot::Invalid(),
HoleCheckMode::kElided);
builder()
->StoreAccumulatorInRegister(promise)
.LoadUndefined()
.StoreAccumulatorInRegister(receiver)
.CallJSRuntime(Context::PROMISE_RESOLVE_INDEX, args)
.LoadAccumulatorWithRegister(promise);
BuildReturn();
}

void BytecodeGenerator::BuildReThrow() { builder()->ReThrow(); }

void BytecodeGenerator::BuildAbort(BailoutReason bailout_reason) {
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/interpreter/bytecode-generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
HoleCheckMode hole_check_mode);

void BuildReturn();
void BuildAsyncReturn();
void BuildReThrow();
void BuildAbort(BailoutReason bailout_reason);
void BuildThrowIfHole(Handle<String> name);
Expand Down
31 changes: 14 additions & 17 deletions deps/v8/src/parsing/parser-base.h
Original file line number Diff line number Diff line change
Expand Up @@ -419,23 +419,12 @@ class ParserBase {
FunctionKind kind() const { return scope()->function_kind(); }
FunctionState* outer() const { return outer_function_state_; }

void set_generator_object_variable(typename Types::Variable* variable) {
DCHECK_NOT_NULL(variable);
DCHECK(IsResumableFunction(kind()));
DCHECK(scope()->has_forced_context_allocation());
generator_object_variable_ = variable;
}
typename Types::Variable* generator_object_variable() const {
return generator_object_variable_;
return scope()->generator_object_var();
}

void set_promise_variable(typename Types::Variable* variable) {
DCHECK(variable != NULL);
DCHECK(IsAsyncFunction(kind()));
promise_variable_ = variable;
}
typename Types::Variable* promise_variable() const {
return promise_variable_;
return scope()->promise_var();
}

const ZoneList<DestructuringAssignment>&
Expand Down Expand Up @@ -1336,6 +1325,15 @@ class ParserBase {
return Call::NOT_EVAL;
}

// Convenience method which determines the type of return statement to emit
// depending on the current function type.
inline StatementT BuildReturnStatement(ExpressionT expr, int pos) {
if (V8_UNLIKELY(is_async_function())) {
return factory()->NewAsyncReturnStatement(expr, pos);
}
return factory()->NewReturnStatement(expr, pos);
}

// Validation per ES6 object literals.
class ObjectLiteralChecker {
public:
Expand Down Expand Up @@ -4074,9 +4072,8 @@ ParserBase<Impl>::ParseArrowFunctionLiteral(
} else {
ExpressionT expression = ParseAssignmentExpression(accept_IN, CHECK_OK);
impl()->RewriteNonPattern(CHECK_OK);
body->Add(
factory()->NewReturnStatement(expression, expression->position()),
zone());
body->Add(BuildReturnStatement(expression, expression->position()),
zone());
if (allow_tailcalls() && !is_sloppy(language_mode())) {
// ES6 14.6.1 Static Semantics: IsInTailPosition
impl()->MarkTailPosition(expression);
Expand Down Expand Up @@ -4981,7 +4978,7 @@ typename ParserBase<Impl>::StatementT ParserBase<Impl>::ParseReturnStatement(
}
ExpectSemicolon(CHECK_OK);
return_value = impl()->RewriteReturn(return_value, loc.beg_pos);
return factory()->NewReturnStatement(return_value, loc.beg_pos);
return BuildReturnStatement(return_value, loc.beg_pos);
}

template <typename Impl>
Expand Down
Loading

0 comments on commit 07088e6

Please sign in to comment.