[llvm] [mlir] [TableGen] Do not exit on template argument check (PR #121636)

[Markus Böck via llvm-commits]

https://github.com/zero9178 created https://github.com/llvm/llvm-project/pull/121636

The signature of `CheckTemplateArgValues` implements error handling via the `bool` return type, yet always returned false. The single possible error case instead used `PrintFatalError,` which exits the program afterward.

This behavior is undesirable: It prevents any further errors from being printed and makes TableGen less usable as a library as it crashes the entire process (e.g. `tblgen-lsp-server`).

This PR therefore fixes the issue by using `Error` instead and returning true if an error occurred. All callers already perform proper error handling.

As `llvm-tblgen` exits on error, a test was also added to the LSP to ensure it exits normally despite the error.

>From 122c9da3a7cded6e9a5dd3a77bf016b381df9755 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Markus=20B=C3=B6ck?= <markus.boeck02 at gmail.com>
Date: Sat, 4 Jan 2025 11:59:46 +0100
Subject: [PATCH] [TableGen] Do not exit on template argument check

The signature of `CheckTemplateArgValues` implements error handling via the `bool` return type, yet always returned false.
The single possible error case instead used `PrintFatalError,` which exits the program afterward.

This behavior is undesirable: It prevents any further errors from being printed and makes TableGen less usable as a library as it crashes the entire process (e.g. `tblgen-lsp-server`).

This PR therefore fixes the issue by using `Error` instead and returning true if an error occurred. All callers already perform proper error handling.

As `llvm-tblgen` exits on error as well, a test was added to the LSP to ensure it exits normally.
---
 llvm/lib/TableGen/TGParser.cpp                  |  9 +++++----
 llvm/test/TableGen/template-args.td             | 17 ++++++++++++++---
 .../test/tblgen-lsp-server/templ-arg-check.test | 15 +++++++++++++++
 3 files changed, 34 insertions(+), 7 deletions(-)
 create mode 100644 mlir/test/tblgen-lsp-server/templ-arg-check.test

diff --git a/llvm/lib/TableGen/TGParser.cpp b/llvm/lib/TableGen/TGParser.cpp
index e8679439c81de3..d7a8bdefa1c508 100644
--- a/llvm/lib/TableGen/TGParser.cpp
+++ b/llvm/lib/TableGen/TGParser.cpp
@@ -4421,6 +4421,7 @@ bool TGParser::CheckTemplateArgValues(
     const Record *ArgsRec) {
   ArrayRef<const Init *> TArgs = ArgsRec->getTemplateArgs();
 
+  bool HasError = false;
   for (const ArgumentInit *&Value : Values) {
     const Init *ArgName = nullptr;
     if (Value->isPositional())
@@ -4439,16 +4440,16 @@ bool TGParser::CheckTemplateArgValues(
                "result of template arg value cast has wrong type");
         Value = Value->cloneWithValue(CastValue);
       } else {
-        PrintFatalError(Loc, "Value specified for template argument '" +
-                                 Arg->getNameInitAsString() + "' is of type " +
-                                 ArgValue->getType()->getAsString() +
+        HasError |= Error(Loc, "Value specified for template argument '" +
+                                Arg->getNameInitAsString() + "' is of type " +
+                                ArgValue->getType()->getAsString() +
                                  "; expected type " + ArgType->getAsString() +
                                  ": " + ArgValue->getAsString());
       }
     }
   }
 
-  return false;
+  return HasError;
 }
 
 #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP)
diff --git a/llvm/test/TableGen/template-args.td b/llvm/test/TableGen/template-args.td
index f3eb02dd823ef5..40623dcb701e0d 100644
--- a/llvm/test/TableGen/template-args.td
+++ b/llvm/test/TableGen/template-args.td
@@ -9,6 +9,7 @@
 // RUN: not llvm-tblgen -DERROR8 %s 2>&1 | FileCheck --check-prefix=ERROR8 %s
 // RUN: not llvm-tblgen -DERROR9 %s 2>&1 | FileCheck --check-prefix=ERROR9 %s
 // RUN: not llvm-tblgen -DERROR10 %s 2>&1 | FileCheck --check-prefix=ERROR10 %s
+// RUN: not llvm-tblgen -DERROR11 %s 2>&1 | FileCheck --check-prefix=ERROR11 %s
 
 // This file tests that all required arguments are specified and template
 // arguments are type-checked and cast if necessary.
@@ -158,13 +159,13 @@ defm MissingComma : TwoArgs<2 "two">;
 #ifdef ERROR8
 def error8: Class1;
 // ERROR8: value not specified for template argument 'Class1:nm'
-// ERROR8: 18:21: note: declared in 'Class1'
+// ERROR8: 19:21: note: declared in 'Class1'
 #endif
 
 #ifdef ERROR9
 defm error9: MC1;
 // ERROR9: value not specified for template argument 'MC1::nm'
-// ERROR9: 99:23: note: declared in 'MC1'
+// ERROR9: 100:23: note: declared in 'MC1'
 #endif
 
 #ifdef ERROR10
@@ -172,5 +173,15 @@ def error10 {
   int value = Class2<>.Code;
 }
 // ERROR10: value not specified for template argument 'Class2:cd'
-// ERROR10: 37:22: note: declared in 'Class2'
+// ERROR10: 38:22: note: declared in 'Class2'
+#endif
+
+#ifdef ERROR11
+
+class Foo<int i, int j>;
+
+def error11 : Foo<"", "">;
+// ERROR11: Value specified for template argument 'Foo:i' is of type string; expected type int: ""
+// ERROR11: Value specified for template argument 'Foo:j' is of type string; expected type int: ""
+
 #endif
diff --git a/mlir/test/tblgen-lsp-server/templ-arg-check.test b/mlir/test/tblgen-lsp-server/templ-arg-check.test
new file mode 100644
index 00000000000000..cda9b79a1f4633
--- /dev/null
+++ b/mlir/test/tblgen-lsp-server/templ-arg-check.test
@@ -0,0 +1,15 @@
+// RUN: tblgen-lsp-server -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"tablegen","capabilities":{},"trace":"off"}}
+// -----
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{
+  "uri":"test:///foo.td",
+  "languageId":"tablegen",
+  "version":1,
+  "text":"class Foo<int i>;\ndef : Foo<\"\">;"
+}}}
+// CHECK: "method": "textDocument/publishDiagnostics",
+// CHECK: "message": "Value specified for template argument 'Foo:i' is of type string; expected type int: \"\"",
+// -----
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+// -----
+{"jsonrpc":"2.0","method":"exit"}