[mlir:PDLL] Fix signature help for operation operands

[rriddle]

Summary

We were currently only completing on the first operand because
the completion check was outside of the parse loop.

mlir/lib/Tools/PDLL/Parser/Parser.cpp

if (!consumeIf(Token::l_paren)) {
     // an inplace unconstrained operand range corresponding to all of the
     // operands of the operation. This avoids treating zero operands the same
     // way as "unconstrained operands".
     if (parserContext != ParserContext::Rewrite) {
       operands.push_back(createImplicitRangeVar(
           ast::ValueRangeConstraintDecl::create(ctx, loc), valueRangeTy));
     }
   } else if (!consumeIf(Token::r_paren)) {
+    // If the operand list was specified and non-empty, parse the operands.
+    do {
       // Check for operand signature code completion.
       if (curToken.is(Token::code_complete)) {
         codeCompleteOperationOperandsSignature(opName, operands.size());
         return failure();
       }
 
-    // If the operand list was specified and non-empty, parse the operands.
-    do {
       FailureOr<ast::Expr *> operand = parseExpr();
       if (failed(operand))
         return failure();
       operands.push_back(*operand);
     } while (consumeIf(Token::comma));
 
     if (failed(parseToken(Token::r_paren,
                           "expected `)` after operation operand list")))

mlir/test/mlir-pdll-lsp-server/include/included.td

 include "mlir/IR/OpBase.td"
 
-// This file is merely to test the processing of includes, it has
-// no other purpose or contents.
+def Test_Dialect : Dialect {
+  let name = "test";
+}
+
+def OpMulti : Op<Test_Dialect, "multi"> {
+  let arguments = (outs I64:$operand, I64:$operand2);
+  let results = (outs I64:$result, I64:$result2);
+}

mlir/test/mlir-pdll-lsp-server/signature-help.test

-// RUN: mlir-pdll-lsp-server -lit-test < %s | FileCheck -strict-whitespace %s
+// RUN: mlir-pdll-lsp-server -pdll-extra-dir %S -pdll-extra-dir %S/../../include -lit-test < %s | FileCheck -strict-whitespace %s
 {"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"pdll","capabilities":{},"trace":"off"}}
 // -----
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{
   "uri":"test:///foo.pdll",
   "languageId":"pdll",
   "version":1,
-  "text":"Constraint ValueCst(value: Value);\nPattern {\nlet root = op<test.op>() -> ();\nValueCst(root);\nerase root;\n}"
+  "text":"#include \"include/included.td\"\nConstraint ValueCst(value: Value, value2: Value);\nPattern {\nlet root = op<test.multi>(val: Value, val) -> (ty: Type, ty);\nValueCst(root.result, root.result2);\nerase root;\n}"
 }}}
 // -----
 {"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{
   "textDocument":{"uri":"test:///foo.pdll"},
-  "position":{"line":2,"character":23}
+  "position":{"line":3,"character":26}
 }}
 //      CHECK:  "id": 1
 // CHECK-NEXT:  "jsonrpc": "2.0",
 // CHECK-NEXT:  "result": {
 // CHECK-NEXT:    "activeParameter": 0,
 // CHECK-NEXT:    "activeSignature": 0,
 // CHECK-NEXT:    "signatures": [
 // CHECK-NEXT:      {
+// CHECK-NEXT:        "documentation": "`op<test.multi>` ODS operand specification",
+// CHECK-NEXT:        "label": "(operand: Value, operand2: Value)",
+// CHECK-NEXT:        "parameters": [
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "documentation": "64-bit signless integer",
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              1,
+// CHECK-NEXT:              15
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          },
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "documentation": "64-bit signless integer",
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              17,
+// CHECK-NEXT:              32
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          }
+// CHECK-NEXT:        ]
+// CHECK-NEXT:      },
+// CHECK-NEXT:      {
 // CHECK-NEXT:        "documentation": "Generic operation operand specification",
 // CHECK-NEXT:        "label": "(<operands>: ValueRange)",
 // CHECK-NEXT:        "parameters": [
 // CHECK-NEXT:          {
 // CHECK-NEXT:            "documentation": "All of the operands of the operation.",
 // CHECK-NEXT:            "label": [
 // CHECK-NEXT:              1,
 // CHECK-NEXT:              23
 // CHECK-NEXT:            ]
 // CHECK-NEXT:          }
 // CHECK-NEXT:        ]
 // CHECK-NEXT:      }
 // CHECK-NEXT:    ]
 // CHECK-NEXT:  }
 // -----
 {"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{
   "textDocument":{"uri":"test:///foo.pdll"},
-  "position":{"line":2,"character":29}
+  "position":{"line":3,"character":38}
+}}
+//      CHECK:  "id": 1
+// CHECK-NEXT:  "jsonrpc": "2.0",
+// CHECK-NEXT:  "result": {
+// CHECK-NEXT:    "activeParameter": 1,
+// CHECK-NEXT:    "activeSignature": 0,
+// CHECK-NEXT:    "signatures": [
+// CHECK-NEXT:      {
+// CHECK-NEXT:        "documentation": "`op<test.multi>` ODS operand specification",
+// CHECK-NEXT:        "label": "(operand: Value, operand2: Value)",
+// CHECK-NEXT:        "parameters": [
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "documentation": "64-bit signless integer",
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              1,
+// CHECK-NEXT:              15
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          },
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "documentation": "64-bit signless integer",
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              17,
+// CHECK-NEXT:              32
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          }
+// CHECK-NEXT:        ]
+// CHECK-NEXT:      }
+// CHECK-NEXT:    ]
+// CHECK-NEXT:  }
+// -----
+{"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{
+  "textDocument":{"uri":"test:///foo.pdll"},
+  "position":{"line":3,"character":47}
 }}
 //      CHECK:  "id": 1
 // CHECK-NEXT:  "jsonrpc": "2.0",
 // CHECK-NEXT:  "result": {
 // CHECK-NEXT:    "activeParameter": 0,
 // CHECK-NEXT:    "activeSignature": 0,
 // CHECK-NEXT:    "signatures": [
 // CHECK-NEXT:      {
