Add more expression test cases, fix bugs

Also added a few more comments + reorganized exceptions that are
invoke-internal.

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/BreakException.java → worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/invoke/BreakException.java

@@ -17,13 +17,13 @@
  * along with this program. If not, see <http://www.gnu.org/licenses/>.
  */
 
-package com.sk89q.worldedit.internal.expression;
+package com.sk89q.worldedit.internal.expression.invoke;
 
 /**
  * Thrown when a break or continue is encountered.
  * Loop constructs catch this exception.
  */
-public class BreakException extends RuntimeException {
+class BreakException extends RuntimeException {
 
     public static final BreakException BREAK = new BreakException(false);
     public static final BreakException CONTINUE = new BreakException(true);

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/invoke/CompilingVisitor.java

@@ -21,7 +21,6 @@
 
 import com.sk89q.worldedit.antlr.ExpressionBaseVisitor;
 import com.sk89q.worldedit.antlr.ExpressionParser;
-import com.sk89q.worldedit.internal.expression.BreakException;
 import com.sk89q.worldedit.internal.expression.EvaluationException;
 import com.sk89q.worldedit.internal.expression.ExecutionData;
 import com.sk89q.worldedit.internal.expression.ExpressionHelper;
@@ -217,12 +216,31 @@ public MethodHandle visitContinueStatement(ExpressionParser.ContinueStatementCon
         return CONTINUE_STATEMENT;
     }
 
+    // MH = (Double)Double
+    // takes the double to return, conveniently has Double return type
+    private static final MethodHandle RETURN_STATEMENT_BASE = MethodHandles.filterReturnValue(
+        // take the (Double)ReturnException constructor
+        ExpressionHandles.NEW_RETURN_EXCEPTION,
+        // and map the return type to Double by throwing it
+        MethodHandles.throwException(Double.class, ReturnException.class)
+    );
+
     @Override
     public MethodHandle visitReturnStatement(ExpressionParser.ReturnStatementContext ctx) {
+        // MH:returnValue = (ExecutionData)Double
+        MethodHandle returnValue;
         if (ctx.value != null) {
-            return evaluate(ctx.value).handle;
+            returnValue = evaluate(ctx.value).handle;
+        } else {
+            returnValue = defaultResult();
         }
-        return defaultResult();
+        return MethodHandles.filterArguments(
+            // take the (Double)Double return statement
+            RETURN_STATEMENT_BASE,
+            0,
+            // map the Double back to ExecutionData via the returnValue
+            returnValue
+        );
     }
 
     @Override
@@ -450,7 +468,7 @@ public MethodHandle visitEqualityExpr(ExpressionParser.EqualityExprContext ctx)
                 case NOT_EQUAL:
                     return (l, r) -> ExpressionHandles.boolToDouble(l != r);
                 case NEAR:
-                    return (l, r) -> ExpressionHandles.boolToDouble(almostEqual2sComplement(l, r, 450359963L));
+                    return (l, r) -> ExpressionHandles.boolToDouble(almostEqual2sComplement(l, r));
                 case GREATER_THAN_OR_EQUAL:
                     return (l, r) -> ExpressionHandles.boolToDouble(l >= r);
             }
@@ -459,7 +477,7 @@ public MethodHandle visitEqualityExpr(ExpressionParser.EqualityExprContext ctx)
     }
 
     // Usable AlmostEqual function, based on http://www.cygnus-software.com/papers/comparingfloats/comparingfloats.htm
-    private static boolean almostEqual2sComplement(double a, double b, long maxUlps) {
+    private static boolean almostEqual2sComplement(double a, double b) {
         // Make sure maxUlps is non-negative and small enough that the
         // default NAN won't compare as equal to anything.
         //assert(maxUlps > 0 && maxUlps < 4 * 1024 * 1024); // this is for floats, not doubles
@@ -473,7 +491,7 @@ private static boolean almostEqual2sComplement(double a, double b, long maxUlps)
         if (bLong < 0) bLong = 0x8000000000000000L - bLong;
 
         final long longDiff = Math.abs(aLong - bLong);
-        return longDiff <= maxUlps;
+        return longDiff <= 450359963L;
     }
 
     @Override
@@ -645,11 +663,7 @@ public MethodHandle visitChildren(RuleNode node) {
                 checkHandle(childResult, (ParserRuleContext) c);
             }
 
-            boolean returning = c instanceof ExpressionParser.ReturnStatementContext;
-            result = aggregateResult(result, childResult, returning);
-            if (returning) {
-                return result;
-            }
+            result = aggregateHandleResult(result, childResult);
         }
 
         return result;
@@ -660,25 +674,26 @@ protected MethodHandle aggregateResult(MethodHandle aggregate, MethodHandle next
         throw new UnsupportedOperationException();
     }
 
