Merge pull request #645 from OpenVADL/bugfix/fix-crash-in-higher-order-macros-too-many-arguments

parser: Fix crash in higher order macros with too many arguments

Author: flofriday

vadl/main/vadl/ast/ParserUtils.java

@@ -528,16 +528,30 @@ static Diagnostic unknownSyntaxTypeError(String name, SymbolTable macroTable,
         .build();
   }
 
-  static Diagnostic tooManyMacroArgumentsError(Macro macro, SourceLocation location) {
+  static Diagnostic tooManyMacroArgumentsError(@Nullable Macro macro, SyntaxType type,
+                                               SourceLocation location) {
     // Unfortunately, we need the types of the macro parameters to parse the invocation to
     // completion. But if more arguments are provided than parameter are defined we cannot parse
     // them and therefore only know that too many exist but not how many were provided.
-    return error("Invalid Model Invocation", location)
-        .locationDescription(location,
-            "Model `%s` only expected %d arguments but, you provided at least %d.",
-            macro.name().name,
-            macro.params().size(), macro.params().size() + 1)
-        .build();
+    // The macro may not exist if we are calling a macro that is passed to the current macro, in
+    // which case we need to rely on the type.
+    var builder = error("Invalid Model Invocation", location);
+    if (macro != null) {
+      builder.locationDescription(location,
+          "Model `%s` only expected %d arguments but, you provided at least %d.",
+          macro.name().name,
+          macro.params().size(), macro.params().size() + 1);
+    } else {
+      var argCount = switch (type) {
+        case ProjectionType pt -> pt.arguments.size();
+        default -> 1;
+      };
+
+      builder.locationDescription(location,
+          "The model only expected %d arguments but, you provided at least %d.",
+          argCount, argCount + 1);
+    }
+    return builder.build();
   }
 
   private static boolean isPlaceholder(Node n) {

vadl/main/vadl/ast/vadl.ATG

@@ -2062,15 +2062,15 @@ PRODUCTIONS
                                                       // If too many arguments exist we cannot parse them because we
                                                       // need the information from the model signature to know how to
                                                       // parse them so instead we must abort parsing and throw an error.
-                                                      if (!params.hasNext()) throw ParserUtils.tooManyMacroArgumentsError(macro, nextTokenLoc());
+                                                      if (!params.hasNext()) throw ParserUtils.tooManyMacroArgumentsError(macro, paramType, nextTokenLoc());
                                                   .)
         macroBody<out Node arg1, params.next()>   (. args.add(arg1); .)
         {
           SYM_SEMICOLON                           (.
                                                       // If too many arguments exist we cannot parse them because we
                                                       // need the information from the model signature to know how to
                                                       // parse them so instead we must abort parsing and throw an error.
-                                                      if (!params.hasNext()) throw ParserUtils.tooManyMacroArgumentsError(macro, nextTokenLoc());
+                                                      if (!params.hasNext()) throw ParserUtils.tooManyMacroArgumentsError(macro, paramType,nextTokenLoc());
                                                   .)
           macroBody<out Node arg2, params.next()> (. args.add(arg2); .)
         }

vadl/test/resources/diagnostics/parser/invalidHigherOrderMacroWithTooManyArguments.vadl

@@ -0,0 +1,25 @@
+// In a previous version this example lead to a crash
+// This was only because the error code assumed that the invalidly called macro always exists but since we support
+// higher order macros like this, the assumption isn't correct.
+// https://github.com/OpenVADL/openvadl/issues/644
+
+record InstrWithFunct (id: Id, mnemo: Str, opcode: Ex, funct: Id)
+
+model-type InstrWithFunctElemSize = (InstrWithFunct, Id, Id) -> IsaDefs
+
+model InstrBHSD (modelid: InstrWithFunctElemSize, i: InstrWithFunct): IsaDefs = {
+  $modelid ($i; ElementsB; SizeB; ".B")
+}
+
+
+//  Reported Diagnostics:
+//
+//  error: Invalid Model Invocation
+//       ╭──[test/resources/diagnostics/parser/invalidHigherOrderMacroWithTooManyArguments.vadl:11:35]
+//       │
+//    11 │   $modelid ($i; ElementsB; SizeB; ".B")
+//       │                                   ^^^^ The model only expected 3 arguments but, you provided at least 4.
+//       │
+//
+//
+// Part of the class vadl.ast.DiagnosticsTest

vadl/test/resources/diagnostics/parser/invalidModelWithTooManyArguments.vadl

@@ -0,0 +1,18 @@
+model flo (id: Id): Ex = {
+  1 + 2
+}
+
+constant x = $flo(y; z)
+
+
+//  Reported Diagnostics:
+//
+//  error: Invalid Model Invocation
+//       ╭──[test/resources/diagnostics/parser/invalidModelWithTooManyArguments.vadl:5:22]
+//       │
+//     5 │ constant x = $flo(y; z)
+//       │                      ^ Model `flo` only expected 1 arguments but, you provided at least 2.
+//       │
+//
+//
+// Part of the class vadl.ast.DiagnosticsTest