fix #2730 ompound assignment with post increment in lhs miscompiles

src/ast.ts

@@ -1292,6 +1292,7 @@ export class ConstructorExpression extends IdentifierExpression {
 
 /** Represents an element access expression, e.g., array access. */
 export class ElementAccessExpression extends Expression {
+  private elementExpressionCompiledCache:Map<string, Expression>;
   constructor(
     /** Expression being accessed. */
     public expression: Expression,
@@ -1301,6 +1302,23 @@ export class ElementAccessExpression extends Expression {
     range: Range
   ) {
     super(NodeKind.ElementAccess, range);
+    this.elementExpressionCompiledCache = new Map<string, Expression>();
+  }
+
+  public addCached(key: string, value:Expression): void {
+    this.elementExpressionCompiledCache.set(key, value);
+  }
+
+  public getCache(key:string): Expression | undefined {
+    return this.elementExpressionCompiledCache.get(key);
+  }
+
+  public getElementExpression(key: string): Expression {
+    let cachedExpression = this.getCache(key);
+
+    let elementExpression: Expression = cachedExpression ?? this.elementExpression;
+
+    return elementExpression;
   }
 }
 

src/compiler.ts

@@ -220,6 +220,7 @@ import {
   liftRequiresExportRuntime,
   lowerRequiresExportRuntime
 } from "./bindings/js";
+import { _BinaryenCallGetOperandAt, _BinaryenCallSetOperandAt } from "./glue/binaryen";
 
 /** Features enabled by default. */
 export const defaultFeatures = Feature.MutableGlobals
@@ -4716,6 +4717,11 @@ export class Compiler extends DiagnosticEmitter {
       }
     }
     if (!compound) return expr;
+
+    let exprTempLocal = this.currentFlow.getTempLocal(this.currentType);
+    let calculationResRef = module.local_set(exprTempLocal.index, expr, false);
+
+    let valueExprRef = module.local_get(exprTempLocal.index, this.currentType.toRef());
     let resolver = this.resolver;
     let target = resolver.lookupExpression(left, this.currentFlow);
     if (!target) return module.unreachable();
@@ -4728,15 +4734,18 @@ export class Compiler extends DiagnosticEmitter {
       );
       return module.unreachable();
     }