-    private MethodHandle aggregateResult(MethodHandle oldResult, MethodHandle result,
-                                         boolean keepDefault) {
+    private MethodHandle aggregateHandleResult(MethodHandle oldResult, MethodHandle result) {
+        // MH:oldResult,result = (ExecutionData)Double
+
         // Execute `oldResult` but ignore its return value, then execute result and return that.
         // If `oldResult` (the old value) is `defaultResult`, it's bogus, so just skip it
         if (oldResult == DEFAULT_RESULT) {
             return result;
         }
-        if (result == DEFAULT_RESULT && !keepDefault) {
-            return oldResult;
-        }
         // Add a dummy Double parameter to the end
+        // MH:dummyDouble = (ExecutionData, Double)Double
         MethodHandle dummyDouble = MethodHandles.dropArguments(
             result, 1, Double.class
         );
         // Have oldResult turn it from data->Double
+        // MH:doubledData = (ExecutionData, ExecutionData)Double
         MethodHandle doubledData = MethodHandles.collectArguments(
             dummyDouble, 1, oldResult
         );
         // Deduplicate the `data` parameter
+        // MH:@return = (ExecutionData)Double
         return ExpressionHandles.dedupData(doubledData);
     }
 }

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/invoke/ExpressionCompiler.java

@@ -66,8 +66,15 @@ public class ExpressionCompiler {
     public CompiledExpression compileExpression(ExpressionParser.AllStatementsContext root,
                                                 Functions functions) {
         MethodHandle invokable = root.accept(new CompilingVisitor(functions));
+        // catch ReturnExpression and substitute its result
+        invokable = MethodHandles.catchException(
+            invokable,
+            ReturnException.class,
+            ExpressionHandles.RETURN_EXCEPTION_GET_RESULT
+        );
+        MethodHandle finalInvokable = invokable;
         return (CompiledExpression) ExpressionHandles.safeInvoke(
-            HANDLE_TO_CE_CONVERTER, h -> h.invoke(invokable)
+            HANDLE_TO_CE_CONVERTER, h -> h.invoke(finalInvokable)
         );
     }
 }

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/invoke/ExpressionHandles.java

@@ -20,7 +20,6 @@
 package com.sk89q.worldedit.internal.expression.invoke;
 
 import com.google.common.base.Throwables;
-import com.sk89q.worldedit.internal.expression.BreakException;
 import com.sk89q.worldedit.internal.expression.CompiledExpression;
 import com.sk89q.worldedit.internal.expression.EvaluationException;
 import com.sk89q.worldedit.internal.expression.ExecutionData;
@@ -70,6 +69,10 @@ class ExpressionHandles {
     // (double, double)Double;
     static final MethodHandle CALL_BINARY_OP;
     static final MethodHandle NEW_LS_CONSTANT;
+    // (Double)ReturnException;
+    static final MethodHandle NEW_RETURN_EXCEPTION;
+    // (ReturnException)Double;
+    static final MethodHandle RETURN_EXCEPTION_GET_RESULT;
 
     static final MethodHandle NULL_DOUBLE = dropData(constant(Double.class, null));
 
@@ -105,6 +108,10 @@ class ExpressionHandles {
                 .asType(methodType(Double.class, DoubleBinaryOperator.class, double.class, double.class));
             NEW_LS_CONSTANT = lookup.findConstructor(LocalSlot.Constant.class,
                 methodType(void.class, double.class));
+            NEW_RETURN_EXCEPTION = lookup.findConstructor(ReturnException.class,
+                methodType(void.class, Double.class));
+            RETURN_EXCEPTION_GET_RESULT = lookup.findVirtual(ReturnException.class,
+                "getResult", methodType(Double.class));
         } catch (NoSuchMethodException | IllegalAccessException e) {
             throw new IllegalStateException(e);
         }

worldedit-core/src/main/java/com/sk89q/worldedit/internal/expression/invoke/ReturnException.java

@@ -0,0 +1,39 @@
+/*
+ * WorldEdit, a Minecraft world manipulation toolkit
+ * Copyright (C) sk89q <http://www.sk89q.com>
+ * Copyright (C) WorldEdit team and contributors
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU Lesser General Public License as published by the
+ * Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+package com.sk89q.worldedit.internal.expression.invoke;
+
+/**
+ * Thrown when a return is encountered, to pop the stack frames and return the value easily.
+ *
+ * Should be caught by the executor.
+ */
+class ReturnException extends RuntimeException {
+
+    private final Double result;
+
+    public ReturnException(Double result) {
+        super("return " + result, null, true, false);
+        this.result = result;
+    }
+
+    public Double getResult() {
+        return result;
+    }
+}

worldedit-core/src/test/java/com/sk89q/worldedit/internal/expression/ExpressionTest.java

@@ -55,7 +55,17 @@ public Stream<DynamicNode> testEvaluate() throws ExpressionException {
             testCase("0 || 5", 5),
             testCase("2 || 5", 2),
             testCase("2 && 5", 5),
-            testCase("5 && 0", 0)
+            testCase("5 && 0", 0),
+            // check ternaries
+            testCase("false ? 1 : 2", 2),
+            testCase("true ? 1 : 2", 1),
+            // - ternary binds inside
+            testCase("true ? true ? 1 : 2 : 3", 1),
+            testCase("true ? false ? 1 : 2 : 3", 2),
+            testCase("false ? true ? 1 : 2 : 3", 3),
+            testCase("false ? false ? 1 : 2 : 3", 3),
+            // check return
+            testCase("return 1; 0", 1)
         );
         return testCases.stream()
             .map(testCase -> DynamicTest.dynamicTest(