+// CHECK-NEXT:        "documentation": "`op<test.multi>` ODS result specification",
+// CHECK-NEXT:        "label": "(result: Type, result2: Type)",
+// CHECK-NEXT:        "parameters": [
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "documentation": "64-bit signless integer",
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              1,
+// CHECK-NEXT:              13
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          },
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "documentation": "64-bit signless integer",
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              15,
+// CHECK-NEXT:              28
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          }
+// CHECK-NEXT:        ]
+// CHECK-NEXT:      },
+// CHECK-NEXT:      {
 // CHECK-NEXT:        "documentation": "Generic operation result specification",
 // CHECK-NEXT:        "label": "(<results>: TypeRange)",
 // CHECK-NEXT:        "parameters": [
 // CHECK-NEXT:          {
 // CHECK-NEXT:            "documentation": "All of the results of the operation.",
 // CHECK-NEXT:            "label": [
 // CHECK-NEXT:              1,
 // CHECK-NEXT:              21
 // CHECK-NEXT:            ]
 // CHECK-NEXT:          }
 // CHECK-NEXT:        ]
 // CHECK-NEXT:      }
 // CHECK-NEXT:    ]
 // CHECK-NEXT:  }
 // -----
 {"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{
   "textDocument":{"uri":"test:///foo.pdll"},
-  "position":{"line":3,"character":9}
+  "position":{"line":3,"character":57}
+}}
+//      CHECK:  "id": 1
+// CHECK-NEXT:  "jsonrpc": "2.0",
+// CHECK-NEXT:  "result": {
+// CHECK-NEXT:    "activeParameter": 1,
+// CHECK-NEXT:    "activeSignature": 0,
+// CHECK-NEXT:    "signatures": [
+// CHECK-NEXT:      {
+// CHECK-NEXT:        "documentation": "`op<test.multi>` ODS result specification",
+// CHECK-NEXT:        "label": "(result: Type, result2: Type)",
+// CHECK-NEXT:        "parameters": [
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "documentation": "64-bit signless integer",
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              1,
+// CHECK-NEXT:              13
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          },
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "documentation": "64-bit signless integer",
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              15,
+// CHECK-NEXT:              28
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          }
+// CHECK-NEXT:        ]
+// CHECK-NEXT:      }
+// CHECK-NEXT:    ]
+// CHECK-NEXT:  }
+// -----
+{"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{
+  "textDocument":{"uri":"test:///foo.pdll"},
+  "position":{"line":4,"character":9}
 }}
 //      CHECK:  "id": 1
 // CHECK-NEXT:  "jsonrpc": "2.0",
 // CHECK-NEXT:  "result": {
 // CHECK-NEXT:    "activeParameter": 0,
 // CHECK-NEXT:    "activeSignature": 0,
 // CHECK-NEXT:    "signatures": [
 // CHECK-NEXT:      {
-// CHECK-NEXT:        "label": "ValueCst(value: Value) -> Tuple<>",
+// CHECK-NEXT:        "label": "ValueCst(value: Value, value2: Value) -> Tuple<>",
 // CHECK-NEXT:        "parameters": [
 // CHECK-NEXT:          {
 // CHECK-NEXT:            "label": [
 // CHECK-NEXT:              9,
 // CHECK-NEXT:              21
 // CHECK-NEXT:            ]
+// CHECK-NEXT:          },
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              23,
+// CHECK-NEXT:              36
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          }
+// CHECK-NEXT:        ]
+// CHECK-NEXT:      }
+// CHECK-NEXT:    ]
+// CHECK-NEXT:  }
+// -----
+{"jsonrpc":"2.0","id":1,"method":"textDocument/signatureHelp","params":{
+  "textDocument":{"uri":"test:///foo.pdll"},
+  "position":{"line":4,"character":21}
+}}
+//      CHECK:  "id": 1
+// CHECK-NEXT:  "jsonrpc": "2.0",
+// CHECK-NEXT:  "result": {
+// CHECK-NEXT:    "activeParameter": 1,
+// CHECK-NEXT:    "activeSignature": 0,
+// CHECK-NEXT:    "signatures": [
+// CHECK-NEXT:      {
+// CHECK-NEXT:        "label": "ValueCst(value: Value, value2: Value) -> Tuple<>",
+// CHECK-NEXT:        "parameters": [
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              9,
+// CHECK-NEXT:              21
+// CHECK-NEXT:            ]
+// CHECK-NEXT:          },
+// CHECK-NEXT:          {
+// CHECK-NEXT:            "label": [
+// CHECK-NEXT:              23,
+// CHECK-NEXT:              36
+// CHECK-NEXT:            ]
 // CHECK-NEXT:          }
 // CHECK-NEXT:        ]
 // CHECK-NEXT:      }
 // CHECK-NEXT:    ]
 // CHECK-NEXT:  }
 // -----
 {"jsonrpc":"2.0","id":3,"method":"shutdown"}
 // -----
 {"jsonrpc":"2.0","method":"exit"}