Skip to content

Commit

Permalink
Initial implementation of optional chaining
Browse files Browse the repository at this point in the history
Each LHS expression that contains an optional chain of some form is
wrapped in an OptionalChain node. This root node allows us to use a
single jump location for every individual item in the chain,
improving the performance and simplifying the implementation.

Bug: v8:9553
Change-Id: I678563928b2dbfd6200bff55801919d4fd816962
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1723359
Commit-Queue: Adam Klein <adamk@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63120}
  • Loading branch information
devsnek authored and Commit Bot committed Aug 7, 2019
1 parent d109cdb commit ceb7bd5
Show file tree
Hide file tree
Showing 20 changed files with 418 additions and 34 deletions.
6 changes: 6 additions & 0 deletions src/ast/ast-traversal-visitor.h
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,12 @@ void AstTraversalVisitor<Subclass>::VisitThrow(Throw* expr) {
RECURSE_EXPRESSION(Visit(expr->exception()));
}

template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitOptionalChain(OptionalChain* expr) {
PROCESS_EXPRESSION(expr);
RECURSE_EXPRESSION(Visit(expr->expression()));
}

template <class Subclass>
void AstTraversalVisitor<Subclass>::VisitProperty(Property* expr) {
PROCESS_EXPRESSION(expr);
Expand Down
52 changes: 44 additions & 8 deletions src/ast/ast.h
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ namespace internal {
V(ImportCallExpression) \
V(Literal) \
V(NativeFunctionLiteral) \
V(OptionalChain) \
V(Property) \
V(ResolvedProperty) \
V(Spread) \
Expand Down Expand Up @@ -1595,6 +1596,20 @@ class VariableProxy final : public Expression {
friend base::ThreadedListTraits<VariableProxy>;
};

// Wraps an optional chain to provide a wrapper for jump labels.
class OptionalChain final : public Expression {
public:
Expression* expression() const { return expression_; }

private:
friend class AstNodeFactory;

explicit OptionalChain(Expression* expression)
: Expression(0, kOptionalChain), expression_(expression) {}

Expression* expression_;
};

// Assignments to a property will use one of several types of property access.
// Otherwise, the assignment is to a non-property (a global, a local slot, a
// parameter slot, or a destructuring pattern).
Expand All @@ -1612,6 +1627,10 @@ enum AssignType {

class Property final : public Expression {
public:
bool is_optional_chain_link() const {
return IsOptionalChainLinkField::decode(bit_field_);
}

bool IsValidReferenceExpression() const { return true; }

Expression* obj() const { return obj_; }
Expand Down Expand Up @@ -1653,10 +1672,13 @@ class Property final : public Expression {
private:
friend class AstNodeFactory;

Property(Expression* obj, Expression* key, int pos)
Property(Expression* obj, Expression* key, int pos, bool optional_chain)
: Expression(pos, kProperty), obj_(obj), key_(key) {
bit_field_ |= IsOptionalChainLinkField::encode(optional_chain);
}

using IsOptionalChainLinkField = Expression::NextBitField<bool, 1>;

Expression* obj_;
Expression* key_;
};
Expand Down Expand Up @@ -1694,6 +1716,10 @@ class Call final : public Expression {
return IsTaggedTemplateField::decode(bit_field_);
}

bool is_optional_chain_link() const {
return IsOptionalChainLinkField::decode(bit_field_);
}

bool only_last_arg_is_spread() {
return !arguments_.is_empty() && arguments_.last()->IsSpread();
}
Expand Down Expand Up @@ -1726,13 +1752,14 @@ class Call final : public Expression {

Call(Zone* zone, Expression* expression,
const ScopedPtrList<Expression>& arguments, int pos,
PossiblyEval possibly_eval)
PossiblyEval possibly_eval, bool optional_chain)
: Expression(pos, kCall),
expression_(expression),
arguments_(0, nullptr) {
bit_field_ |=
IsPossiblyEvalField::encode(possibly_eval == IS_POSSIBLY_EVAL) |
IsTaggedTemplateField::encode(false);
IsTaggedTemplateField::encode(false) |
IsOptionalChainLinkField::encode(optional_chain);
arguments.CopyTo(&arguments_, zone);
}

Expand All @@ -1743,12 +1770,14 @@ class Call final : public Expression {
expression_(expression),
arguments_(0, nullptr) {
bit_field_ |= IsPossiblyEvalField::encode(false) |
IsTaggedTemplateField::encode(true);
IsTaggedTemplateField::encode(true) |
IsOptionalChainLinkField::encode(false);
arguments.CopyTo(&arguments_, zone);
}

using IsPossiblyEvalField = Expression::NextBitField<bool, 1>;
using IsTaggedTemplateField = IsPossiblyEvalField::Next<bool, 1>;
using IsOptionalChainLinkField = IsTaggedTemplateField::Next<bool, 1>;

Expression* expression_;
ZonePtrList<Expression> arguments_;
Expand Down Expand Up @@ -3040,8 +3069,13 @@ class AstNodeFactory final {
return new (zone_) Variable(variable);
}

Property* NewProperty(Expression* obj, Expression* key, int pos) {
return new (zone_) Property(obj, key, pos);
OptionalChain* NewOptionalChain(Expression* expression) {
return new (zone_) OptionalChain(expression);
}

Property* NewProperty(Expression* obj, Expression* key, int pos,
bool optional_chain = false) {
return new (zone_) Property(obj, key, pos, optional_chain);
}

ResolvedProperty* NewResolvedProperty(VariableProxy* obj,
Expand All @@ -3052,8 +3086,10 @@ class AstNodeFactory final {

Call* NewCall(Expression* expression,
const ScopedPtrList<Expression>& arguments, int pos,
Call::PossiblyEval possibly_eval = Call::NOT_EVAL) {
return new (zone_) Call(zone_, expression, arguments, pos, possibly_eval);
Call::PossiblyEval possibly_eval = Call::NOT_EVAL,
bool optional_chain = false) {
return new (zone_)
Call(zone_, expression, arguments, pos, possibly_eval, optional_chain);
}

Call* NewTaggedTemplate(Expression* expression,
Expand Down
14 changes: 14 additions & 0 deletions src/ast/prettyprinter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -342,19 +342,28 @@ void CallPrinter::VisitAwait(Await* node) { Find(node->expression()); }

void CallPrinter::VisitThrow(Throw* node) { Find(node->exception()); }

void CallPrinter::VisitOptionalChain(OptionalChain* node) {
Find(node->expression());
}

void CallPrinter::VisitProperty(Property* node) {
Expression* key = node->key();
Literal* literal = key->AsLiteral();
if (literal != nullptr &&
literal->BuildValue(isolate_)->IsInternalizedString()) {
Find(node->obj(), true);
if (node->is_optional_chain_link()) {
Print("?");
}
Print(".");
// TODO(adamk): Teach Literal how to print its values without
// allocating on the heap.
PrintLiteral(literal->BuildValue(isolate_), false);
} else {
Find(node->obj(), true);
if (node->is_optional_chain_link()) {
Print("?.");
}
Print("[");
Find(key, true);
Print("]");
Expand Down Expand Up @@ -1272,6 +1281,11 @@ void AstPrinter::VisitThrow(Throw* node) {
Visit(node->exception());
}

void AstPrinter::VisitOptionalChain(OptionalChain* node) {
IndentedScope indent(this, "OPTIONAL_CHAIN", node->position());
Visit(node->expression());
}

void AstPrinter::VisitProperty(Property* node) {
EmbeddedVector<char, 128> buf;
SNPrintF(buf, "PROPERTY");
Expand Down
5 changes: 4 additions & 1 deletion src/common/message-template.h
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,10 @@ namespace internal {
"FinalizationGroup.prototype.register: target and holdings must not be " \
"same") \
T(WeakRefsWeakRefConstructorTargetMustBeObject, \
"WeakRef: target must be an object")
"WeakRef: target must be an object") \
T(OptionalChainingNoNew, "Invalid optional chain from new expression") \
T(OptionalChainingNoSuper, "Invalid optional chain from super property") \
T(OptionalChainingNoTemplate, "Invalid tagged template on optional chain")

enum class MessageTemplate {
#define TEMPLATE(NAME, STRING) k##NAME,
Expand Down
3 changes: 2 additions & 1 deletion src/flags/flag-definitions.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,8 @@ DEFINE_IMPLICATION(harmony_import_meta, harmony_dynamic_import)
#define HARMONY_INPROGRESS_BASE(V) \
V(harmony_private_methods, "harmony private methods in class literals") \
V(harmony_regexp_sequence, "RegExp Unicode sequence properties") \
V(harmony_weak_refs, "harmony weak references")
V(harmony_weak_refs, "harmony weak references") \
V(harmony_optional_chaining, "harmony optional chaining syntax")

#ifdef V8_INTL_SUPPORT
#define HARMONY_INPROGRESS(V) \
Expand Down
1 change: 1 addition & 0 deletions src/init/bootstrapper.cc
Original file line number Diff line number Diff line change
Expand Up @@ -4260,6 +4260,7 @@ EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_private_methods)
EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_dynamic_import)
EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_import_meta)
EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_regexp_sequence)
EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_optional_chaining)

#ifdef V8_INTL_SUPPORT
EMPTY_INITIALIZE_GLOBAL_FOR_FEATURE(harmony_intl_add_calendar_numbering_system)
Expand Down
73 changes: 73 additions & 0 deletions src/interpreter/bytecode-generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -915,6 +915,27 @@ class BytecodeGenerator::IteratorRecord final {
Register next_;
};

class BytecodeGenerator::OptionalChainNullLabelScope final {
public:
explicit OptionalChainNullLabelScope(BytecodeGenerator* bytecode_generator)
: bytecode_generator_(bytecode_generator),
labels_(bytecode_generator->zone()) {
prev_ = bytecode_generator_->optional_chaining_null_labels_;
bytecode_generator_->optional_chaining_null_labels_ = &labels_;
}

~OptionalChainNullLabelScope() {
bytecode_generator_->optional_chaining_null_labels_ = prev_;
}

BytecodeLabels* labels() { return &labels_; }

private:
BytecodeGenerator* bytecode_generator_;
BytecodeLabels labels_;
BytecodeLabels* prev_;
};

namespace {

// A map from property names to getter/setter pairs allocated in the zone that
Expand Down Expand Up @@ -994,6 +1015,7 @@ BytecodeGenerator::BytecodeGenerator(
execution_context_(nullptr),
execution_result_(nullptr),
incoming_new_target_or_generator_(),
optional_chaining_null_labels_(nullptr),
dummy_feedback_slot_(feedback_spec(), FeedbackSlotKind::kCompareOp),
generator_jump_table_(nullptr),
suspend_count_(0),
Expand Down Expand Up @@ -4318,7 +4340,17 @@ void BytecodeGenerator::VisitThrow(Throw* expr) {
}

void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* property) {
if (property->is_optional_chain_link()) {
DCHECK_NOT_NULL(optional_chaining_null_labels_);
// TODO(ignition): Add a single opcode for JumpIfNullOrUndefined
builder()
->LoadAccumulatorWithRegister(obj)
.JumpIfUndefined(optional_chaining_null_labels_->New())
.JumpIfNull(optional_chaining_null_labels_->New());
}

AssignType property_kind = Property::GetAssignType(property);

switch (property_kind) {
case NON_PROPERTY:
UNREACHABLE();
Expand Down Expand Up @@ -4416,6 +4448,16 @@ void BytecodeGenerator::VisitKeyedSuperPropertyLoad(Property* property,
}
}

void BytecodeGenerator::VisitOptionalChain(OptionalChain* expr) {
BytecodeLabel done;
OptionalChainNullLabelScope label_scope(this);
VisitForAccumulatorValue(expr->expression());
builder()->Jump(&done);
label_scope.labels()->Bind(builder());
builder()->LoadUndefined();
builder()->Bind(&done);
}

void BytecodeGenerator::VisitProperty(Property* expr) {
AssignType property_kind = Property::GetAssignType(expr);
if (property_kind != NAMED_SUPER_PROPERTY &&
Expand Down Expand Up @@ -4549,6 +4591,15 @@ void BytecodeGenerator::VisitCall(Call* expr) {
UNREACHABLE();
}

if (expr->is_optional_chain_link()) {
DCHECK_NOT_NULL(optional_chaining_null_labels_);
// TODO(ignition): Add a single opcode for JumpIfNullOrUndefined
builder()
->LoadAccumulatorWithRegister(callee)
.JumpIfUndefined(optional_chaining_null_labels_->New())
.JumpIfNull(optional_chaining_null_labels_->New());
}

// Evaluate all arguments to the function call and store in sequential args
// registers.
VisitArguments(expr->arguments(), &args);
Expand Down Expand Up @@ -4810,6 +4861,28 @@ void BytecodeGenerator::VisitDelete(UnaryOperation* unary) {
Register object = VisitForRegisterValue(property->obj());
VisitForAccumulatorValue(property->key());
builder()->Delete(object, language_mode());
} else if (expr->IsOptionalChain()) {
Expression* expr_inner = expr->AsOptionalChain()->expression();
if (expr_inner->IsProperty()) {
Property* property = expr_inner->AsProperty();
DCHECK(!property->IsPrivateReference());
BytecodeLabel done;
OptionalChainNullLabelScope label_scope(this);
VisitForAccumulatorValue(property->obj());
builder()
->JumpIfUndefined(label_scope.labels()->New())
.JumpIfNull(label_scope.labels()->New());
Register object = register_allocator()->NewRegister();
builder()->StoreAccumulatorInRegister(object);
VisitForAccumulatorValue(property->key());
builder()->Delete(object, language_mode()).Jump(&done);
label_scope.labels()->Bind(builder());
builder()->LoadTrue();
builder()->Bind(&done);
} else {
VisitForEffect(expr);
builder()->LoadTrue();
}
} else if (expr->IsVariableProxy() &&
!expr->AsVariableProxy()->is_new_target()) {
// Delete of an unqualified identifier is allowed in sloppy mode but is
Expand Down
3 changes: 3 additions & 0 deletions src/interpreter/bytecode-generator.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {
class AccumulatorPreservingScope;
class TestResultScope;
class ValueResultScope;
class OptionalChainNullLabelScope;

using ToBooleanMode = BytecodeArrayBuilder::ToBooleanMode;

Expand Down Expand Up @@ -491,6 +492,8 @@ class BytecodeGenerator final : public AstVisitor<BytecodeGenerator> {

Register incoming_new_target_or_generator_;

BytecodeLabels* optional_chaining_null_labels_;

// Dummy feedback slot for compare operations, where we don't care about
// feedback
SharedFeedbackSlot dummy_feedback_slot_;
Expand Down
1 change: 1 addition & 0 deletions src/parsing/parse-info.cc
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ ParseInfo::ParseInfo(Isolate* isolate, AccountingAllocator* zone_allocator)
set_allow_natives_syntax(FLAG_allow_natives_syntax);
set_allow_harmony_dynamic_import(FLAG_harmony_dynamic_import);
set_allow_harmony_import_meta(FLAG_harmony_import_meta);
set_allow_harmony_optional_chaining(FLAG_harmony_optional_chaining);
set_allow_harmony_private_methods(FLAG_harmony_private_methods);
}

Expand Down
11 changes: 7 additions & 4 deletions src/parsing/parse-info.h
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,8 @@ class V8_EXPORT_PRIVATE ParseInfo {
set_allow_harmony_dynamic_import)
FLAG_ACCESSOR(kAllowHarmonyImportMeta, allow_harmony_import_meta,
set_allow_harmony_import_meta)
FLAG_ACCESSOR(kAllowHarmonyOptionalChaining, allow_harmony_optional_chaining,
set_allow_harmony_optional_chaining)
FLAG_ACCESSOR(kAllowHarmonyPrivateMethods, allow_harmony_private_methods,
set_allow_harmony_private_methods)
FLAG_ACCESSOR(kIsOneshotIIFE, is_oneshot_iife, set_is_oneshot_iife)
Expand Down Expand Up @@ -304,10 +306,11 @@ class V8_EXPORT_PRIVATE ParseInfo {
kAllowHarmonyStaticFields = 1 << 24,
kAllowHarmonyDynamicImport = 1 << 25,
kAllowHarmonyImportMeta = 1 << 26,
kAllowHarmonyPrivateFields = 1 << 27,
kAllowHarmonyPrivateMethods = 1 << 28,
kIsOneshotIIFE = 1 << 29,
kCollectSourcePositions = 1 << 30,
kAllowHarmonyOptionalChaining = 1 << 27,
kAllowHarmonyPrivateFields = 1 << 28,
kAllowHarmonyPrivateMethods = 1 << 29,
kIsOneshotIIFE = 1 << 30,
kCollectSourcePositions = 1 << 31,
};

//------------- Inputs to parsing and scope analysis -----------------------
Expand Down
Loading

0 comments on commit ceb7bd5

Please sign in to comment.