[TableGen] Do not exit in template argument check #121636
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
zero9178
changed the title
Member
|
@llvm/pr-subscribers-tablegen @llvm/pr-subscribers-mlir Author: Markus Böck (zero9178) ChangesThe signature of 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. This PR therefore fixes the issue by using As Full diff: https://github.com/llvm/llvm-project/pull/121636.diff 3 Files Affected:
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"}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
jurahul
reviewed
Jan 5, 2025
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/30/builds/13245 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The signature of
CheckTemplateArgValuesimplements error handling via theboolreturn type, yet always returned false. The single possible error case instead usedPrintFatalError,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
Errorinstead and returning true if an error occurred. All callers already perform proper error handling.As
llvm-tblgenexits on error, a test was also added to the LSP to ensure it exits normally despite the error.