-    return this.makeAssignment(
+
+    let assignmentExprRef = this.makeAssignment(
       target,
-      expr,
+      valueExprRef,
       this.currentType,
       right,
       resolver.currentThisExpression,
       resolver.currentElementExpression,
       contextualType != Type.void
     );
+
+    return module.block(null, [calculationResRef, assignmentExprRef], this.currentType.toRef());
   }
 
   makeLt(leftExpr: ExpressionRef, rightExpr: ExpressionRef, type: Type): ExpressionRef {
@@ -6295,7 +6304,7 @@ export class Compiler extends DiagnosticEmitter {
     }
 
     // Inline if explicitly requested
-    if (instance.hasDecorator(DecoratorFlags.Inline) && (!instance.is(CommonFlags.Overridden) || reportNode.isAccessOnSuper)) {
+    if (instance.canInline() && (!instance.is(CommonFlags.Overridden) || reportNode.isAccessOnSuper)) {
       assert(!instance.is(CommonFlags.Stub)); // doesn't make sense
       let inlineStack = this.inlineStack;
       if (inlineStack.includes(instance)) {
@@ -6757,7 +6766,7 @@ export class Compiler extends DiagnosticEmitter {
     reportNode: Node,
     immediatelyDropped: bool = false
   ): ExpressionRef {
-    if (instance.hasDecorator(DecoratorFlags.Inline)) {
+    if (instance.canInline()) {
       if (!instance.is(CommonFlags.Overridden)) {
         assert(!instance.is(CommonFlags.Stub)); // doesn't make sense
         let inlineStack = this.inlineStack;
@@ -7022,9 +7031,27 @@ export class Compiler extends DiagnosticEmitter {
               expression.range
             );
           }
-          return this.compileCallDirect(indexedGet, [
-            expression.elementExpression
+
+          let indexLocal = this.currentFlow.getTempLocal(Type.i32);
+          let indexExpression = expression.getElementExpression(this.currentFlow.targetFunction.internalName);
+
+          indexedGet.setNoInline();
+
+          let callExprRef = this.compileCallDirect(indexedGet, [
+            indexExpression
           ], expression, thisArg, constraints);
+
+          if(indexExpression === expression.elementExpression){
+            let indexType = indexedGet.signature.parameterTypes[0];
+            let indexExpressionRef = _BinaryenCallGetOperandAt(callExprRef, 1);
+            let teeExprRef = module.local_tee(indexLocal.index, indexExpressionRef, false, TypeRef.I32);
+            _BinaryenCallSetOperandAt(callExprRef, 1, teeExprRef);
+            let indexGetExpr = module.local_get(indexLocal.index, TypeRef.I32);
+
+            expression.addCached(this.currentFlow.targetFunction.internalName, Node.createCompiledExpression(indexGetExpr, indexType, expression.elementExpression.range));
+          }
+
+          return callExprRef;
         }
       }
       this.error(
@@ -8727,7 +8754,7 @@ export class Compiler extends DiagnosticEmitter {
     if (!classInstance) return module.unreachable();
     if (contextualType == Type.void) constraints |= Constraints.WillDrop;
     let ctor = this.ensureConstructor(classInstance, expression);
-    if (!ctor.hasDecorator(DecoratorFlags.Inline)) {
+    if (!ctor.canInline()) {
       // Inlined ctors haven't been compiled yet and are checked upon inline
       // compilation of their body instead.
       this.checkFieldInitialization(classInstance, expression);
@@ -8747,7 +8774,7 @@ export class Compiler extends DiagnosticEmitter {
       // shortcut if already compiled
       if (instance.is(CommonFlags.Compiled)) return instance;
       // do not attempt to compile if inlined anyway
-      if (!instance.hasDecorator(DecoratorFlags.Inline)) this.compileFunction(instance);
+      if (!instance.canInline()) this.compileFunction(instance);
     } else {
       // clone base constructor if a derived class. note that we cannot just
       // call the base ctor since the derived class may have additional fields.
@@ -9355,37 +9382,31 @@ export class Compiler extends DiagnosticEmitter {
       return module.unreachable();
     }
 
-    // simplify if dropped anyway
-    if (!tempLocal) {
-      return this.makeAssignment(
-        target,
-        expr,
-        this.currentType,
-        expression.operand,
-        resolver.currentThisExpression,
-        resolver.currentElementExpression,
-        false
-      );
-    }
+    let exprTempLocal = this.currentFlow.getTempLocal(this.currentType);
+    let calculationResRef = module.local_set(exprTempLocal.index, expr, false);
+
+    let valueExprRef = module.local_get(exprTempLocal.index, this.currentType.toRef());
 
-    // otherwise use the temp. local for the intermediate value (always possibly overflows)
     let setValue = this.makeAssignment(
       target,
-      expr, // includes a tee of getValue to tempLocal
+      valueExprRef, // includes a tee of getValue to tempLocal
       this.currentType,
       expression.operand,
       resolver.currentThisExpression,
       resolver.currentElementExpression,
       false
     );
 
-    this.currentType = tempLocal.type;
-    let typeRef = tempLocal.type.toRef();
+    let blockExpressions = [calculationResRef, setValue,];
 
-    return module.block(null, [
-      setValue,
-      module.local_get(tempLocal.index, typeRef)
-    ], typeRef); // result of 'x++' / 'x--' might overflow
+    // otherwise use the temp. local for the intermediate value (always possibly overflows)
+    if(tempLocal){
+      this.currentType = tempLocal.type;
+      let typeRef = tempLocal.type.toRef();
+      blockExpressions.push(module.local_get(tempLocal.index, typeRef));
+    }
+
+    return module.block(null, blockExpressions, this.currentType.toRef()); // result of 'x++' / 'x--' might overflow
   }
 
   private compileUnaryPrefixExpression(

src/program.ts

@@ -3766,6 +3766,8 @@ export class Function extends TypedElement {
   memorySegment: MemorySegment | null = null;
   /** Original function, if a stub. Otherwise `this`. */
   original!: Function;
+  /** force the function to be compiled as no inline */
+  noInline = false;
 
   /** Counting id of inline operations involving this function. */
   nextInlineId: i32 = 0;
@@ -3949,6 +3951,14 @@ export class Function extends TypedElement {
       }
     }
   }
+
+  setNoInline(): void{
+    this.noInline = true;
+  }
+
+  canInline(): bool{
+    return this.hasDecorator(DecoratorFlags.Inline) && (!this.noInline);
+  }
 }
 
 /** A property comprised of a getter and a setter function. */

src/resolver.ts

@@ -1553,7 +1553,7 @@ export class Resolver extends DiagnosticEmitter {
         let indexSignature = classReference.indexSignature;
         if (indexSignature) {
           this.currentThisExpression = targetExpression;
-          this.currentElementExpression = node.elementExpression;
+          this.currentElementExpression = node.getElementExpression(ctxFlow.targetFunction.internalName);
           return indexSignature;
         }
         classReference = classReference.base;

tests/compiler/NonNullable.debug.wat

@@ -42,8 +42,14 @@
   (local $ptr1 i32)
   (local $ptr2 i32)
   (local $7 i32)
+  (local $8 i32)
+  (local $9 i32)
+  (local $10 i32)
+  (local $11 i32)
   (local $a i32)
   (local $b i32)
+  (local $14 i32)
+  (local $15 i32)
   local.get $str1
   local.get $index1
   i32.const 1
@@ -89,14 +95,20 @@
      local.get $ptr1
      i32.const 8
      i32.add
+     local.set $7
+     local.get $7
      local.set $ptr1
      local.get $ptr2
      i32.const 8
      i32.add
+     local.set $8
+     local.get $8
      local.set $ptr2
      local.get $len
      i32.const 4
      i32.sub
+     local.set $9
+     local.get $9
      local.set $len
      local.get $len
      i32.const 4
@@ -107,11 +119,13 @@
   end
   loop $while-continue|1
    local.get $len
-   local.tee $7
+   local.tee $10
    i32.const 1
    i32.sub
+   local.set $11
+   local.get $11
    local.set $len
-   local.get $7
+   local.get $10
    if
     local.get $ptr1
     i32.load16_u $0
@@ -131,10 +145,14 @@
     local.get $ptr1
     i32.const 2
     i32.add
+    local.set $14
+    local.get $14
     local.set $ptr1
     local.get $ptr2
     i32.const 2
     i32.add
+    local.set $15
+    local.get $15
     local.set $ptr2
     br $while-continue|1
    end