Fix update expressions getting values multiple times (#2733)

Currently update expressions get values multiple times. This can lead to multiple executions of object property getters.

This is very similar to #2551. Unfortunateley it seems like we have to sacrifice some code duplication for correctness in these cases. But that is probably for the best, as we can generate more optimized bytecode for each of these `get/set` cases.

boa_engine/src/bytecompiler/expression/update.rs

@@ -2,41 +2,136 @@ use crate::{
     bytecompiler::{Access, ByteCompiler},
     vm::Opcode,
 };
-use boa_ast::expression::operator::{update::UpdateOp, Update};
+use boa_ast::expression::{
+    access::{PropertyAccess, PropertyAccessField},
+    operator::{update::UpdateOp, Update},
+};
 
 impl ByteCompiler<'_, '_> {
     pub(crate) fn compile_update(&mut self, update: &Update, use_expr: bool) {
-        let access = Access::from_update_target(update.target());
-
-        match update.op() {
-            UpdateOp::IncrementPre => {
-                self.access_set(access, true, |compiler, _| {
-                    compiler.access_get(access, true);
-                    compiler.emit_opcode(Opcode::Inc);
-                });
-            }
-            UpdateOp::DecrementPre => {
-                self.access_set(access, true, |compiler, _| {
-                    compiler.access_get(access, true);
-                    compiler.emit_opcode(Opcode::Dec);
-                });
-            }
-            UpdateOp::IncrementPost => {
-                self.access_set(access, false, |compiler, level| {
-                    compiler.access_get(access, true);
-                    compiler.emit_opcode(Opcode::IncPost);
-                    compiler.emit_opcode(Opcode::RotateRight);
-                    compiler.emit_u8(level + 2);
-                });
-            }
-            UpdateOp::DecrementPost => {
-                self.access_set(access, false, |compiler, level| {
-                    compiler.access_get(access, true);
-                    compiler.emit_opcode(Opcode::DecPost);
-                    compiler.emit_opcode(Opcode::RotateRight);
-                    compiler.emit_u8(level + 2);
-                });
+        let opcode = match update.op() {
+            UpdateOp::IncrementPre => Opcode::Inc,
+            UpdateOp::DecrementPre => Opcode::Dec,
+            UpdateOp::IncrementPost => Opcode::IncPost,
+            UpdateOp::DecrementPost => Opcode::DecPost,
+        };
+        let post = matches!(
+            update.op(),
+            UpdateOp::IncrementPost | UpdateOp::DecrementPost
+        );
+
+        match Access::from_update_target(update.target()) {
+            Access::Variable { name } => {
+                let binding = self.context.get_binding_value(name);
+                let index = self.get_or_insert_binding(binding);
+                self.emit(Opcode::GetName, &[index]);
+
+                self.emit_opcode(opcode);
+                if post {
+                    self.emit_opcode(Opcode::Swap);
+                } else {
+                    self.emit_opcode(Opcode::Dup);
+                }
+
+                let binding = self.context.set_mutable_binding(name);
+                let index = self.get_or_insert_binding(binding);
+                self.emit(Opcode::SetName, &[index]);
+            }
+            Access::Property { access } => match access {
+                PropertyAccess::Simple(access) => match access.field() {
+                    PropertyAccessField::Const(name) => {
+                        let index = self.get_or_insert_name((*name).into());
+                        self.compile_expr(access.target(), true);
+                        self.emit_opcode(Opcode::Dup);
+                        self.emit_opcode(Opcode::Dup);
+
+                        self.emit(Opcode::GetPropertyByName, &[index]);
+                        self.emit_opcode(opcode);
+                        if post {
+                            self.emit_opcode(Opcode::RotateRight);
+                            self.emit_u8(4);
+                        }
+
+                        self.emit(Opcode::SetPropertyByName, &[index]);
+                        if post {
+                            self.emit_opcode(Opcode::Pop);
+                        }
+                    }
+                    PropertyAccessField::Expr(expr) => {
+                        self.compile_expr(access.target(), true);
+                        self.emit_opcode(Opcode::Dup);
+                        self.compile_expr(expr, true);
+
+                        self.emit_opcode(Opcode::GetPropertyByValuePush);
+                        self.emit_opcode(opcode);
+                        if post {
+                            self.emit_opcode(Opcode::RotateRight);
+                            self.emit_u8(4);
+                        }
+
+                        self.emit_opcode(Opcode::SetPropertyByValue);
+                        if post {
+                            self.emit_opcode(Opcode::Pop);
+                        }
+                    }
+                },
+                PropertyAccess::Private(access) => {
+                    let index = self.get_or_insert_private_name(access.field());
+                    self.compile_expr(access.target(), true);
+                    self.emit_opcode(Opcode::Dup);
+
+                    self.emit(Opcode::GetPrivateField, &[index]);
+                    self.emit_opcode(opcode);
+                    if post {
+                        self.emit_opcode(Opcode::RotateRight);
+                        self.emit_u8(3);
+                    }
+
+                    self.emit(Opcode::SetPrivateField, &[index]);
+                    if post {
+                        self.emit_opcode(Opcode::Pop);
+                    }
+                }
+                PropertyAccess::Super(access) => match access.field() {
+                    PropertyAccessField::Const(name) => {
+                        let index = self.get_or_insert_name((*name).into());
+                        self.emit_opcode(Opcode::Super);
+                        self.emit_opcode(Opcode::Dup);
+                        self.emit_opcode(Opcode::This);
+                        self.emit_opcode(Opcode::Swap);
+
+                        self.emit(Opcode::GetPropertyByName, &[index]);
+                        self.emit_opcode(opcode);
+                        if post {
+                            self.emit_opcode(Opcode::RotateRight);
+                            self.emit_u8(3);
+                        }
+
+                        self.emit(Opcode::SetPropertyByName, &[index]);
+                        if post {
+                            self.emit_opcode(Opcode::Pop);
+                        }
+                    }
+                    PropertyAccessField::Expr(expr) => {
+                        self.emit_opcode(Opcode::Super);
+                        self.emit_opcode(Opcode::Dup);
+                        self.compile_expr(expr, true);
+
+                        self.emit_opcode(Opcode::GetPropertyByValuePush);
+                        self.emit_opcode(opcode);
+                        if post {
+                            self.emit_opcode(Opcode::RotateRight);
+                            self.emit_u8(2);
+                        }
+
+                        self.emit_opcode(Opcode::SetPropertyByValue);
+                        if post {
+                            self.emit_opcode(Opcode::Pop);
+                        }
                     }
+                },
+            },
+            Access::This => unreachable!(),
         }
 
         if !use_expr {