More precise conflict resolution of overloaded functions

CHANGELOG.markdown

@@ -31,10 +31,10 @@ All notable changes to this project will be documented in this file.
 
 ### Changed
 
-* **Breaking:** Changed the syntax of properties to use the `@` prefix for Mindustry built-in variables (e.g. `vault1.@coal`). 
-* **Breaking:** Changed Mindustry Logic functions to require the `out` modifier when passing in an output argument, in the same way as the user-defined functions. For backward compatibility, omitting the `out` modifier from a Mindustry Logic function calls generates only a warning, not an error.
-* Changed Loop Unrolling to replace output iterator variables with the variable assigned to them.
-* Changed Single Step Eliminator to remove a jump is if there is an identical jump preceding it and there are no other jumps or labels between them. Active on `experimental`.
+* **Deprecation:** Changed the syntax of properties to use the `@` prefix for Mindustry built-in variables (e.g. `vault1.@coal`). Omitting the `@` prefix from a property name generates a deprecation warning. 
+* **Deprecation:** Changed Mindustry Logic functions to require the `out` modifier when passing in an output argument, in the same way as the user-defined functions. Omitting the `out` modifier from a Mindustry Logic function calls generates a deprecation warning.
+* Changed the [Loop Unrolling optimization](doc/syntax/SYNTAX-6-OPTIMIZATIONS.markdown#loop-unrolling) to replace output iterator variables with the variable assigned to them.
+* Changed the [Single Step Eliminator](doc/syntax/SYNTAX-6-OPTIMIZATIONS.markdown#single-step-elimination) to remove a jump is if there is an identical jump preceding it and there are no other jumps or labels between them. Active on `experimental`.
 * Changed the expression evaluator to evaluate operations over known built-in values. The change enhances the Data Flow and Jump Normalization optimizations.
 * Changed the Schemacode compiler to correctly output positions of error messages generated by Mindcode compiler, taking into account both the source file and/or position of the Mindcode program or program snippet within the source file.  
 

compiler/src/main/java/info/teksol/mindcode/compiler/generator/CallGraph.java

@@ -5,7 +5,6 @@
 
 import java.util.List;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Stream;
 
@@ -56,15 +55,12 @@ public List<LogicFunction> getFunctions() {
         return functions;
     }
 
-    /**
-     * Finds the closes matching function to given function call. The closest matching function is the one
-     * giving the best match score. If a function matching the call exactly exists, it will be chosen.
-     *
-     * @param call function call to match
-     * @return function matching the given call best
-     */
-    public Optional<LogicFunction> getFunction(FunctionCall call) {
-        return functionDefinitions.getFunction(call);
+    public List<LogicFunction> getExactMatches(FunctionCall call) {
+        return functionDefinitions.getExactMatches(call);
+    }
+
+    public List<LogicFunction> getLooseMatches(FunctionCall call) {
+        return functionDefinitions.getLooseMatches(call);
     }
 
     /**

compiler/src/main/java/info/teksol/mindcode/compiler/generator/CallGraphCreator.java

@@ -111,9 +111,7 @@ void buildCallTree() {
         // Must end eventually, as there's a finite number of functions to add
         while (propagateIndirectCalls()) ;
 
-        functions.stream()
-                .filter(f -> f.isUsed() && !f.isInline())
-                .forEach(this::setupOutOfLineFunction);
+        functions.stream().filter(f -> !f.isInline()).forEach(this::setupOutOfLineFunction);
     }
 
     private void setupOutOfLineFunction(LogicFunction function) {

compiler/src/main/java/info/teksol/mindcode/compiler/generator/FunctionDefinitions.java

@@ -1,12 +1,15 @@
 package info.teksol.mindcode.compiler.generator;
 
 import info.teksol.mindcode.MindcodeMessage;
-import info.teksol.mindcode.Tuple2;
 import info.teksol.mindcode.ast.FunctionCall;
 import info.teksol.mindcode.ast.FunctionDeclaration;
+import info.teksol.mindcode.ast.FunctionParameter;
 import info.teksol.mindcode.ast.NoOp;
 
-import java.util.*;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import java.util.function.Consumer;
 
 class FunctionDefinitions extends AbstractMessageEmitter {
@@ -25,17 +28,36 @@ public LogicFunction addFunctionDeclaration(FunctionDeclaration declaration) {
         functionList.add(current);
 
         List<LogicFunction> functions = functionMap.computeIfAbsent(declaration.getName(), k -> new ArrayList<>());
-        functions.stream().filter(f -> f.getParameterCount().overlaps(current.getParameterCount()))
-                        .forEach(f -> reportClash(current, f));
+        functions.stream().filter(f -> conflicts(current, f)).forEach(f -> reportConflict(current, f));
 
         functions.add(current);
         return current;
     }
 
-    private void reportClash(LogicFunction current, LogicFunction previous) {
+    private boolean conflicts(LogicFunction f1, LogicFunction f2) {
+        if (!f1.getParameterCount().overlaps(f2.getParameterCount())) {
+            return false;
+        }
+
+        int count = Math.min(f1.getStandardParameterCount(), f2.getStandardParameterCount());
+        for (int index = 0; index < count; index++) {
+            FunctionParameter p1 = f1.getDeclaredParameter(index);
+            FunctionParameter p2 = f2.getDeclaredParameter(index);
+            if (!conflicts(p1, p2)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    private boolean conflicts(FunctionParameter p1, FunctionParameter p2) {
+        return p1.isInput() && p2.isInput() || p1.isOutput() && p2.isOutput();
+    }
+
+    private void reportConflict(LogicFunction current, LogicFunction previous) {
         String defined = previous.getInputPosition().inputFile() == current.getInputPosition().inputFile()
                 ? "" :  " defined in " + previous.getInputPosition().inputFile().fileName();
-        error(current.getInputPosition(), "Function '%s' clashes with function '%s'%s.",
+        error(current.getInputPosition(), "Function '%s' conflicts with function '%s'%s.",
                 current.format(), previous.format(), defined);
     }
 
@@ -47,21 +69,29 @@ public List<LogicFunction> getFunctions() {
         return functionList;
     }
 
+    public List<LogicFunction> getLooseMatches(FunctionCall call) {
+        return functionMap.getOrDefault(call.getFunctionName(), List.of());
+    }
+
+    public List<LogicFunction> getExactMatches(FunctionCall call) {
+        return functionMap.getOrDefault(call.getFunctionName(), List.of())
+                .stream()
+                .filter(f -> f.exactMatch(call))
+                .toList();
+    }
+
     /**
-     * Finds the closes matching function to given function call. The closest matching function is the one
-     * giving the best match score. If a function matching the call perfectly exists, it will be chosen.
+     * If there's just one exact match to this function, it is returned. Otherwise, null is returned.
      *
      * @param call function call to match
-     * @return function matching the given call best
+     * @return unique function exactly matching the given call
      */
-    public Optional<LogicFunction> getFunction(FunctionCall call) {
-        return functionMap.getOrDefault(call.getFunctionName(), List.of())
-                .stream()
-                .map(f -> Tuple2.of(f, f.matchScore(call)))
-                .max(Comparator.comparingInt(Tuple2::e2))
-                .map(Tuple2::e1);
+    public LogicFunction getExactMatch(FunctionCall call) {
+        List<LogicFunction> result = getExactMatches(call);
+        return result.size() == 1 ? result.get(0) : null;
     }
 
+
     private static FunctionDeclaration createMain() {
         return new FunctionDeclaration(null, true, false,
                 true, "", List.of(), new NoOp());

compiler/src/main/java/info/teksol/mindcode/compiler/generator/LogicFunction.java

@@ -47,7 +47,7 @@ void addCall(FunctionCall call) {
     }
 
     void initializeCalls(FunctionDefinitions functions) {
-        List<LogicFunction> calls = functionCalls.stream().map(functions::getFunction).filter(Optional::isPresent).map(Optional::get).toList();
+        List<LogicFunction> calls = functionCalls.stream().map(functions::getExactMatch).filter(Objects::nonNull).toList();
         directCalls.addAll(calls);
 
         Map<LogicFunction, Long> cardinality = calls.stream().collect(Collectors.groupingBy(f -> f, Collectors.counting()));
@@ -225,6 +225,10 @@ public int matchScore(FunctionCall call) {
     }
 
     public boolean exactMatch(FunctionCall call) {
+        if (!parameterCount.contains(call.getArguments().size())) {
+            return false;
+        }
+
         boolean isVarArgs = isVarArgs();
         int size = getStandardParameterCount();
         for (int i = 0; i < call.getArguments().size(); i++) {

compiler/src/main/java/info/teksol/mindcode/compiler/generator/LogicInstructionGenerator.java

@@ -498,26 +498,47 @@ private LogicValue handleFunctionCall(FunctionCall call) {
         setSubcontextType(AstSubcontextType.ARGUMENTS, 1.0);
         final List<LogicFunctionArgument> arguments = processArguments(call.getArguments());
 
+        LogicValue output = handleFunctionCall(call, arguments);
+
+        clearSubcontextType();
+        return output;
+    }
+
+    private LogicValue handleFunctionCall(FunctionCall call, List<LogicFunctionArgument> arguments) {
         final String functionName = call.getFunctionName();
-        final Optional<LogicFunction> function = callGraph.getFunction(call);
-        LogicValue output = null;
+        List<LogicFunction> exactMatches = callGraph.getExactMatches(call);
 
         // Try built-in function if there's not an exact match
-        if (function.isEmpty() || !function.get().exactMatch(call)) {
+        if (!exactMatches.isEmpty()) {
+            if (exactMatches.size() > 1) {
+                error(call,"Cannot resolve function '%s'.", functionName);
+                return NULL;
+            } else {
+                return handleUserFunctionCall(exactMatches.get(0), call, arguments);
+            }
+        } else {
             setSubcontextType(AstSubcontextType.SYSTEM_CALL, 1.0);
-            output = functionMapper.handleFunction(call, instructions::add, functionName, arguments);
-        }
-
-        if (output == null && function.isPresent()) {
-            output = handleUserFunctionCall(function.get(), call, arguments);
-        }
+            LogicValue output = functionMapper.handleFunction(call, instructions::add, functionName, arguments);
+            if (output != null) {
+                return output;
+            }
 
-        if (output == null) {
-            error(call, "Cannot resolve function '%s'.", functionName);
+            // We know there wasn't an exact match. Try loose matches to obtain better error messages
+            List<LogicFunction> looseMatches = callGraph.getLooseMatches(call);
+            return switch (looseMatches.size()) {
+                case 0  -> {
+                    error(call,"Unknown function '%s'.", functionName);
+                    yield NULL;
+                }
+                case 1  -> {
+                    yield handleUserFunctionCall(looseMatches.get(0), call, arguments);
+                }
+                default -> {
+                    error(call, "Cannot resolve function '%s'.", functionName);
+                    yield NULL;
+                }
+            };
         }
-
-        clearSubcontextType();
-        return output == null ? NULL : output;
     }
 
     private void validateUserFunctionArguments(LogicFunction function, List<LogicFunctionArgument> arguments) {

compiler/src/test/java/info/teksol/mindcode/compiler/generator/LogicInstructionGeneratorFunctionsTest.java

@@ -1067,7 +1067,7 @@ void handlesOverloadedFunctions() {
                         void foo(a, b) print(2); end;
                         foo();
                         foo(1);
-                        foo(1,1);
+                        foo(1, 1);
                         """,
                 createInstruction(LABEL, var(1000)),
                 createInstruction(PRINT, "0"),
@@ -1101,12 +1101,14 @@ def foo(n)
     void reportsFunctionConflicts() {
         assertGeneratesMessages(
                 ExpectedMessages.create()
-                        .add(2, 1, "Function 'foo(a, out b)' clashes with function 'foo(a)'.")
-                        .add(3, 1, "Function 'foo(a, b)' clashes with function 'foo(a, out b)'."),
+                        .add(3, 1, "Function 'foo(a, out b)' conflicts with function 'foo(a)'.")
+                        .add(4, 1, "Function 'foo(a, in out b)' conflicts with function 'foo(a, b)'.")
+                        .add(4, 1, "Function 'foo(a, in out b)' conflicts with function 'foo(a, out b)'."),
                 """
                         inline void foo(a) print(a); end;
-                        inline void foo(a, out b) print(a); b = a; end;
                         inline void foo(a, b) print(a, b); end;
+                        inline void foo(a, out b) print(a); b = a; end;
+                        inline void foo(a, in out b) print(a, b); b = a; end;
                         """
         );
     }
@@ -1115,7 +1117,7 @@ void reportsFunctionConflicts() {
     void reportsVarargFunctionConflict() {
         assertGeneratesMessages(
                 ExpectedMessages.create()
-                        .add(2, 1, "Function 'foo(a, b, c, d...)' clashes with function 'foo(a, b, c)'."),
+                        .add(2, 1, "Function 'foo(a, b, c, d...)' conflicts with function 'foo(a, b, c)'."),
                 """
                         inline void foo(a, b, c) print(a, b, c); end;
                         inline void foo(a, b, c, d...) print(a, b, c); end;
@@ -1124,12 +1126,12 @@ void reportsVarargFunctionConflict() {
     }
 
     @Test
-    void reportsFunctionCallMismatch() {
+    void reportsUnresolvedFunctionCalls() {
         assertGeneratesMessages(
                 ExpectedMessages.create()
-                        .add(5, 1, "Function 'foo': wrong number of arguments (expected at least 3, found 2).")
-                        .add(6, 12, "Parameter 'b' isn't output, 'out' modifier not allowed.")
-                        .add(7, 11, "Parameter 'c' isn't output, 'out' modifier not allowed.")
+                        .add(5, 1, "Cannot resolve function 'foo'.")
+                        .add(6, 1, "Cannot resolve function 'foo'.")
+                        .add(7, 1, "Cannot resolve function 'foo'.")
                 ,
                 """
                         inline void foo(a) print(a); end;
@@ -1143,6 +1145,27 @@ void reportsFunctionCallMismatch() {
         );
     }
 
+
+    @Test
+    void reportsWrongFunctionArguments() {
+        assertGeneratesMessages(
+                ExpectedMessages.create()
+                        .add(5, 1, "Function 'foo': wrong number of arguments (expected 1, found 2).")
+                        .add(6, 12, "Parameter 'b' isn't output, 'out' modifier not allowed.")
+                        .add(7, 11, "Parameter 'c' isn't output, 'out' modifier not allowed.")
+                ,
+                """
+                        inline void foo(a) print(a); end;
+                        inline void bar(out a, b) print(b); a = b; end;
+                        inline void baz(a, b, c, d...) print(a, b); end;
+                        
+                        foo(1, 2);
+                        bar(out a, out b);
+                        baz(a, b, out c);
+                        """
+        );
+    }
+
     @Test
     void generatesMissingOutModifierWarning() {
         assertGeneratesMessages(

doc/syntax/SYNTAX-4-FUNCTIONS.markdown

@@ -525,6 +525,22 @@ foo(1, 2, 3);               // Error - bar needs an out argument at the third po
 > [!NOTE]
 > The errors produced when passing the vararg parameter to another function are reported at the call of the inner function, not at the call of the vararg function, and may therefore be a bit misleading. Use with caution. 
 
+The vararg parameter may be mixed with ordinary variables or expressions, and used more than once, even in the same list iteration or function call:
+
+```
+inline void foo(arg...)
+    bar(10, arg, 20, arg); 
+end;
+
+inline void bar(a, b, c...)
+    for i in a, b, c, c do
+        println(i);
+    end;
+end;
+
+foo(1, 2, 3);
+```
+
 ## Function overloading
 
 Mindcode supports function overloading. Several functions can have the same name, provided they differ in the number of arguments they take. For example:
@@ -542,13 +558,19 @@ foo(1);     // Calls foo(x)
 foo(1, 1);  // Calls foo(y) 
 ```
 
-Functions are primarily distinguished by the number of parameters. Declaring two functions with the same name and the same number of parameters results in an error. Please note that a function with output parameters, as well as a vararg function, takes variable number of arguments and therefore may clash with a function having a different number of parameters:
+To match a function call with function declaration, the number and types (input, output or input/output) of function call arguments must match the number and types of function parameters. Optional and vararg arguments are taken into account when evaluating the match.
+
+When two or more function declarations could be matched by the same function call, the functions conflicts with each other. Declaring conflicting functions results in an error. Please note that a function with output parameters, as well as a vararg function, takes variable number of arguments and therefore may conflict with a function having a different number of parameters:
+
+* `void foo(a, b)`: called with two arguments.
+* `void foo(x, y, out z)`: conflict - may be also called with two arguments when `z` is omitted.
+* `void foo(m, n, out o, args...)`: conflict - may be also called with two arguments when `o` is omitted and no additional arguments are given.
 
-* `void foo(x, y)`: called with two arguments
-* `foo(x, y, out z)`: may be also called with two arguments when `z` is omitted
-* `void foo(x, y, out z, args...)`: may be also called with two arguments when `z` is omitted and no additional arguments given
+* `void bar(x)`: `x` is an input parameter
+* `void bar(out y)`: `y` is an output parameter, therefore the function is different from `bar(x)`.
+* `void bar(in out z)`: `z` is an input/output parameter, therefor the function clashes with both `bar(x)` and `bar(out y)`.
 
-Mindcode will report all clashes of function declarations as errors, even if there aren't any ambiguous function calls.
+Mindcode will report all conflicts of function declarations as errors, even if there aren't any ambiguous function calls.
 
 A user defined function may have the same name as a Mindustry Logic function. User defined functions override Mindustry Logic functions of the same name. When a function call matches the user defined function, the user defined function will be called instead of Mindustry Logic function: