Skip to content

Commit

Permalink
Implement optional chains (#2390)
Browse files Browse the repository at this point in the history 
This Pull Request implements optional chains.

Example:

```Javascript
const adventurer = {
    name: 'Alice',
    cat: {
        name: 'Dinah'
    }
};

console.log(adventurer.cat?.name); // Dinah
console.log(adventurer.dog?.name); // undefined
```

Since I needed to implement `Opcode::RotateLeft`, and #2378 had an implementation for `Opcode::RotateRight`, I took the opportunity to integrate both ops into this PR (big thanks to @HalidOdat for the original implementation!).

This PR almost has 100% conformance for the `optional-chaining` test suite. However, there's this one [test](https://github.com/tc39/test262/blob/85373b4ce12a908f8fc517093d95cf2ed2f5ee6a/test/language/expressions/optional-chaining/member-expression.js) that can't be solved until we properly set function names for function expressions in object and class definitions.
  • Loading branch information
@jedel1043
jedel1043 committed Oct 29, 2022
1 parent f446c09 commit 4b892a9
Show file tree
Hide file tree
Showing 17 changed files with 898 additions and 113 deletions.
257 changes: 211 additions & 46 deletions boa_engine/src/bytecompiler/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
binary::{ArithmeticOp, BinaryOp, BitwiseOp, LogicalOp, RelationalOp},
unary::UnaryOp,
},
Call, Identifier, New,
Call, Identifier, New, Optional, OptionalOperationKind,
},
function::{
ArrowFunction, AsyncFunction, AsyncGenerator, Class, ClassElement, FormalParameterList,
Expand Down Expand Up @@ -430,6 +430,14 @@ impl<'b> ByteCompiler<'b> {
Label { index }
}

#[inline]
fn jump_if_null_or_undefined(&mut self) -> Label {
let index = self.next_opcode_location();
self.emit(Opcode::JumpIfNullOrUndefined, &[Self::DUMMY_ADDRESS]);

Label { index }
}

/// Emit an opcode with a dummy operand.
/// Return the `Label` of the operand.
#[inline]
Expand Down Expand Up @@ -1109,6 +1117,7 @@ impl<'b> ByteCompiler<'b> {
}
},
PropertyDefinition::MethodDefinition(name, kind) => match kind {
// TODO: set function name for getter and setters
MethodDefinition::Get(expr) => match name {
PropertyName::Literal(name) => {
self.function(expr.into(), NodeKind::Expression, true)?;
Expand All @@ -1123,6 +1132,7 @@ impl<'b> ByteCompiler<'b> {
self.emit_opcode(Opcode::SetPropertyGetterByValue);
}
},
// TODO: set function name for getter and setters
MethodDefinition::Set(expr) => match name {
PropertyName::Literal(name) => {
self.function(expr.into(), NodeKind::Expression, true)?;
Expand Down Expand Up @@ -1443,12 +1453,201 @@ impl<'b> ByteCompiler<'b> {
self.emit_opcode(Opcode::PushNewTarget);
}
}
Expression::Optional(opt) => {
self.compile_optional_preserve_this(opt)?;

self.emit_opcode(Opcode::Swap);
self.emit_opcode(Opcode::Pop);

if !use_expr {
self.emit_opcode(Opcode::Pop);
}
}
// TODO: try to remove this variant somehow
Expression::FormalParameterList(_) => unreachable!(),
}
Ok(())
}

/// Compile a property access expression, prepending `this` to the property value in the stack.
///
/// This compiles the access in a way that the state of the stack after executing the property
/// access becomes `...rest, this, value`. where `...rest` is the rest of the stack, `this` is the
/// `this` value of the access, and `value` is the final result of the access.
///
/// This is mostly useful for optional chains with calls (`a.b?.()`) and for regular chains
/// with calls (`a.b()`), since both of them must have `a` be the value of `this` for the function
/// call `b()`, but a regular compilation of the access would lose the `this` value after accessing
/// `b`.
fn compile_access_preserve_this(&mut self, access: &PropertyAccess) -> JsResult<()> {
match access {
PropertyAccess::Simple(access) => {
self.compile_expr(access.target(), true)?;
self.emit_opcode(Opcode::Dup);
match access.field() {
PropertyAccessField::Const(field) => {
let index = self.get_or_insert_name((*field).into());
self.emit(Opcode::GetPropertyByName, &[index]);
}
PropertyAccessField::Expr(field) => {
self.compile_expr(field, true)?;
self.emit_opcode(Opcode::Swap);
self.emit_opcode(Opcode::GetPropertyByValue);
}
}
}
PropertyAccess::Private(access) => {
self.compile_expr(access.target(), true)?;
self.emit_opcode(Opcode::Dup);
let index = self.get_or_insert_name(access.field().into());
self.emit(Opcode::GetPrivateField, &[index]);
}
PropertyAccess::Super(access) => {
self.emit_opcode(Opcode::This);
self.emit_opcode(Opcode::Super);
match access.field() {
PropertyAccessField::Const(field) => {
let index = self.get_or_insert_name((*field).into());
self.emit(Opcode::GetPropertyByName, &[index]);
}
PropertyAccessField::Expr(expr) => {
self.compile_expr(expr, true)?;
self.emit_opcode(Opcode::Swap);
self.emit_opcode(Opcode::GetPropertyByValue);
}
}
}
}
Ok(())
}

/// Compile an optional chain expression, prepending `this` to the property value in the stack.
///
/// This compiles the access in a way that the state of the stack after executing the optional
/// chain becomes `...rest, this, value`. where `...rest` is the rest of the stack, `this` is the
/// `this` value of the chain, and `value` is the result of the chain.
///
/// This is mostly useful for inner optional chains with external calls (`(a?.b)()`), because the
/// external call is not in the optional chain, and compiling an optional chain in the usual way
/// would only return the result of the chain without preserving the `this` value. In other words,
/// `this` would be set to `undefined` for that call, which is incorrect since `a` should be the
/// `this` value of the call.
fn compile_optional_preserve_this(&mut self, optional: &Optional) -> JsResult<()> {
let mut jumps = Vec::with_capacity(optional.chain().len());

match optional.target() {
Expression::PropertyAccess(access) => {
self.compile_access_preserve_this(access)?;
}
Expression::Optional(opt) => self.compile_optional_preserve_this(opt)?,
expr => {
self.emit(Opcode::PushUndefined, &[]);
self.compile_expr(expr, true)?;
}
}
jumps.push(self.jump_if_null_or_undefined());

let (first, rest) = optional
.chain()
.split_first()
.expect("chain must have at least one element");
assert!(first.shorted());

self.compile_optional_item_kind(first.kind())?;

for item in rest {
if item.shorted() {
jumps.push(self.jump_if_null_or_undefined());
}
self.compile_optional_item_kind(item.kind())?;
}
let skip_undef = self.jump();

for label in jumps {
self.patch_jump(label);
}

self.emit_opcode(Opcode::Pop);
self.emit_opcode(Opcode::PushUndefined);

self.patch_jump(skip_undef);

Ok(())
}

/// Compile a single operation in an optional chain.
///
/// On successful compilation, the state of the stack on execution will become `...rest, this, value`,
/// where `this` is the target of the property access (`undefined` on calls), and `value` is the
/// result of executing the action.
/// For example, in the expression `a?.b.c()`, after compiling and executing:
///
/// - `a?.b`, the state of the stack will become `...rest, a, b`.
/// - `b.c`, the state of the stack will become `...rest, b, c`.
/// - `c()`, the state of the stack will become `...rest, undefined, c()`.
///
/// # Requirements
/// - This should only be called after verifying that the previous value of the chain
/// is not null or undefined (if the operator `?.` was used).
/// - This assumes that the state of the stack before compiling is `...rest, this, value`,
/// since the operation compiled by this function could be a call.
fn compile_optional_item_kind(&mut self, kind: &OptionalOperationKind) -> JsResult<()> {
match kind {
OptionalOperationKind::SimplePropertyAccess { field } => {
self.emit_opcode(Opcode::Dup);
match field {
PropertyAccessField::Const(name) => {
let index = self.get_or_insert_name((*name).into());
self.emit(Opcode::GetPropertyByName, &[index]);
}
PropertyAccessField::Expr(expr) => {
self.compile_expr(expr, true)?;
self.emit(Opcode::Swap, &[]);
self.emit(Opcode::GetPropertyByValue, &[]);
}
}
self.emit_opcode(Opcode::RotateLeft);
self.emit_u8(3);
self.emit_opcode(Opcode::Pop);
}
OptionalOperationKind::PrivatePropertyAccess { field } => {
self.emit_opcode(Opcode::Dup);
let index = self.get_or_insert_name((*field).into());
self.emit(Opcode::GetPrivateField, &[index]);
self.emit_opcode(Opcode::RotateLeft);
self.emit_u8(3);
self.emit_opcode(Opcode::Pop);
}
OptionalOperationKind::Call { args } => {
let args = &**args;
let contains_spread = args.iter().any(|arg| matches!(arg, Expression::Spread(_)));

if contains_spread {
self.emit_opcode(Opcode::PushNewArray);
for arg in args {
self.compile_expr(arg, true)?;
if let Expression::Spread(_) = arg {
self.emit_opcode(Opcode::InitIterator);
self.emit_opcode(Opcode::PushIteratorToArray);
} else {
self.emit_opcode(Opcode::PushValueToArray);
}
}
self.emit_opcode(Opcode::CallSpread);
} else {
for arg in args {
self.compile_expr(arg, true)?;
}
self.emit(Opcode::Call, &[args.len() as u32]);
}

self.emit_opcode(Opcode::PushUndefined);
self.emit_opcode(Opcode::Swap);
}
}
Ok(())
}

pub fn compile_var_decl(&mut self, decl: &VarDeclaration) -> JsResult<()> {
for variable in decl.0.as_ref() {
match variable.binding() {
Expand Down Expand Up @@ -2292,7 +2491,6 @@ impl<'b> ByteCompiler<'b> {
function.is_async(),
function.is_arrow(),
);

let FunctionSpec {
name,
parameters,
Expand Down Expand Up @@ -2357,50 +2555,13 @@ impl<'b> ByteCompiler<'b> {
};

match call.function() {
Expression::PropertyAccess(access) => match access {
PropertyAccess::Simple(access) => {
self.compile_expr(access.target(), true)?;
if kind == CallKind::Call {
self.emit(Opcode::Dup, &[]);
}
match access.field() {
PropertyAccessField::Const(field) => {
let index = self.get_or_insert_name((*field).into());
self.emit(Opcode::GetPropertyByName, &[index]);
}
PropertyAccessField::Expr(field) => {
self.compile_expr(field, true)?;
self.emit(Opcode::Swap, &[]);
self.emit(Opcode::GetPropertyByValue, &[]);
}
}
}
PropertyAccess::Private(access) => {
self.compile_expr(access.target(), true)?;
if kind == CallKind::Call {
self.emit(Opcode::Dup, &[]);
}
let index = self.get_or_insert_name(access.field().into());
self.emit(Opcode::GetPrivateField, &[index]);
}
PropertyAccess::Super(access) => {
if kind == CallKind::Call {
self.emit_opcode(Opcode::This);
}
self.emit_opcode(Opcode::Super);
match access.field() {
PropertyAccessField::Const(field) => {
let index = self.get_or_insert_name((*field).into());
self.emit(Opcode::GetPropertyByName, &[index]);
}
PropertyAccessField::Expr(expr) => {
self.compile_expr(expr, true)?;
self.emit_opcode(Opcode::Swap);
self.emit_opcode(Opcode::GetPropertyByValue);
}
}
}
},
Expression::PropertyAccess(access) if kind == CallKind::Call => {
self.compile_access_preserve_this(access)?;
}

Expression::Optional(opt) if kind == CallKind::Call => {
self.compile_optional_preserve_this(opt)?;
}
expr => {
self.compile_expr(expr, true)?;
if kind == CallKind::Call || kind == CallKind::CallEval {
Expand Down Expand Up @@ -2958,6 +3119,7 @@ impl<'b> ByteCompiler<'b> {
self.emit_opcode(Opcode::SetClassPrototype);
self.emit_opcode(Opcode::Swap);

// TODO: set function name for getter and setters
for element in class.elements() {
match element {
ClassElement::StaticMethodDefinition(name, method_definition) => {
Expand Down Expand Up @@ -3049,6 +3211,7 @@ impl<'b> ByteCompiler<'b> {
},
}
}
// TODO: set names for private methods
ClassElement::PrivateStaticMethodDefinition(name, method_definition) => {
self.emit_opcode(Opcode::Dup);
match method_definition {
Expand Down Expand Up @@ -3218,6 +3381,7 @@ impl<'b> ByteCompiler<'b> {
self.emit(Opcode::Call, &[0]);
self.emit_opcode(Opcode::Pop);
}
// TODO: set names for private methods
ClassElement::PrivateMethodDefinition(name, method_definition) => {
self.emit_opcode(Opcode::Dup);
match method_definition {
Expand Down Expand Up @@ -3263,6 +3427,7 @@ impl<'b> ByteCompiler<'b> {
match element {
ClassElement::MethodDefinition(name, method_definition) => {
self.emit_opcode(Opcode::Dup);
// TODO: set names for getters and setters
match method_definition {
MethodDefinition::Get(expr) => match name {
PropertyName::Literal(name) => {
Expand Down
10 changes: 8 additions & 2 deletions boa_engine/src/syntax/ast/expression/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,15 @@ mod r#await;
mod call;
mod identifier;
mod new;
mod optional;
mod spread;
mod tagged_template;
mod r#yield;

pub use call::{Call, SuperCall};
pub use identifier::Identifier;
pub use new::New;
pub use optional::{Optional, OptionalOperation, OptionalOperationKind};
pub use r#await::Await;
pub use r#yield::Yield;
pub use spread::Spread;
Expand Down Expand Up @@ -110,10 +112,11 @@ pub enum Expression {
/// See [`Call`].
Call(Call),

/// See [`SuperCall`]
/// See [`SuperCall`].
SuperCall(SuperCall),

// TODO: Optional chains
/// See [`Optional`].
Optional(Optional),

// TODO: Import calls
/// See [`TaggedTemplate`].
Expand Down Expand Up @@ -173,6 +176,7 @@ impl Expression {
Self::New(new) => new.to_interned_string(interner),
Self::Call(call) => call.to_interned_string(interner),
Self::SuperCall(supc) => supc.to_interned_string(interner),
Self::Optional(opt) => opt.to_interned_string(interner),
Self::NewTarget => "new.target".to_owned(),
Self::TaggedTemplate(tag) => tag.to_interned_string(interner),
Self::Assign(assign) => assign.to_interned_string(interner),
Expand Down Expand Up @@ -213,6 +217,7 @@ impl Expression {
Expression::New(new) => new.contains_arguments(),
Expression::Call(call) => call.contains_arguments(),
Expression::SuperCall(call) => call.contains_arguments(),
Expression::Optional(opt) => opt.contains_arguments(),
Expression::TaggedTemplate(tag) => tag.contains_arguments(),
Expression::Assign(assign) => assign.contains_arguments(),
Expression::Unary(unary) => unary.contains_arguments(),
Expand Down Expand Up @@ -252,6 +257,7 @@ impl Expression {
Expression::Call(call) => call.contains(symbol),
Expression::SuperCall(_) if symbol == ContainsSymbol::SuperCall => true,
Expression::SuperCall(expr) => expr.contains(symbol),
Expression::Optional(opt) => opt.contains(symbol),
Expression::TaggedTemplate(temp) => temp.contains(symbol),
Expression::NewTarget => symbol == ContainsSymbol::NewTarget,
Expression::Assign(assign) => assign.contains(symbol),
Expand Down
Loading

0 comments on commit 4b892a9

Please sign in to comment.